linux-kernel - Re: [PATCH v10 01/10] drm/shmem-helper: Simplify page offset calculation in fault handler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e2102c82-6b8f-4f6e-80ea-ee185bb1e52e@collabora.com>
Date: Mon, 1 Dec 2025 19:06:11 +0100
From: Loïc Molinari <loic.molinari@...labora.com>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Jani Nikula <jani.nikula@...ux.intel.com>,
 Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>, Tvrtko Ursulin
 <tursulin@...ulin.net>, Rob Herring <robh@...nel.org>,
 Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
 Melissa Wen <mwen@...lia.com>, Maíra Canal
 <mcanal@...lia.com>, Hugh Dickins <hughd@...gle.com>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Al Viro
 <viro@...iv.linux.org.uk>, Mikołaj Wasiak
 <mikolaj.wasiak@...el.com>, Christian Brauner <brauner@...nel.org>,
 Nitin Gote <nitin.r.gote@...el.com>, Andi Shyti
 <andi.shyti@...ux.intel.com>, Jonathan Corbet <corbet@....net>,
 Christopher Healy <healych@...zon.com>, Matthew Wilcox
 <willy@...radead.org>, Bagas Sanjaya <bagasdotme@...il.com>,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 intel-gfx@...ts.freedesktop.org, linux-mm@...ck.org,
 linux-doc@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH v10 01/10] drm/shmem-helper: Simplify page offset
 calculation in fault handler

Hi Boris,

On 01/12/2025 09:05, Boris Brezillon wrote:
> On Fri, 28 Nov 2025 19:52:43 +0100
> Loïc Molinari <loic.molinari@...labora.com> wrote:
> 
>> For a fault at address addr, the page offset is
>>    page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT
>>                = ((addr & PAGE_MASK) - vma->vm_start) >> PAGE_SHIFT
>> 	      = (addr - vma->vm_start) >> PAGE_SHIFT
>>
>> Since the faulty logical page offset based on VMA is
>>    vmf->pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT)
>>
>> We can slightly simplify the calculation using
>>    page_offset = vmf->pgoff - vma->vm_pgoff
>>
>> Signed-off-by: Loïc Molinari <loic.molinari@...labora.com>
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@...labora.com>
> 
> One nit below
> 
>> ---
>>   drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index dc94a27710e5..be89be1c804c 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -577,8 +577,8 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>>   	struct page *page;
>>   	pgoff_t page_offset;
>>   
>> -	/* We don't use vmf->pgoff since that has the fake offset */
>> -	page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>> +	/* Offset to faulty address in the VMA (without the fake offset). */
> 
> It's weird to say "without the fake offset" here, because IIUC, both
> vmf->pgoff and vma->vm_pgoff contain the fake offset. And that's fine,
> the problem really is when one of the subtraction operand is not
> relative to the fake offset.

Yes, both values contain the fake offset. vma->vm_pgoff is the actual 
fake offset (mmap offset in the GEM context). vmf->pgoff is the fake 
offset added to the offset we're looking for (offset from start of VMA 
to faulty address). So the difference just gets rid of it, hence the 
precision, but now that I read it again after a few weeks, it's a bit 
misleading so I'll just remove it.

Regards,
Loïc

>> +	page_offset = vmf->pgoff - vma->vm_pgoff;
>>   
>>   	dma_resv_lock(shmem->base.resv, NULL);
>>   
>