On Thu, Jun 11, 2026 at 05:32:45PM +0800, John Groves wrote: > From: John Groves <[email protected]> > > dax_holder_notify_failure() reads dax_dev->holder_ops twice without > READ_ONCE() -- once for the NULL check and once for the indirect > notify_failure() call. A concurrent fs_put_dax() or kill_dax() can clear > holder_ops between the two reads, so the check can observe a non-NULL > pointer while the call dereferences NULL. >
Hello John, Thanks for this. Reviewed-by: Richard Cheng <[email protected]> Small message nit, kill_dax() isn't a racing clearer. Plus I think this only fix holder_ops double-fetch, the fs_put_dax() race issue is separate and pre-existing. Best regards, Richard Cheng. > Fetch holder_ops once into a local with READ_ONCE() so the NULL check and > the indirect call observe the same value. > > Fixes: 8012b86608552 ("dax: introduce holder for dax_device") > Suggested-by: Richard Cheng <[email protected]> > Signed-off-by: John Groves <[email protected]> > --- > drivers/dax/super.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 25cf99dd9360b..6b5ee6589e39b 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -303,6 +303,7 @@ EXPORT_SYMBOL_GPL(dax_recovery_write); > int dax_holder_notify_failure(struct dax_device *dax_dev, u64 off, > u64 len, int mf_flags) > { > + const struct dax_holder_operations *ops; > int rc, id; > > id = dax_read_lock(); > @@ -311,12 +312,18 @@ int dax_holder_notify_failure(struct dax_device > *dax_dev, u64 off, > goto out; > } > > - if (!dax_dev->holder_ops) { > + /* > + * Read holder_ops once: a concurrent fs_put_dax() or kill_dax() can > + * clear it. Without the single fetch the compiler could reload > + * between the NULL check and the call and dereference a NULL ops. > + */ > + ops = READ_ONCE(dax_dev->holder_ops); > + if (!ops) { > rc = -EOPNOTSUPP; > goto out; > } > > - rc = dax_dev->holder_ops->notify_failure(dax_dev, off, len, mf_flags); > + rc = ops->notify_failure(dax_dev, off, len, mf_flags); > out: > dax_read_unlock(id); > return rc; > -- > 2.53.0 > >
