Thread (42 messages) 42 messages, 7 authors, 2025-12-12

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

From: Tzung-Bi Shih <tzungbi@kernel.org>
Date: 2025-10-17 02:37:03
Also in: chrome-platform, linux-kselftest, lkml

On Thu, Oct 16, 2025 at 09:31:49AM -0300, Jason Gunthorpe wrote:
On Thu, Oct 16, 2025 at 05:42:02AM +0000, Tzung-Bi Shih wrote:
quoted
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@kernel.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:
quoted
+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.
Imagining the following example:

/* res1 and res2 are provided by hot-pluggable devices. */
struct filp_priv {
    void *res1;
    void *res2;
};

/* In .open() fops */
priv = kzalloc(sizeof(struct filp_priv), ...);
priv->res1 = ...;
priv->res2 = ...;
filp->private_data = priv;

/* In .read() fops */
priv = filp->private_data;
priv->res1    // could result UAF if the device has gone
priv->res2    // could result UAF if the device has gone


How does the bool * work for the example?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help