linux-newbie.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* debugfs_remove_recursive() while a file is in use by userspace
@ 2015-12-28 20:27 Rajat Jain
  2015-12-28 20:30 ` Al Viro
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rajat Jain @ 2015-12-28 20:27 UTC (permalink / raw)
  To: linux-kernel, linux-newbie; +Cc: Greg Kroah-Hartman

Hi,

I wanted to understand the behavior taken when a module calls
debugfs_remove_recursive() on a directory, while files under that
directory may still be in use by the userspace (for instance an
ongoing read / write operation).

Does the function wait

(1) until all the currently executing file operation methods
(read/write/map etc) have returned?
OR
(2) until the user has given up all references (descriptors) to the
files under the directory (i.e. until release() method has been
called)?

Thanks,

Rajat

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:27 debugfs_remove_recursive() while a file is in use by userspace Rajat Jain
@ 2015-12-28 20:30 ` Al Viro
  2015-12-28 20:31 ` Greg Kroah-Hartman
  2015-12-28 20:42 ` Nicolai Stange
  2 siblings, 0 replies; 9+ messages in thread
From: Al Viro @ 2015-12-28 20:30 UTC (permalink / raw)
  To: Rajat Jain; +Cc: linux-kernel, linux-newbie, Greg Kroah-Hartman

On Mon, Dec 28, 2015 at 12:27:22PM -0800, Rajat Jain wrote:
> Hi,
> 
> I wanted to understand the behavior taken when a module calls
> debugfs_remove_recursive() on a directory, while files under that
> directory may still be in use by the userspace (for instance an
> ongoing read / write operation).
> 
> Does the function wait
> 
> (1) until all the currently executing file operation methods
> (read/write/map etc) have returned?

No

> OR
> (2) until the user has given up all references (descriptors) to the
> files under the directory (i.e. until release() method has been
> called)?

... and no.  Don't use debugfs for objects that might have a finite
lifetime, or you'll get trouble.  And don't use debugfs outside of
well-isolated testing systems.
--
To unsubscribe from this list: send the line "unsubscribe linux-newbie" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.linux-learn.org/faqs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:27 debugfs_remove_recursive() while a file is in use by userspace Rajat Jain
  2015-12-28 20:30 ` Al Viro
@ 2015-12-28 20:31 ` Greg Kroah-Hartman
  2015-12-28 20:51   ` Rajat Jain
  2015-12-28 20:42 ` Nicolai Stange
  2 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-28 20:31 UTC (permalink / raw)
  To: Rajat Jain; +Cc: linux-kernel, linux-newbie

On Mon, Dec 28, 2015 at 12:27:22PM -0800, Rajat Jain wrote:
> Hi,
> 
> I wanted to understand the behavior taken when a module calls
> debugfs_remove_recursive() on a directory, while files under that
> directory may still be in use by the userspace (for instance an
> ongoing read / write operation).

Bad things :(

> Does the function wait
> 
> (1) until all the currently executing file operation methods
> (read/write/map etc) have returned?

Nope.

> OR
> (2) until the user has given up all references (descriptors) to the
> files under the directory (i.e. until release() method has been
> called)?

Nope.

There are some patches on the mailing list that I need to review that
hopefully should resolve this problem, as it's been known for a very
long time.

In short, just don't remove debugfs files unless your module is
unloading, and all should be good as modules are never auto-unloaded.
If you remove debugfs files when a device is removed, be careful.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:27 debugfs_remove_recursive() while a file is in use by userspace Rajat Jain
  2015-12-28 20:30 ` Al Viro
  2015-12-28 20:31 ` Greg Kroah-Hartman
@ 2015-12-28 20:42 ` Nicolai Stange
  2015-12-28 20:56   ` Rajat Jain
  2 siblings, 1 reply; 9+ messages in thread
From: Nicolai Stange @ 2015-12-28 20:42 UTC (permalink / raw)
  To: Rajat Jain; +Cc: linux-kernel, linux-newbie, Greg Kroah-Hartman

Hi Rajat,

Rajat Jain <rajatxjain@gmail.com> writes:

> Hi,
>
> I wanted to understand the behavior taken when a module calls
> debugfs_remove_recursive() on a directory, while files under that
> directory may still be in use by the userspace (for instance an
> ongoing read / write operation).
>
> Does the function wait
>
> (1) until all the currently executing file operation methods
> (read/write/map etc) have returned?
> OR
> (2) until the user has given up all references (descriptors) to the
> files under the directory (i.e. until release() method has been
> called)?

The current state is that both question are to be answered with "no",
i.e. debugfs file removal is racy.

I've recently sent a patch addressing this issue:
  https://lkml.kernel.org/g/87y4dfukzy.fsf@gmail.com

Basically, it turns the answer to your first question into "yes".
Subsequent reads/writes will return a -EIO.

That patch (series) is still under review though.

Further reference can be found in the *sub*-thread rooted at
  http://thread.gmane.org/gmane.linux.kernel/1452470/focus=1467314

Best,

Nicolai
--
To unsubscribe from this list: send the line "unsubscribe linux-newbie" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.linux-learn.org/faqs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:31 ` Greg Kroah-Hartman
@ 2015-12-28 20:51   ` Rajat Jain
  2015-12-28 20:58     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2015-12-28 20:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-newbie

Thanks Greg and Al for the quick turnaround.

Essentially I have a device that supports something called "contexts"
that can be "created" and "destroyed" during the life of the device. I
want to expose some debug files for the context when it is created,
and destroy the files when the context is destroyed. However, I'm not
sure how do I ensure that the user is not in the middle of reading /
writing / mmaping to those files. Also how do I know that user is
still not holding a reference to the file structure.

It seems like debugfs is currently not a good choice for this? Would
you recommend me to any other fs or subsystem that I should use for
this?

Would hanging those files under the sysfs node for the device sound
any better (by representing each "context" using an embedded kobject)?

Thanks,

Rajat


On Mon, Dec 28, 2015 at 12:31 PM, Greg Kroah-Hartman <greg@kroah.com> wrote:
> On Mon, Dec 28, 2015 at 12:27:22PM -0800, Rajat Jain wrote:
>> Hi,
>>
>> I wanted to understand the behavior taken when a module calls
>> debugfs_remove_recursive() on a directory, while files under that
>> directory may still be in use by the userspace (for instance an
>> ongoing read / write operation).
>
> Bad things :(
>
>> Does the function wait
>>
>> (1) until all the currently executing file operation methods
>> (read/write/map etc) have returned?
>
> Nope.
>
>> OR
>> (2) until the user has given up all references (descriptors) to the
>> files under the directory (i.e. until release() method has been
>> called)?
>
> Nope.
>
> There are some patches on the mailing list that I need to review that
> hopefully should resolve this problem, as it's been known for a very
> long time.
>
> In short, just don't remove debugfs files unless your module is
> unloading, and all should be good as modules are never auto-unloaded.
> If you remove debugfs files when a device is removed, be careful.
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-newbie" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.linux-learn.org/faqs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:42 ` Nicolai Stange
@ 2015-12-28 20:56   ` Rajat Jain
  0 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2015-12-28 20:56 UTC (permalink / raw)
  To: Nicolai Stange; +Cc: linux-kernel, linux-newbie, Greg Kroah-Hartman

>
> The current state is that both question are to be answered with "no",
> i.e. debugfs file removal is racy.
>

Ouch! :-(. Thanks for confirming!

Somewhat related, is my understanding correct that the behaviour of
character device layer w.r.t. the two questions that I asked the
answer to my questions would be "yes"?

i.e. cdev_del() would wait

>> (1) until all the currently executing file operation methods
>> (read/write/map etc) have returned?

AND

>> (2) until the user has given up all references (descriptors) to the
>> files (i.e. until release() method has been
>> called)?

Thanks,

Rajat


On Mon, Dec 28, 2015 at 12:42 PM, Nicolai Stange <nicstange@gmail.com> wrote:
> Hi Rajat,
>
> Rajat Jain <rajatxjain@gmail.com> writes:
>
>> Hi,
>>
>> I wanted to understand the behavior taken when a module calls
>> debugfs_remove_recursive() on a directory, while files under that
>> directory may still be in use by the userspace (for instance an
>> ongoing read / write operation).
>>
>> Does the function wait
>>
>> (1) until all the currently executing file operation methods
>> (read/write/map etc) have returned?
>> OR
>> (2) until the user has given up all references (descriptors) to the
>> files under the directory (i.e. until release() method has been
>> called)?
>
> The current state is that both question are to be answered with "no",
> i.e. debugfs file removal is racy.
>
> I've recently sent a patch addressing this issue:
>   https://lkml.kernel.org/g/87y4dfukzy.fsf@gmail.com
>
> Basically, it turns the answer to your first question into "yes".
> Subsequent reads/writes will return a -EIO.
>
> That patch (series) is still under review though.
>
> Further reference can be found in the *sub*-thread rooted at
>   http://thread.gmane.org/gmane.linux.kernel/1452470/focus=1467314
>
> Best,
>
> Nicolai
--
To unsubscribe from this list: send the line "unsubscribe linux-newbie" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.linux-learn.org/faqs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:51   ` Rajat Jain
@ 2015-12-28 20:58     ` Greg Kroah-Hartman
  2015-12-28 21:11       ` Rajat Jain
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-28 20:58 UTC (permalink / raw)
  To: Rajat Jain; +Cc: linux-kernel, linux-newbie

On Mon, Dec 28, 2015 at 12:51:32PM -0800, Rajat Jain wrote:
> Thanks Greg and Al for the quick turnaround.
> 
> Essentially I have a device that supports something called "contexts"
> that can be "created" and "destroyed" during the life of the device. I
> want to expose some debug files for the context when it is created,
> and destroy the files when the context is destroyed. However, I'm not
> sure how do I ensure that the user is not in the middle of reading /
> writing / mmaping to those files. Also how do I know that user is
> still not holding a reference to the file structure.

You don't.

> It seems like debugfs is currently not a good choice for this? Would
> you recommend me to any other fs or subsystem that I should use for
> this?

What exactly do you need to export to userspace and for what purpose?
For debugging-only stuff, sure, use debugfs, but don't rely on it for
any "real" tools, only your own debugging.

> Would hanging those files under the sysfs node for the device sound
> any better (by representing each "context" using an embedded kobject)?

That would ensure that things work properly.  But you don't need a whole
kobject, just use a named group and a subdir will be created properly
for you.

good luck,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 20:58     ` Greg Kroah-Hartman
@ 2015-12-28 21:11       ` Rajat Jain
  2015-12-28 21:17         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 9+ messages in thread
From: Rajat Jain @ 2015-12-28 21:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-newbie

On Mon, Dec 28, 2015 at 12:58 PM, Greg Kroah-Hartman <greg@kroah.com> wrote:
> On Mon, Dec 28, 2015 at 12:51:32PM -0800, Rajat Jain wrote:
>> Thanks Greg and Al for the quick turnaround.
>>
>> Essentially I have a device that supports something called "contexts"
>> that can be "created" and "destroyed" during the life of the device. I
>> want to expose some debug files for the context when it is created,
>> and destroy the files when the context is destroyed. However, I'm not
>> sure how do I ensure that the user is not in the middle of reading /
>> writing / mmaping to those files. Also how do I know that user is
>> still not holding a reference to the file structure.
>
> You don't.
>
>> It seems like debugfs is currently not a good choice for this? Would
>> you recommend me to any other fs or subsystem that I should use for
>> this?
>
> What exactly do you need to export to userspace and for what purpose?
> For debugging-only stuff, sure, use debugfs, but don't rely on it for
> any "real" tools, only your own debugging.

I'm actually writing a driver that would expose a "dummy device" to a
real driver. The dummy driver relies on user space to feed in the
device attributes (no of supported contexts etc). I am now thinking
that a character device interface to user space may actually be a
better choice. Question: Does cdev_del() ensure that all references to
the file are dropped before it returns?

Thanks,

Rajat

>
>> Would hanging those files under the sysfs node for the device sound
>> any better (by representing each "context" using an embedded kobject)?
>
> That would ensure that things work properly.  But you don't need a whole
> kobject, just use a named group and a subdir will be created properly
> for you.
>
> good luck,
>
> greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: debugfs_remove_recursive() while a file is in use by userspace
  2015-12-28 21:11       ` Rajat Jain
@ 2015-12-28 21:17         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-28 21:17 UTC (permalink / raw)
  To: Rajat Jain; +Cc: linux-kernel, linux-newbie

On Mon, Dec 28, 2015 at 01:11:53PM -0800, Rajat Jain wrote:
> On Mon, Dec 28, 2015 at 12:58 PM, Greg Kroah-Hartman <greg@kroah.com> wrote:
> > On Mon, Dec 28, 2015 at 12:51:32PM -0800, Rajat Jain wrote:
> >> Thanks Greg and Al for the quick turnaround.
> >>
> >> Essentially I have a device that supports something called "contexts"
> >> that can be "created" and "destroyed" during the life of the device. I
> >> want to expose some debug files for the context when it is created,
> >> and destroy the files when the context is destroyed. However, I'm not
> >> sure how do I ensure that the user is not in the middle of reading /
> >> writing / mmaping to those files. Also how do I know that user is
> >> still not holding a reference to the file structure.
> >
> > You don't.
> >
> >> It seems like debugfs is currently not a good choice for this? Would
> >> you recommend me to any other fs or subsystem that I should use for
> >> this?
> >
> > What exactly do you need to export to userspace and for what purpose?
> > For debugging-only stuff, sure, use debugfs, but don't rely on it for
> > any "real" tools, only your own debugging.
> 
> I'm actually writing a driver that would expose a "dummy device" to a
> real driver. The dummy driver relies on user space to feed in the
> device attributes (no of supported contexts etc).

That sounds "odd".

Why not configfs?

> I am now thinking that a character device interface to user space may
> actually be a better choice.

Not really, you would now have to parse things in a character driver
write() callback, not very good or robust.

> Question: Does cdev_del() ensure that all references to the file are
> dropped before it returns?

No, but when the reference is dropped, everything cleans up properly.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-newbie" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.linux-learn.org/faqs

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-12-28 21:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-28 20:27 debugfs_remove_recursive() while a file is in use by userspace Rajat Jain
2015-12-28 20:30 ` Al Viro
2015-12-28 20:31 ` Greg Kroah-Hartman
2015-12-28 20:51   ` Rajat Jain
2015-12-28 20:58     ` Greg Kroah-Hartman
2015-12-28 21:11       ` Rajat Jain
2015-12-28 21:17         ` Greg Kroah-Hartman
2015-12-28 20:42 ` Nicolai Stange
2015-12-28 20:56   ` Rajat Jain

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).