linux-kernel - Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZJx02OVPd4BJGmZk@slm.duckdns.org>
Date:   Wed, 28 Jun 2023 07:58:48 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Christian Brauner <brauner@...nel.org>
Cc:     Suren Baghdasaryan <surenb@...gle.com>, gregkh@...uxfoundation.org,
        peterz@...radead.org, lujialin4@...wei.com,
        lizefan.x@...edance.com, hannes@...xchg.org, mingo@...hat.com,
        ebiggers@...nel.org, oleg@...hat.com, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, vschneid@...hat.com,
        linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free
 resources tied to the file

Hello,

On Wed, Jun 28, 2023 at 09:26:07AM +0200, Christian Brauner wrote:
> > I think the root cause of this problem is that ->release() in kernfs
> > does not adhere to the common rule that ->release() is called only
> > when the file is going away and has no users left. Am I wrong?
> 
> So imho, ultimately this all comes down to rmdir() having special
> semantics in kernfs. On any regular filesystem an rmdir() on a directory

Yeap, rmdir needs to revoke all the existing open files for kernfs to allow
the subsystem to disappear afterwards.

> which is still referenced by a struct file doesn't trigger an
> f_op->release() operation. It's just that directory is unlinked and
> you get some sort of errno like ENOENT when you try to create new files
> in there or whatever. The actual f_op->release) however is triggered
> on last fput().
> 
> But in essence, kernfs treats an rmdir() operation as being equivalent
> to a final fput() such that it somehow magically kills all file
> references. And that's just wrong and not supported.

It is not supported in linux vfs but kernfs users need it, so it's a
semantic implemented in kernfs, which does add some complications but that's
the cost we pay for solving the problem of allowing device drivers or
whatever backing kernfs to go away when they want to.

I'm not sure what classifying a behavior requirement as wrong means. Do you
mean that we shouldn't allow device drives to be unloaded if someone forget
to close a sysfs file?

Thanks.

-- 
tejun