Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1562969

Re: debugfs vs. device removal

From Omar Sandoval <osandov@osandov.com>
Newsgroups linux.kernel
Subject Re: debugfs vs. device removal
Date 2017-01-19 18:40 +0100
Message-ID <t1oQP-4uu-37@gated-at.bofh.it> (permalink)
References <t1n8n-3mv-37@gated-at.bofh.it> <t1ni2-3q2-35@gated-at.bofh.it> <t1nrI-3IU-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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
> 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

debugfs vs. device removal Omar Sandoval <osandov@osandov.com> - 2017-01-19 16:50 +0100
  Re: debugfs vs. device removal Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-01-19 17:00 +0100
    Re: debugfs vs. device removal Jiri Kosina <jikos@kernel.org> - 2017-01-19 17:10 +0100
      Re: debugfs vs. device removal Omar Sandoval <osandov@osandov.com> - 2017-01-19 18:40 +0100
        Re: debugfs vs. device removal Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-01-19 19:30 +0100
          Re: debugfs vs. device removal Omar Sandoval <osandov@osandov.com> - 2017-01-19 20:50 +0100

csiph-web