Re: [PATCH V5 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] Validation check for 'pgmap_phys > phys' executes too late, allowing 'fsdev_clear_folio_state()' to process out-of-bounds PFNs and crash the kernel.
--

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

dax/fsdev: fail probe on invalid pgmap offset

This commit converts a WARN_ON to a fatal error when pgmap_phys exceeds phys,
preventing probe completion with an impossible state. It now returns -EINVAL 
with an error message instead of proceeding with an invalid 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 validation check execute too late
to prevent a crash?

Looking at fsdev_dax_probe(), the call to fsdev_clear_folio_state() happens
just before this check is reached. fsdev_clear_folio_state() iterates over
the physical PFNs defined in dev_dax->ranges, starting at phys. 

Because devm_memremap_pages() only populates vmemmap starting from
pgmap_phys, any PFNs between phys and pgmap_phys lack vmemmap backing. This
causes an out-of-bounds access when attempting to read the folio state:

fsdev_dax_probe()
  fsdev_clear_folio_state()
    struct folio *folio = pfn_folio(pfn);
    int order = dax_folio_reset_order(folio);

Could this trigger a fatal kernel page fault on the unmapped PFNs before the
probe has a chance to fail gracefully?

Should the validation block be moved above the call to
fsdev_clear_folio_state() to reject the invalid configuration before memory
is accessed?

> +		data_offset = phys - pgmap_phys;
>  
>  		pr_debug("%s: offset detected phys=%llx pgmap_phys=%llx offset=%llx\n",
>  		       __func__, phys, pgmap_phys, data_offset);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019eb7bcda4b-3f8edae9-d7a4-4bfa-aaea-fcef77fdbbc3-000000@email.amazonses.com?part=6