Re: [PATCH v4 18/19] mm: memfd_luo: Add feature header to struct memfd_luo_folio_ser

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm: memfd_luo: folio leak on kho_ser_validate() failure in
  retrieve/discard
--

commit c3d8dd278c3f1e63ea2c973472e060bae843ec49
Author: Tarun Sahu <tarunsahu@google.com>

mm: memfd_luo: Add feature header to struct memfd_luo_folio_ser

This commit adds a struct kho_ser_hdr feature header to each serialized
memfd folio entry and validates it during retrieve and discard.

> diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c
> index 94510f73a6069..c00daa8a508e1 100644
> --- a/mm/memfd_luo.c
> +++ b/mm/memfd_luo.c

[ ... ]

> @@ -384,16 +391,23 @@ static void memfd_luo_unpreserve(struct liveupdate_file_op_args *args)
>  	inode_unlock(inode);
>  }
>  
> -static void memfd_luo_discard_folios(const struct memfd_luo_folio_ser *folios_ser,
> +static void memfd_luo_discard_folios(const DECLARE_KHO_SER_HDR(memfd_luo_folio_ser, *folios_ser),
>  				     u64 nr_folios)
>  {
> +	const DECLARE_KHO_SER_HDR(memfd_luo_folio_ser, *fhdr) = folios_ser;
>  	u64 i;
>  
> -	for (i = 0; i < nr_folios; i++) {
> -		const struct memfd_luo_folio_ser *pfolio = &folios_ser[i];
> +	for (i = 0; i < nr_folios; i++, fhdr = (const void *)fhdr + fhdr->size) {
> +		const struct memfd_luo_folio_ser *pfolio;
>  		struct folio *folio;
>  		phys_addr_t phys;
>  
> +		if (kho_ser_validate(fhdr, memfd_luo_folio_ser)) {
> +			pr_warn("invalid folio feature header, folios not discarded\n");
> +			return;
> +		}
> +
> +		pfolio = kho_ser_payload(fhdr, memfd_luo_folio_ser);
>  		if (!pfolio->pfn)
>  			continue;

[ ... ]

> @@ -447,9 +461,10 @@ static void memfd_luo_finish(struct liveupdate_file_op_args *args)
>  }
>  
>  static int memfd_luo_retrieve_folios(struct file *file,
> -				     struct memfd_luo_folio_ser *folios_ser,
> +				     const DECLARE_KHO_SER_HDR(memfd_luo_folio_ser, *folios_ser),
>  				     u64 nr_folios)
>  {
> +	const DECLARE_KHO_SER_HDR(memfd_luo_folio_ser, *fhdr) = folios_ser;
>  	struct inode *inode = file_inode(file);
>  	struct address_space *mapping = inode->i_mapping;
>  	struct folio *folio;
> @@ -457,12 +472,20 @@ static int memfd_luo_retrieve_folios(struct file *file,
>  	int err = -EIO;
>  	long i;
>  
> -	for (i = 0; i < nr_folios; i++) {
> -		const struct memfd_luo_folio_ser *pfolio = &folios_ser[i];
> +	for (i = 0; i < nr_folios; i++, fhdr = (const void *)fhdr + fhdr->size) {
> +		const struct memfd_luo_folio_ser *pfolio;
>  		phys_addr_t phys;
>  		u64 index;
>  		int flags;
>  
> +		err = kho_ser_validate(fhdr, memfd_luo_folio_ser);
> +		if (err) {
> +			pr_err("memfd_luo_folio_ser feature validation failed: %pe\n",
> +			       ERR_PTR(err));
> +			goto put_folios;
> +		}
> +
> +		pfolio = kho_ser_payload(fhdr, memfd_luo_folio_ser);
>  		if (!pfolio->pfn)
>  			continue;
>  
> @@ -472,7 +495,7 @@ static int memfd_luo_retrieve_folios(struct file *file,
>  			pr_err("Unable to restore folio at physical address: %llx\n",
>  			       phys);
>  			err = -EIO;
> -			goto put_folios;
> +			goto next_put_folios;
>  		}
>  		index = pfolio->index;
>  		flags = pfolio->flags;
> @@ -524,15 +547,22 @@ static int memfd_luo_retrieve_folios(struct file *file,
>  unlock_folio:
>  	folio_unlock(folio);
>  	folio_put(folio);
> +next_put_folios:
> +	i++;
> +	fhdr = (const void *)fhdr + fhdr->size;
>  put_folios:
>  	/*
>  	 * Note: don't free the folios already added to the file. They will be
>  	 * freed when the file is freed. Free the ones not added yet here.
>  	 */
> -	for (long j = i + 1; j < nr_folios; j++) {
> -		const struct memfd_luo_folio_ser *pfolio = &folios_ser[j];
> +	for (; i < nr_folios; i++, fhdr = (const void *)fhdr + fhdr->size) {
> +		const struct memfd_luo_folio_ser *pfolio;
>  		phys_addr_t phys;
>  
> +		if (kho_ser_validate(fhdr, memfd_luo_folio_ser))
> +			break;
> +
> +		pfolio = kho_ser_payload(fhdr, memfd_luo_folio_ser);
>  		if (!pfolio->pfn)
>  			continue;

[Severity: Medium]
Does this leak the remaining preserved folios when kho_ser_validate()
fails?

When kho_ser_validate() fails on entry i in memfd_luo_retrieve_folios(),
it jumps to put_folios with fhdr still pointing at entry i.  The cleanup
loop then calls kho_ser_validate(fhdr, memfd_luo_folio_ser) on that same
entry and immediately breaks, leaving entry i and all subsequent entries
un-restored and permanently leaked.

Similarly, memfd_luo_discard_folios() returns on the first validation
failure instead of continuing or still restoring pfolio->pfn across the
array using fhdr->size, which only checks for unknown required feature
bits while the base payload and entry stride remain valid.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110050202.529473-1-tarunsahu@google.com?part=18