On 1/23/18 8:49 PM, Ming Lei wrote: > On Tue, Jan 23, 2018 at 02:32:06PM -0700, Jens Axboe wrote: >> On 1/23/18 10:20 AM, Eryu Guan wrote: >>> 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. > >> 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. Yeah good point, I guess the proposed fix is the simplest version we can do. At least it has a comment. I'll apply it, thanks. -- Jens Axboe
