On Fri, Nov 07, 2025 at 05:07:54AM +0000, Tzung-Bi Shih wrote:
On Thu, Nov 06, 2025 at 11:47:15AM -0400, Jason Gunthorpe wrote:
On Thu, Nov 06, 2025 at 11:27:10PM +0800, Tzung-Bi Shih wrote:
+/*
- Recover the private_data to its original one.
- */
+static struct fops_replacement *_recover_private_data(struct file *filp) +{
- struct fops_replacement *fr = filp->private_data;
- filp->private_data = fr->orig_private_data;
- return fr;
+}
+/*
- Replace the private_data to fops_replacement.
- */
+static void _replace_private_data(struct fops_replacement *fr) +{
- fr->filp->private_data = fr;
+}
This switching of private_data isn't reasonable, it breaks too much stuff. I think I showed a better idea in my sketch.
The approach assumes the filp->private_data should be set once by the filp->f_op->open() if any. Is it common that the filp->private_data be updated in other file operations?
You can set it once during open, but you can't change it around every fops callback. This stuff is all concurrent.
This probably doesn't work out, is likely to make a memory leak. It will be hard for the owning driver to free its per-file memory without access to release.
Ah, I think this reveals a drawback of the approach.
- Without calling ->release(), some memory may leak.
- With calling ->release(), some UAF may happen.
It just means the user of this needs to understand there are limitations on what release can do. Usually release just frees memory, that is fine.
I think it would be strange for a release to touch revocable data, that might suggest some larger problem.
Jason