On Wed, Jan 24, 2018 at 11:49:26AM +0800, Ming Lei wrote:

On Tue, Jan 23, 2018 at 02:32:06PM -0700, Jens Axboe wrote:

quoted

On 1/23/18 10:20 AM, Eryu Guan wrote:

quoted

Attributes that only implement .seq_ops are read-only, any write to
them should be rejected. But currently kernel would crash when
writing to such debugfs entries, e.g.

chmod +w /sys/kernel/debug/block/<dev>/requeue_list
echo 0 > /sys/kernel/debug/block/<dev>/requeue_list
chmod -w /sys/kernel/debug/block/<dev>/requeue_list

Fix it by returning -EPERM in blk_mq_debugfs_write() when writing to
such attributes.

I don't particularly like the fix, since it's not really clear why
that comparison makes sense. Can't we just prevent anyone from

It might be the simplest way to check if the attribute defines .seq_ops
or not. If it is .seq_ops, it is wrong to interpret m->private as
'struct blk_mq_debugfs_attr *' because it actually points to 'struct
request_queue *' or others, which depends on the specific attribute.

So it works for avoiding the oops.

I agreed this is not a elegant fix but, as Ming suggested, might be the
simplest. I could put more comments in the code about why the comparison
makes sense.

Thanks,
Eryu

quoted

making the debugfs entries writable? Seems like a much more sane
approach.

I guess fs should allow root user to do 'chmod +w' on files in proc,
debugfs or sysfs. I just tried, it works on proc, debugfs and sysfs.


Thanks,
Ming