From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-lf0-f44.google.com ([209.85.215.44]:35070 "EHLO
        mail-lf0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1168878AbcKALWs (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Tue, 1 Nov 2016 07:22:48 -0400
Received: by mail-lf0-f44.google.com with SMTP id f134so118700198lfg.2
        for <linux-fsdevel@vger.kernel.org>; Tue, 01 Nov 2016 04:22:47 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <CAOg9mSQnLScUGiduGCDHsSPGWf4+6g4eqNmYhOqfvPyLgt=Tyg@mail.gmail.com>
References: <CAOg9mSSje9unMDnw9NhDd+cC=Ns9c+TK1fE2S0HytLU=HXYDpQ@mail.gmail.com>
 <20161031193823.GA11187@kroah.com> <87wpgoff0o.fsf@gmail.com> <CAOg9mSQnLScUGiduGCDHsSPGWf4+6g4eqNmYhOqfvPyLgt=Tyg@mail.gmail.com>
From: Mike Marshall <hubcap@omnibond.com>
Date: Tue, 1 Nov 2016 07:22:45 -0400
Message-ID: <CAOg9mSTWsSQk2-Zu6szeuYp8n-6kH3BUCmD0j_eUCkaN68kshQ@mail.gmail.com>
Subject: Re: debugfs question...
To: Nicolai Stange <nicstange@gmail.com>
Cc: Greg KH <greg@kroah.com>, Al Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Martin Brandenburg <martin@omnibond.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

Hi Nicolai...

I responded while rushing out of the office yesterday. Martin pushed
me to look for problems in that area after he refactored our debugfs code.
You seasoned folks probably think it is silly to desire "linux winkie buttons",
but it would be accurate to stamp a "reported by" on there for
Martin Brandenburg too <g> ...

-Mike

On Mon, Oct 31, 2016 at 4:30 PM, Mike Marshall <hubcap@omnibond.com> wrote:
>  > May I add your (Mike Marshall's?) Reported-by?
>
> Yes please <g> ...
>
> -Mike
>
> On Mon, Oct 31, 2016 at 4:19 PM, Nicolai Stange <nicstange@gmail.com> 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?
>>
>>
>>>> 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