Re: [PATCH V6 02/10] dax/fsdev: fix multi-range offset in memory_failure handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data Race / Out-Of-Bounds Read / Use-After-Free: Lockless iteration over mutable dev_dax->ranges causes new Out-Of-Bounds read
--

commit 41ef54df14f24d7b6da66ee222110e1acd13b89c
Author: John Groves <John@Groves.net>

dax/fsdev: fix multi-range offset in memory_failure handler

This commit introduces fsdev_pfn_to_offset() to calculate the correct 
multi-range offset for a faulting PFN. It changes the calculation to walk 
the dev_dax->ranges[] array instead of just reading index 0 to account 
for physical gaps between ranges.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -135,11 +135,26 @@ static void fsdev_clear_ops(void *data)
>   * The core mm code in free_zone_device_folio() handles the wake_up_var()
>   * directly for this memory type.
>   */
> +static u64 fsdev_pfn_to_offset(struct dev_dax *dev_dax, unsigned long pfn)
> +{
> +	phys_addr_t phys = PFN_PHYS(pfn);
> +	u64 offset = 0;
> +
> +	for (int i = 0; i < dev_dax->nr_range; i++) {
> +		struct range *range = &dev_dax->ranges[i].range;

[Severity: High]
Does this lockless iteration introduce an out-of-bounds read risk?

The commit message notes that reading dev_dax->ranges[0] locklessly is a
pre-existing exposure. However, looping over the array using nr_range
adds a new issue.

If this runs concurrently with alloc_dev_dax_range() in drivers/dax/bus.c,
the ranges array can be reallocated and nr_range incremented:

drivers/dax/bus.c:alloc_dev_dax_range() {
        ranges = krealloc(dev_dax->ranges, ...);
        ...
        dev_dax->ranges = ranges;
        ranges[dev_dax->nr_range++] = (struct dev_dax_range) {
        ...
}

Without read memory barriers here in fsdev_pfn_to_offset(), could a CPU
observe the updated dev_dax->nr_range but still see the old (freed)
dev_dax->ranges pointer?

If so, this loop could iterate past the bounds of the old freed allocation,
causing an out-of-bounds read on the freed slab object.

> +
> +		if (phys >= range->start && phys <= range->end)
> +			return offset + (phys - range->start);
> +		offset += range_len(range);
> +	}
> +	return -1ULL;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0100019ecc080a68-8dc0c99f-ab17-4aa9-83d9-490e9c97ac2e-000000@email.amazonses.com?part=2