Re: [PATCH 20/46] mm/gup: fix clearing folios in memfd_pin_folios()

[Pratyush Yadav]

On Tue, Oct 27 2026, David Hildenbrand (Arm) wrote:

> On 10/27/26 16:00, Pratyush Yadav wrote:
>> On Tue, Oct 27 2026, David Hildenbrand (Arm) wrote:
>> 
>>> From: Chris Mason <mason@xxxxxxxxxx>
>>>
>>> memfd_pin_folios() pins folios obtained directly from the page cache
>>> without checking whether they are uptodate. However, shmem fallocate()
>>> can leave newly allocated folios uncleared and not uptodate. Callers
>>> such as udmabuf can then expose uninitialized memory to userspace.
>>>
>>> Clear non-uptodate folios and mark them uptodate under the folio lock
>>> before returning them to the caller.
>>>
>>> Fix generated by an LLM and sent by Chris Mason in private.
>>>
>>> Fixes: 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Signed-off-by: Chris Mason <mason@xxxxxxxxxx>
>>> [ david: simplify description and comment ]
>>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>>> ---
>>>  mm/gup.c | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/mm/gup.c b/mm/gup.c
>>> index efc9aa2654625..47e7ae152cdf1 100644
>>> --- a/mm/gup.c
>>> +++ b/mm/gup.c
>>> @@ -3516,6 +3516,20 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end,
>>>  					goto err;
>>>  				}
>>>  
>>> +				/*
>>> +				 * fallocate() can leave folios in the
>>> +				 * page cache that were never cleared.
>>> +				 */
>>> +				if (!folio_test_uptodate(folio)) {
>>> +					folio_lock(folio);
>>> +					if (!folio_test_uptodate(folio)) {
>>> +						folio_zero_range(folio, 0, folio_size(folio));
>>> +						flush_dcache_folio(folio);
>>> +						folio_mark_uptodate(folio);
>>> +					}
>>> +					folio_unlock(folio);
>>> +				}
>>> +
>> 
>> Nit: this logic is repeated in a couple places in shmem.c and in
>> memfd_luo.c. Perhaps it deserves a helper?
>> 
>> Anyway, sounds like an important fix so don't make my nitpick stop you
>> from landing this if you don't want to bother.
>
> Thanks! yeah, I had the same thought when trying to improve this. We should have
> an easier way to just not hand out unzeroed folios outside of shmem core
> functions in the first place ...

Normally this should use shmem_read_folio() to get all the folios, which
does zero the folios before handing them out. I guess memfd_pin_folios()
uses filemap_get_folios_contig() for performance reasons. It does the
lookup under RCU and doesn't need to take the folio lock. Perhaps we
need a shmem_get_folios_contig()?

-- 
Regards,
Pratyush Yadav