From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-wm0-f65.google.com ([74.125.82.65]:36656 "EHLO
        mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S934067AbcKMSvF (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Sun, 13 Nov 2016 13:51:05 -0500
Received: by mail-wm0-f65.google.com with SMTP id m203so9764123wma.3
        for <linux-fsdevel@vger.kernel.org>; Sun, 13 Nov 2016 10:51:05 -0800 (PST)
From: Nicolai Stange <nicstange@gmail.com>
To: Mike Marshall <hubcap@omnibond.com>
Cc: Greg KH <greg@kroah.com>, Nicolai Stange <nicstange@gmail.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>
Subject: Re: debugfs question...
References: <CAOg9mSSje9unMDnw9NhDd+cC=Ns9c+TK1fE2S0HytLU=HXYDpQ@mail.gmail.com>
        <20161031193823.GA11187@kroah.com> <87wpgoff0o.fsf@gmail.com>
Date: Sun, 13 Nov 2016 19:51:02 +0100
In-Reply-To: <87wpgoff0o.fsf@gmail.com> (Nicolai Stange's message of "Mon, 31
        Oct 2016 21:19:03 +0100")
Message-ID: <877f876wo9.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

Hi again,

bad news: my previous analysis was completely wrong, c.f. below.
Good news (from my point of view): debugfs is correct, no fix needed for
it.

Apologies for the confusion...


Nicolai Stange <nicstange@gmail.com> writes:

> Greg KH <greg@kroah.com> writes:
>
>> On Mon, Oct 31, 2016 at 02:32:56PM -0400, Mike Marshall wrote:
>>
>>> 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.

This is what I got wrong: debugfs does acquire the needed references
correctly (details below). For the case of f2fs' "status" file, the
file_operations ->owner is simply not set as it should have been,
i.e. to THIS_MODULE.


Details on debugfs' reference acquisition:
The open proxy, full_proxy_open(), gets a reference to the "real"
file_operations, hence to its module. (Only in its error path it
releases it again).

full_proxy_release() is in charge of dropping that reference again, but
only *after* it has attempted to call the "real" ->release().

So, as long as a file has been (successfully) opened, a reference to the
original file_operation's ->owner is owned, preventing it from getting
unloaded.


Can you confirm that you didn't set ->owner in your Orangefs related
tests, too?


Thanks,

Nicolai