On 26/05/26 05:28PM, Dave Jiang wrote: > > > On 5/22/26 12:19 PM, John Groves wrote: > > From: John Groves <[email protected]> > > > > This fix is in response to a Sashiko review, and some subsequent > > analysis. > > > > dax_dev_get() uses iget5_locked() which creates a new inode if no > > matching one exists. This is correct for the internal caller > > (alloc_dax), but dangerous for external callers that look up devices > > from user-supplied or metadata-supplied dev_t values: > > > > 1. A new inode is created with DAXDEV_ALIVE set but no backing driver, > > no ops, and no IDA-allocated minor number. > > > > 2. On teardown, dax_destroy_inode() warns because kill_dax() was never > > called, and dax_free_inode() calls ida_free() for a minor that was > > never ida_alloc'd — potentially freeing the minor of a real device. > > > > Add dax_dev_find() which uses ilookup5() for lookup-only semantics: > > it returns an existing dax_device with an elevated inode reference, or > > NULL if no device with the given dev_t exists. It never creates inodes. > > > > Make dax_dev_get() static again (internal to super.c for alloc_dax), > > export dax_dev_find() instead, and update the two external callers > > (famfs_inode.c, famfs.c). Also add the missing CONFIG_DAX=n stub. > > > > Fixes: 2ae624d5a555d ("dax: export dax_dev_get()") > > Signed-off-by: John Groves <[email protected]> > > --- > > drivers/dax/super.c | 27 +++++++++++++++++++++++++-- > > include/linux/dax.h | 6 +++++- > > 2 files changed, 30 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > > index fa1d2a6eb2408..79e5823d1010d 100644 > > --- a/drivers/dax/super.c > > +++ b/drivers/dax/super.c > > @@ -541,7 +541,7 @@ static int dax_set(struct inode *inode, void *data) > > return 0; > > } > > > > -struct dax_device *dax_dev_get(dev_t devt) > > +static struct dax_device *dax_dev_get(dev_t devt) > > { > > struct dax_device *dax_dev; > > struct inode *inode; > > @@ -564,7 +564,30 @@ struct dax_device *dax_dev_get(dev_t devt) > > > > return dax_dev; > > } > > -EXPORT_SYMBOL_GPL(dax_dev_get); > > + > > +/** > > + * dax_dev_find - look up an existing dax_device by dev_t > > + * @devt: the device number to find > > + * > > + * Returns a dax_device with an elevated inode reference, or NULL if no > > + * device with the given dev_t exists. Unlike dax_dev_get(), this never > > + * allocates a new inode — it is safe for external callers that are looking > > + * up devices from user-supplied or metadata-supplied dev_t values. > > + * > > + * Caller must put_dax() the returned device when done. > > + */ > > +struct dax_device *dax_dev_find(dev_t devt) > > +{ > > + struct inode *inode; > > + > > + inode = ilookup5(dax_superblock, hash_32(devt + DAXFS_MAGIC, 31), > > + dax_test, &devt); > > + if (!inode) > > + return NULL; > > + > > Claude mentions that dax_alive() check may be a good idea after grabbing the > inode ref. Otherwise famfs may get a dax_dev that may be racing with a > teardown. Do something similar that fs_dax_get_by_dev() or fs_dax_get() do > WRT dax_alive() check perhaps. > > DJ
I think that's right. Calling dax_alive() requires a dax_read_lock(); Adding that too. Thanks! John <snip>
