Re: [PATCH V4 2/9] 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:

Pre-existing issues:
- [High] Lockless iteration over the dynamically reallocated `dev_dax->ranges` array can cause a Use-After-Free or out-of-bounds access.
--

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

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

This commit fixes the memory failure offset calculation for multi-range
devices. It introduces fsdev_pfn_to_offset to walk the range list and
compute the correct device-linear byte offset instead of relying on the
start of the first range.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index 188b2526bee45..f315533b299e9 100644
> --- 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]
This is a pre-existing issue, but does lockless iteration over
dev_dax->ranges expose us to a use-after-free here?

If a privileged user writes to the sysfs mapping attribute concurrently,
it can trigger an array reallocation:

mapping_store()
  alloc_dev_dax_range()
    krealloc()

Since this iteration in fsdev_pfn_to_offset() does not hold a synchronization
lock like dax_dev_rwsem, a concurrent krealloc() could free the array while
it is being walked during a memory failure event. The previous code
locklessly accessed the first index, but the new loop extends the window
where this could occur.

Would it be safer to iterate over the immutable snapshot stored in
pgmap->ranges instead, since that is populated during probe and passed into
the memory failure handler?

> +
> +		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/0100019ea3929225-a0f8e6f7-30ae-4f8e-ae6f-19129666c4c3-000000@email.amazonses.com?part=2