linux-kernel - Re: [PATCH v5 5/7] revocable: Add fops replacement

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: <20251016123149.GA88213@nvidia.com>
Date: Thu, 16 Oct 2025 09:31:49 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Tzung-Bi Shih <tzungbi@...nel.org>
Cc: Benson Leung <bleung@...omium.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	chrome-platform@...ts.linux.dev, linux-kselftest@...r.kernel.org,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Simona Vetter <simona.vetter@...ll.ch>,
	Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH v5 5/7] revocable: Add fops replacement

On Thu, Oct 16, 2025 at 05:42:02AM +0000, Tzung-Bi Shih wrote:
> Introduce fs_revocable_replace() to simplify the use of the revocable
> API with file_operations.
> 
> The function, should be called from a driver's ->open(), replaces the
> fops with a wrapper that automatically handles the `try_access` and
> `withdraw_access`.
> 
> When the file is closed, the wrapper's ->release() restores the original
> fops and cleanups.  This centralizes the revocable logic, making drivers
> cleaner and easier to maintain.
> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@...nel.org>
> ---
> PoC patch.
> 
> Known issues:
> - All file operations call revocable_try_access() for guaranteeing the
>   resource even if the resource may be unused in the fops.

Why is this so complicated??

You already added a per-flip struct:

> +struct fs_revocable_replacement {
> +	const struct fs_revocable_operations *frops;
> +	const struct file_operations *orig_fops;
> +	struct file_operations fops;
> +	struct revocable **revs;
> +	size_t num_revs;
> +};

Why does it need so much junk in it?

struct fs_revocable_replacement {
   struct srcu_struct srcu;
   bool *alive;
};

That's it. When the caller sets this up it provides a bool * pointer
from it's own private struct that is kept krefcounted to life cycle of
the struct file.

Then the ops wrapers are a simple thing - generate them with a macro:

srcu_read_lock(&f_rr->srcu);
if (*f_rr_>alive) 
   ret = f_rr->orig_fops->XX(...)
else
   ret = -ENODEV;
srcu_read_unlock(&f_rr->srcu);
return ret;

No need for all this revokable maze to do somethinig so simple.

Also, I don't think srcu is a good idea for this use case, maybe as an
option, but the default should be to use rwsem.

Jason