Re: debugfs question...

[Greg KH <greg@kroah.com>]

On Thu, Nov 10, 2016 at 06:48:44PM +0100, Nicolai Stange wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > On Mon, Oct 31, 2016 at 09:19:03PM +0100, Nicolai Stange wrote:
> >> Hi,
> >> 
> >> Greg KH <greg@kroah.com> writes:
> >> 
> >> > On Mon, Oct 31, 2016 at 02:32:56PM -0400, Mike Marshall wrote:
> >> 
> >> >
> >> > [adding Nicolai to thread...]
> >> >
> >> 
> >> >> 
> >> >> Our debugfs code results in three files in /sys/kernel/debug/orangefs.
> >> >> One of the files gets deleted (debugfs_remove'd) and re-created
> >> >> (debugfs_create_file'd) the first time someone fires up the
> >> >> user-space part of Orangefs after a reboot.
> >> >> 
> >> >> We wondered what awful things might happen if someone was
> >> >> reading the file across the delete/re-create, so I wrote a
> >> >> program that opens the file, sleeps ten seconds and then
> >> >> starts reading, and I fired up the Orangefs userspace part
> >> >> during the sleep. I didn't see any problems there, we get
> >> >> EIO when the read happens.
> >> >> 
> >> >> But... really bad things happen if someone unloads the Orangefs
> >> >> module after my test program does the open and before the read
> >> >> starts. So I picked another debugfs-using-filesystem (f2fs) and
> >> >> pointed my tester-program at /sys/kernel/debug/f2fs/status, and
> >> >> the same bad thing happens there.
> >> 
> >> [...]
> >> 
> >> >> [ 1240.144316] Call Trace:
> >> >> [ 1240.144450]  [<ffffffff8122907f>] __fput+0xdf/0x1d0
> >> >> [ 1240.144704]  [<ffffffff812291ae>] ____fput+0xe/0x10
> >> >> [ 1240.144962]  [<ffffffff810b97de>] task_work_run+0x8e/0xc0
> >> >> [ 1240.145243]  [<ffffffff8109b98e>] do_exit+0x2ae/0xae0
> >> 
> >> 
> >> Thank you very much for this detailed report!
> >> 
> >> At least for the .../f2fs/status file, your splat at fput() can be
> >> readily explained with the full proxy's releaser not being protected
> >> against file removals in any way.
> >> 
> >> Partly this is on purpose, c.f. the comment in full_proxy_release().
> >> 
> >> However, I should have at least tried to acquire a reference to the
> >> owning module before accessing some static struct file_operations or
> >> even calling some ->release() within it. Meh.
> >> 
> >> The fix should be relatively trivial and I'll hopefully manage to
> >> submit a patch tomorrow.
> >> 
> >> May I add your (Mike Marshall's?) Reported-by?
> >
> > Did this patch ever show up?
> 
> Not yet, sorry. I still have this on my list though and will definitely
> be able to find some free time this weekend.
> 
> I hope this is Ok.

That's fine, I was just worried I missed it.

thanks,

greg k-h