Re: [PATCH V4 6/9] dax/fsdev: fail probe on invalid pgmap offset

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The validation check for `pgmap_phys > phys` is unreachable in the exact failure case it is meant to handle, resulting in a kernel panic due to out-of-bounds `vmemmap` access before the bounds check is evaluated.
--

commit 18b2ee922105fd88cbce7e6ed1c6000666fc3c6c
Author: John Groves <John@Groves.net>
Subject: dax/fsdev: fail probe on invalid pgmap offset

This commit converts a warning into a fatal error during probe when the
remapped region starts after the device's data region. It aims to prevent
virt_addr misalignments by returning an error instead of continuing with a
zero offset.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
[ ... ]
> @@ -320,8 +320,12 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
>  		u64 phys = dev_dax->ranges[0].range.start;
>  		u64 pgmap_phys = pgmap[0].range.start;
>  
> -		if (!WARN_ON(pgmap_phys > phys))
> -			data_offset = phys - pgmap_phys;
> +		if (pgmap_phys > phys) {
> +			dev_err(dev, "pgmap start %#llx exceeds data start %#llx\n",
> +				pgmap_phys, phys);
> +			return -EINVAL;
> +		}

[Severity: High]
This is a pre-existing issue, but does this new validation check actually
execute before a crash occurs?

Looking at fsdev_dax_probe() in drivers/dax/fsdev.c, the validation block
handling the pgmap_phys > phys state is placed after
fsdev_clear_folio_state() is called.

fsdev_clear_folio_state() iterates unconditionally starting at phys:

drivers/dax/fsdev.c:fsdev_clear_folio_state() {
    ...
    while (pfn < end_pfn) {
        struct folio *folio = pfn_folio(pfn);
        ...
}

If phys < pgmap_phys, pfn_folio(pfn) dereferences pfns that fall outside
the remapped pgmap region, because vmemmap is only populated starting at
pgmap_phys via the earlier call to devm_memremap_pages().

Accessing these unpopulated vmemmap pages triggers an out-of-bounds page
fault.

Should the validation check and error return be relocated to execute before
fsdev_clear_folio_state() is invoked so that this graceful failure path is
reachable?

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