Re: [PATCH V4 3/9] dax/fsdev: clear vmemmap_shift when binding static pgmap

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Dangling `pgmap->ops` and `pgmap->owner` function pointers cause Use-After-Free/Panic upon memory failure.
- [High] Stale `vmemmap_shift` in `device_dax` when rebinding static DAX devices with reduced alignment.
- [High] Re-initializing active `cdev` structure in probe leads to Use-After-Free and list corruption.
--

commit 98c8cbe94424d5d0c2736d9ed4d3c34493e26f6a
Author: John Groves <John@Groves.net>

dax/fsdev: clear vmemmap_shift when binding static pgmap

This commit explicitly zeros pgmap->vmemmap_shift for static dax devices
before calling devm_memremap_pages() in fsdev_dax. This ensures the
vmemmap is correctly built for order-0 folios, clearing any stale shift
value left behind when rebinding from device_dax.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index f315533b299e9..dbd722ed7ab05 100644
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -237,6 +237,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
>  		}
>  
>  		pgmap = dev_dax->pgmap;
> +		pgmap->vmemmap_shift = 0;

[Severity: High]
This isn't a bug introduced by this patch, but does drivers/dax/device.c
also need this fix?

In device.c:dev_dax_probe(), vmemmap_shift is updated conditionally:

	if (dev_dax->align > PAGE_SIZE)
		pgmap->vmemmap_shift =
			order_base_2(dev_dax->align >> PAGE_SHIFT);

If a static dax device is bound with an alignment greater than PAGE_SIZE,
unbound, has its alignment reduced to PAGE_SIZE via sysfs, and then rebound
to device_dax, it seems vmemmap_shift is not cleared and retains its stale
value.

Could this cause devm_memremap_pages() to erroneously build compound folios
for an order-0 dax device?


[Severity: Critical]
This is a pre-existing issue, but does rebinding between fsdev_dax and
device_dax leave dangling pointers in the shared pgmap?

When fsdev_dax binds to the pgmap, it populates its module-specific
callbacks:

drivers/dax/fsdev.c:fsdev_dax_probe() {
    ...
	pgmap->ops = &fsdev_pagemap_ops;
	pgmap->owner = dev_dax;
    ...
}

If the device is later rebound to device_dax, dev_dax_probe() only updates
pgmap->type and does not appear to clear pgmap->ops or pgmap->owner:

drivers/dax/device.c:dev_dax_probe() {
    ...
	pgmap->type = MEMORY_DEVICE_GENERIC;
    ...
}

If fsdev_dax is then unloaded and a memory failure occurs, could
memory_failure() invoke the dangling pgmap->ops->memory_failure() pointer,
resulting in a use-after-free and kernel panic?


[Severity: High]
This is a pre-existing issue, but does calling cdev_init() during probe
risk list corruption and use-after-free if the vfs still holds references?

Both fsdev_dax_probe() and dev_dax_probe() call cdev_init() on the cdev
embedded within the long-lived dax_device structure:

drivers/dax/fsdev.c:fsdev_dax_probe() {
    ...
	cdev = inode->i_cdev;
	cdev_init(cdev, &fsdev_fops);
    ...
}

If the driver is unbound and rebound while userspace still holds open file
descriptors for the device, the vfs maintains references to the cdev.

Doesn't cdev_init() zero-initialize the cdev and re-initialize its kobject,
overwriting the active reference count and clearing cdev->list?

>  	} else {
>  		size_t pgmap_size;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019ea3929225-a0f8e6f7-30ae-4f8e-ae6f-19129666c4c3-000000@email.amazonses.com?part=3