From: Omar Sandoval <osandov@osandov.com>
To: Jiri Kosina <jikos@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org
Subject: Re: debugfs vs. device removal
Date: Thu, 19 Jan 2017 09:33:50 -0800 [thread overview]
Message-ID: <20170119173350.GA7599@vader.DHCP.thefacebook.com> (raw)
In-Reply-To: <alpine.LSU.2.20.1701191700150.25515@cbobk.fhfr.pm>
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
>
next prev parent reply other threads:[~2017-01-19 17:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-19 15:48 debugfs vs. device removal Omar Sandoval
2017-01-19 15:53 ` Greg Kroah-Hartman
2017-01-19 16:03 ` Jiri Kosina
2017-01-19 17:33 ` Omar Sandoval [this message]
2017-01-19 18:03 ` Greg Kroah-Hartman
2017-01-19 19:40 ` Omar Sandoval
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170119173350.GA7599@vader.DHCP.thefacebook.com \
--to=osandov@osandov.com \
--cc=gregkh@linuxfoundation.org \
--cc=jikos@kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).