Re: [PATCH V3 8/9] dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Recycled `dax_device` objects inherit stale flags and operations due to lack of zero-initialization.
- [High] `alloc_dax()` exposes a partially initialized `dax_device` to concurrent lookups before `ops` and `private` are configured.
- [High] `dax_core_init()` incorrectly returns 0 on failure, leading to use-after-free panics during delayed DAX device allocations.
--

commit d84f23ffdf147bc18ecb76242e383acc9eb6cbc0
Author: John Groves <John@Groves.net>

dax: replace exported dax_dev_get() with non-allocating dax_dev_find()

This commit introduces dax_dev_find() for lookup-only semantics and makes
dax_dev_get() static. It resolves teardown warnings and potential minor
number corruption caused by allocating inodes without backing drivers.

> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index 4c56ac2faacdb..6cb271e034a70 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
[ ... ]
> @@ -573,7 +573,41 @@ 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
[ ... ]
> +struct dax_device *dax_dev_find(dev_t devt)
> +{
[ ... ]
> +	dax_dev = to_dax_dev(inode);
> +	id = dax_read_lock();
> +	if (!dax_alive(dax_dev)) {
> +		dax_read_unlock(id);
> +		iput(inode);
> +		return NULL;
> +	}
> +	dax_read_unlock(id);
> +
> +	return dax_dev;
> +}
> +EXPORT_SYMBOL_GPL(dax_dev_find);
>  
>  struct dax_device *alloc_dax(void *private, const struct dax_operations *ops)
>  {
>  	struct dax_device *dax_dev;
[ ... ]
>  	dax_dev = dax_dev_get(devt);
>  	if (!dax_dev)
>  		goto err_dev;
>  
>  	dax_dev->ops = ops;
>  	dax_dev->private = private;

[Severity: High]
This is a pre-existing issue, but does setting the operations pointer here
create a race condition with concurrent lookups?

Looking at dax_dev_get(), it calls unlock_new_inode() which makes the
inode globally visible in the VFS cache before alloc_dax() configures
these critical fields:

drivers/dax/super.c:dax_dev_get() {
    ...
    if (inode_state_read_once(inode) & I_NEW) {
        set_bit(DAXDEV_ALIVE, &dax_dev->flags);
        ...
        unlock_new_inode(inode);
    }
    return dax_dev;
}

Could a concurrent thread calling the newly introduced dax_dev_find()
observe dax_alive() returning true, but dereference an uninitialized or 
stale ops pointer when performing DAX operations?

>  	return dax_dev;
>  
>   err_dev:
[ ... ]


While reviewing this patch, I also noticed a couple of other pre-existing 
issues in drivers/dax/super.c that might be worth investigating:

[Severity: High]
This is a pre-existing issue, but can recycled dax_device objects inherit
stale flags?

In dax_alloc_inode(), the dax_device is allocated from the slab without
clearing the outer fields:

drivers/dax/super.c:dax_alloc_inode() {
    ...
    dax_dev = alloc_inode_sb(sb, dax_cache, GFP_KERNEL);
    ...
}

The init_once() function only clears the memory on the initial slab creation:

drivers/dax/super.c:init_once() {
    ...
    memset(dax_dev, 0, sizeof(*dax_dev));
    ...
}

Since bits like DAXDEV_SYNC and DAXDEV_WRITE_CACHE are set but never cleared
on teardown, could a newly allocated DAX device erroneously inherit these
states and skip required CPU cache flushes?


[Severity: High]
This is a pre-existing issue, but does dax_core_init() hide initialization
failures?

If subsystem initialization fails, the error path destroys critical global
structures but explicitly returns 0:

drivers/dax/super.c:dax_core_init() {
    ...
err_chrdev:
    dax_fs_exit();
    return 0;
}

Would this hide the failure from the kernel's init system, causing delayed
DAX device allocations via alloc_dax() or dax_dev_find() to attempt 
accessing the destroyed dax_cache or unmounted dax_superblock and panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019e79caead2-5795328c-af48-4a93-b147-c11df7446e1a-000000@email.amazonses.com?part=8