Re: [PATCH V6 06/10] dax/fsdev: use __va(phys) for kaddr in direct_access

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Multi-range dax_direct_access requests fail entirely instead of truncating to the contiguous physical range.
--

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

dax/fsdev: use __va(phys) for kaddr in direct_access
    
This commit changes __fsdev_dax_direct_access() to use __va(phys) instead of
a linear offset to correctly calculate virtual addresses for multi-range
devices. It also removes the now-unused virt_addr field from struct dev_dax.

> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -51,9 +51,7 @@ static long __fsdev_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
>  	struct dev_dax *dev_dax = dax_get_private(dax_dev);
>  	size_t size = nr_pages << PAGE_SHIFT;
>  	size_t offset = pgoff << PAGE_SHIFT;
> -	void *virt_addr = dev_dax->virt_addr + offset;
>  	phys_addr_t phys;
> -	unsigned long local_pfn;
>  
>  	phys = dax_pgoff_to_phys(dev_dax, pgoff, size);
>  	if (phys == -1) {
>  		dev_dbg(&dev_dax->dev,
>  			"pgoff (%#lx) out of range\n", pgoff);
>  		return -EFAULT;
>  	}

[Severity: High]
This is a pre-existing issue, but does the current multi-range logic cause
valid requests to fail outright if they span a physical boundary?

When __fsdev_dax_direct_access() is called with a size that crosses a
physical range gap, dax_pgoff_to_phys() evaluates the full request size.
Because it enforces phys + size - 1 <= range->end, it returns -1 when the
request exceeds the current contiguous range.

Instead of truncating the response length to the contiguous pages available
in the current range (as the direct_access API contract requires),
__fsdev_dax_direct_access() completely rejects the request and returns
-EFAULT.

Could this prevent chunked processing of large requests, such as I/O reads
or huge page faults, that happen to cross a range gap?

Furthermore, the return value at the end of this function appears to ignore
physical boundaries:

return PHYS_PFN(min(size, dev_dax->cached_size - offset));

If dax_pgoff_to_phys() didn't fail on requests spanning gaps, wouldn't this
erroneously report non-contiguous memory as contiguous?

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