Re: debugfs question...

[Nicolai Stange <nicstange@gmail.com>]

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?


>> I was hoping that f2fs, or some other debugfs-using-filesystem, would be
>> able to handle my rmmod test and then I could look at their code for
>> inspiration, but no such luck so far. Is there something that me and the
>> f2fs guys aren't doing right or is this just something about debugfs
>> that's fragile?

It's debugfs which is broken as explained above, the code in f2fs looks
correct at a first glance.


Thanks again,

Nicolai