Re: [PATCH 2/2] mm/memfd_luo: keep memfd frozen until finish

[Pratyush Yadav <pratyush@kernel.org>]

On Tue, Nov 03 2026, Pasha Tatashin wrote:

>> After a memfd is retrieved, it immediately goes back into "normal"
>> operation. The file comes back unfrozen, and its pages can migrate, it
>> can be truncated, and so on. If the memfd is depended on by something,
>> like iommufd, that can open a window for folios to move around before
>> the consumer can re-establish its references.
>> 
>> Finish is the conceptual point when a retrieved file can start to behave
>> as "normal". So take an extra reference on the folios and freeze the
>> memfd on retrieve until finish.
>> 
>> Suggested-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
>> Link: https://patch.msgid.link/20261103181916.2462825-3-pratyush@kernel.org
>>
>> diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
>> index 6b68220275eb..5abb9c0956af 100644
>> --- a/mm/memfd_luo.c
>> +++ b/mm/memfd_luo.c
>> @@ -55,6 +55,14 @@
>>   *   Only seals known to this LUO version (see ``MEMFD_LUO_ALL_SEALS``) may
>>   *   be present; preservation fails with ``-EOPNOTSUPP`` otherwise.
>>   *
>> + * Restrictions
>> + * ============
>> + *
>> + * While a memfd is preserved, and after it is retrieved until the session is
>> + * finished, its size can't be changed and holes can't be punched in it.
>> + * Operations that would do so (``ftruncate()``, ``fallocate()``, writes past
>> + * the end of the file) fail.
>> + *
>>   * Non-Preserved Properties
>>   * ========================
>>   *
>> @@ -383,33 +391,56 @@ static void memfd_luo_discard_folios(const struct memfd_luo_folio_ser *folios_se
>>  	}
>>  }
>>  
>> +/* Drop the references retrieve() holds on the folios added to the file. */
>> +static void
>> +memfd_luo_put_retrieved_folios(const struct memfd_luo_folio_ser *folios_ser,
>> +			       u64 nr_folios)
>> +{
>> +	for (u64 i = 0; i < nr_folios; i++) {
>> +		const struct memfd_luo_folio_ser *pfolio = &folios_ser[i];
>> +
>> +		if (pfolio->pfn)
>> +			folio_put(pfn_folio(pfolio->pfn));
>> +	}
>> +}
>> +
>>  static void memfd_luo_finish(struct liveupdate_file_op_args *args)
>>  {
>>  	struct memfd_luo_folio_ser *folios_ser;
>>  	struct memfd_luo_ser *ser;
>>  
>> -	/*
>> -	 * If retrieve was successful, nothing to do. If it failed, retrieve()
>> -	 * already cleaned up everything it could. So nothing to do there
>> -	 * either. Only need to clean up when retrieve was not called.
>> -	 */
>> -	if (args->retrieve_status)
>> +	/* If retrieve failed, it already cleaned up everything it could. */
>> +	if (args->retrieve_status < 0)
>>  		return;
>>  
>>  	ser = phys_to_virt(args->serialized_data);
>>  	if (!ser)
>>  		return;
>>  
>> -	if (ser->nr_folios) {
>> -		folios_ser = kho_restore_vmalloc(&ser->folios);
>> -		if (!folios_ser)
>> -			goto out;
>> +	if (args->retrieve_status > 0) {
>> +		/*
>> +		 * Retrieve was successful, release the folios and unfreeze the
>> +		 * file so it can be used normally.
>> +		 */
>> +		struct inode *inode = file_inode(args->file);
>> +
>> +		folios_ser = args->private_data;
>> +
>> +		inode_lock(inode);
>> +		memfd_luo_put_retrieved_folios(folios_ser, ser->nr_folios);
>> +		shmem_freeze(inode, false);
>> +		inode_unlock(inode);
>>  
>> -		memfd_luo_discard_folios(folios_ser, ser->nr_folios);
>>  		vfree(folios_ser);
>> +	} else if (args->retrieve_status == 0 && ser->nr_folios) {
>> +		/* Retrieve was not called, discard the preserved folios. */
>> +		folios_ser = kho_restore_vmalloc(&ser->folios);
>> +		if (folios_ser) {
>> +			memfd_luo_discard_folios(folios_ser, ser->nr_folios);
>> +			vfree(folios_ser);
>> +		}
>
> Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com> 
>
> Side note (pre-existing, not an issue with this patch):
>
> Initially, I wondered whether we should disallow auto-finishing an 
> unretrieved memfd, in case a preserved device still had this memory 
> mapped for DMA. However, if we reach this point, can_finish() has 
> already passed for every other FD in this session, which means any 
> possibly dependent FDs (e.g. iommufd/vfio) have already determined that 
> this FD was not needed to be retrieved for their states.

I think that is still a problem in a different path.
memfd_luo_retrieve_folios() frees all the folios on error. I think it
should hold them until finish(), which would give IOMMU or other
dependents a chance to block the finish.

But yeah, that would be a separate patch.

-- 
Regards,
Pratyush Yadav