Re: [PATCH V4 4/9] dax/fsdev: don't leave a dangling dev_dax->pgmap on probe failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The identical dangling pointer bug that causes permanent probe failure and was fixed in `fsdev.c` still exists in `dev_dax_probe` within `drivers/dax/device.c`.
--

commit 4bfae7ab5cd902255b5838eb3c590fffc93926ef
Author: John Groves <John@Groves.net>

dax/fsdev: don't leave a dangling dev_dax->pgmap on probe failure

This commit fixes a bug in fsdev.c where dev_dax->pgmap was left dangling
if a probe failed after allocation. It defers assigning dev_dax->pgmap
until the probe can no longer fail.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index dbd722ed7ab05..0fd5e1293d725 100644
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
[ ... ]
> @@ -339,7 +354,13 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
>  		return rc;
>  
>  	run_dax(dax_dev);
> -	return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
> +	rc = devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
> +	if (rc)
> +		return rc;
> +
> +	/* Probe can no longer fail; expose the pgmap via dev_dax */
> +	dev_dax->pgmap = pgmap;
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does the same dangling pointer bug still
exist in dev_dax_probe() in drivers/dax/device.c?

The identical pattern of assigning dev_dax->pgmap immediately after the
devm_kzalloc() allocation appears to be present there:

drivers/dax/device.c:dev_dax_probe() {
        ...
        pgmap = devm_kzalloc(dev, ...);
        if (!pgmap)
                return -ENOMEM;
        
        dev_dax->pgmap = pgmap;
        ...
}

If a later step in the probe fails (such as devm_request_mem_region() or
devm_memremap_pages()), the devres framework automatically frees the
allocated pgmap. Since the cleanup action is only registered at the end of
a successful probe, the error paths leave the pointer dangling.

Would subsequent attempts to probe the driver then hit the check for
dev_dax->pgmap and erroneously fail with -EINVAL, permanently preventing
successful device binding? Does device.c need a similar fix?

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