On Wed, Sep 24, 2025 at 01:54:14PM -0700, [email protected] wrote: > Tzung-Bi Shih wrote: > > +int revocable_replace_fops(struct file *filp, struct revocable_provider > > *rp, > > + const struct revocable_operations *rops) > > +{ > > + struct fops_replacement *fr; > > + > > + fr = kzalloc(sizeof(*fr), GFP_KERNEL); > > + if (!fr) > > + return -ENOMEM; > > + > > + fr->filp = filp; > > + fr->rops = rops; > > + fr->orig_fops = filp->f_op; > > + fr->rev = revocable_alloc(rp); > > + if (!fr->rev) > > + return -ENOMEM; > > + memcpy(&fr->fops, filp->f_op, sizeof(struct file_operations)); > > + scoped_guard(mutex, &fops_replacement_mutex) > > + list_add(&fr->list, &fops_replacement_list); > > This list grows for every active instance? Unless I am misreading, that > looks like a scaling burden that the simple approach below does not > have.
Correct, unless we want to embed the context (e.g. struct fops_replacement) into struct file. FWIW: the issue also listed as a known issue after "---". > > + fr->fops.release = revocable_fr_release; > > + > > + if (filp->f_op->read) > > + fr->fops.read = revocable_fr_read; > > + if (filp->f_op->poll) > > + fr->fops.poll = revocable_fr_poll; > > + if (filp->f_op->unlocked_ioctl) > > + fr->fops.unlocked_ioctl = revocable_fr_unlocked_ioctl; > > + > > + filp->f_op = &fr->fops; > > + return 0; > > +} > > This facility is protecting the wrong resource, and I argue hides bugs > in drivers that think they need this. That matches the conclusion I came > to with my "managed_fops" attempt. > > The resource that is being revoked is the device's attachment to its > driver. Whether that is dev_get_drvdata() or some other device-to-data > lookup, that is the resource that gets removed, not the fops themselves. > The only resource race with fops is whether the code text section > remains available while the fops are registered, but that lifetime scope > is not at a per-device instance scope. revocable_replace_fops() doesn't protect any resources. It replaces the fops to revocable wrappers and recovers the fops when the file is releasing.
