[PATCH] iommufd: Fix offset handling in folio subroutines

[PATCH] iommufd: Fix offset handling in folio subroutines

[Alexey Kardashevskiy]

The @offset from memfd_pin_folios() is pgoff_t and is bytes, not
a number of pages as batch_from_folios() treats it.

Convert byte offset to pfn offset and use that.

Fixes: ed9178fbfd4e ("iommufd: Folio subroutines")
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---

Found this when tried mapping a bunch of folios (>64) with an offset and
the first folio was used for all the mappings in the batch.

---
 drivers/iommu/iommufd/pages.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c
index 3427749bc5ce..6075d11691a6 100644
--- a/drivers/iommu/iommufd/pages.c
+++ b/drivers/iommu/iommufd/pages.c
@@ -646,8 +646,9 @@ static int batch_from_folios(struct pfn_batch *batch, struct folio ***folios_p,
 
 	while (npages) {
 		struct folio *folio = *folios;
-		unsigned long nr = folio_nr_pages(folio) - offset;
-		unsigned long pfn = page_to_pfn(folio_page(folio, offset));
+		unsigned long offset_nr = offset >> PAGE_SHIFT;
+		unsigned long nr = folio_nr_pages(folio) - offset_nr;
+		unsigned long pfn = page_to_pfn(folio_page(folio, offset_nr));
 
 		nr = min(nr, npages);
 		npages -= nr;
-- 
2.47.1

RE: [PATCH] iommufd: Fix offset handling in folio subroutines

[Tian, Kevin]

> From: Alexey Kardashevskiy <aik@amd.com>
> Sent: Wednesday, April 2, 2025 9:44 AM
> 
> The @offset from memfd_pin_folios() is pgoff_t and is bytes, not
> a number of pages as batch_from_folios() treats it.
> 
> Convert byte offset to pfn offset and use that.
> 
> Fixes: ed9178fbfd4e ("iommufd: Folio subroutines")
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
> 
> Found this when tried mapping a bunch of folios (>64) with an offset and
> the first folio was used for all the mappings in the batch.
> 
> ---
>  drivers/iommu/iommufd/pages.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/pages.c
> b/drivers/iommu/iommufd/pages.c
> index 3427749bc5ce..6075d11691a6 100644
> --- a/drivers/iommu/iommufd/pages.c
> +++ b/drivers/iommu/iommufd/pages.c
> @@ -646,8 +646,9 @@ static int batch_from_folios(struct pfn_batch *batch,
> struct folio ***folios_p,
> 
>  	while (npages) {
>  		struct folio *folio = *folios;
> -		unsigned long nr = folio_nr_pages(folio) - offset;
> -		unsigned long pfn = page_to_pfn(folio_page(folio, offset));
> +		unsigned long offset_nr = offset >> PAGE_SHIFT;
> +		unsigned long nr = folio_nr_pages(folio) - offset_nr;
> +		unsigned long pfn = page_to_pfn(folio_page(folio, offset_nr));

pin_memfd_pages():
	nfolios = memfd_pin_folios(user->file, start, end, user->ufolios,
				      nfolios, &offset);
	offset >>= PAGE_SHIFT;
	user->ufolios_next = user->ufolios;
	user->ufolios_offset = offset;

pfn_reader_fill_span():
	if (!user->file)
		batch_from_pages(&pfns->batch, user->upages + start_index,
				    npages);
	else
		rc = batch_from_folios(&pfns->batch, &user->ufolios_next,
					&user->ufolios_offset, npages); 

So the offset is already about a page number as batch_from_folios() expects.

> 
>  		nr = min(nr, npages);
>  		npages -= nr;
> --
> 2.47.1

Re: [PATCH] iommufd: Fix offset handling in folio subroutines

[Alexey Kardashevskiy]

On 7/4/25 15:12, Tian, Kevin wrote:
>> From: Alexey Kardashevskiy <aik@amd.com>
>> Sent: Wednesday, April 2, 2025 9:44 AM
>>
>> The @offset from memfd_pin_folios() is pgoff_t and is bytes, not
>> a number of pages as batch_from_folios() treats it.
>>
>> Convert byte offset to pfn offset and use that.
>>
>> Fixes: ed9178fbfd4e ("iommufd: Folio subroutines")
>> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
>> ---
>>
>> Found this when tried mapping a bunch of folios (>64) with an offset and
>> the first folio was used for all the mappings in the batch.
>>
>> ---
>>   drivers/iommu/iommufd/pages.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/iommufd/pages.c
>> b/drivers/iommu/iommufd/pages.c
>> index 3427749bc5ce..6075d11691a6 100644
>> --- a/drivers/iommu/iommufd/pages.c
>> +++ b/drivers/iommu/iommufd/pages.c
>> @@ -646,8 +646,9 @@ static int batch_from_folios(struct pfn_batch *batch,
>> struct folio ***folios_p,
>>
>>   	while (npages) {
>>   		struct folio *folio = *folios;
>> -		unsigned long nr = folio_nr_pages(folio) - offset;
>> -		unsigned long pfn = page_to_pfn(folio_page(folio, offset));
>> +		unsigned long offset_nr = offset >> PAGE_SHIFT;
>> +		unsigned long nr = folio_nr_pages(folio) - offset_nr;
>> +		unsigned long pfn = page_to_pfn(folio_page(folio, offset_nr));
> 
> pin_memfd_pages():
> 	nfolios = memfd_pin_folios(user->file, start, end, user->ufolios,
> 				      nfolios, &offset);
> 	offset >>= PAGE_SHIFT;

Aaaaahhh missed that so there is a bug in my variant of 
memfd_pin_folios() for gmemfd and sorry for the noise. Thanks,


> 	user->ufolios_next = user->ufolios;
> 	user->ufolios_offset = offset;
> 
> pfn_reader_fill_span():
> 	if (!user->file)
> 		batch_from_pages(&pfns->batch, user->upages + start_index,
> 				    npages);
> 	else
> 		rc = batch_from_folios(&pfns->batch, &user->ufolios_next,
> 					&user->ufolios_offset, npages);
> 
> So the offset is already about a page number as batch_from_folios() expects.
> 
>>
>>   		nr = min(nr, npages);
>>   		npages -= nr;
>> --
>> 2.47.1
> 

-- 
Alexey