On Thu, Jan 19, 2017 at 05:03:48PM +0100, Jiri Kosina wrote: > On Thu, 19 Jan 2017, Greg Kroah-Hartman wrote: > > > > In the block layer, we abuse sysfs to export some per-device debugging > > > information. I was looking into moving this to debugfs, but I realized > > > that debugfs doesn't have a mechanism to ensure that a file associated > > > with a device is safe to use when the device is removed. > > > > What do you mean by "safe"? The race conditions where you remove a file > > and still have it open should all now be resolved in 4.8 and 4.9, di dwe > > miss something? > > This is something else -- Omar is right, hid-debugfs interface is buggy. > It basically doesn't synchronize the data dumping with device removal, so > if device is removed and deallocated and the race is hit, it tries to > dereference struct hid_device which has already been freed.
Yup, I'm talking about the case where I create a debugfs file and the data pointer is, say, a struct request_queue. If userspace calls open() on a debugfs file, then the device goes away, the struct request_queue is going to get freed and read() will blow up. If we're talking about objects with a struct kobject (like struct request_queue), can we just grab an extra reference in open() and drop it in release()? This allows userspace to keep stuff pinned indefinitely, but debugfs is root-only and the use-case is usually just `cat`. > I'll look into fixing this later today or tomorrow. Basically we'd need to > synchronize between hid_remove_device() and anything in hid-debug and > whenever removal is pending, not to try to get any data out of it any more > and bail immediately. Something like rwlock (debugfs being the reader and > device removal being the writer) should work. > > Thanks, > > -- > Jiri Kosina > SUSE Labs >
