Re: [PATCH] btrfs: handle highmem folios in read_key_bytes

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/8/17 11:50, Hongling Zeng 写道:
> On 32-bit systems with highmem, folio_address() can return NULL for
> unmapped highmem folios. When this NULL is passed as the dest parameter
> to read_key_bytes() with a non-NULL dest_folio, it violates the function's
> contract (which requires dest to be non-NULL when dest_folio is provided).

But metadata folios are not allocated by page cache, but by btrfs 
itself, which always use GFP_NOFS then attach the folio to page cache, 
thus should not get highmem memory in the first place.

And there is no way to trigger read from userspace on btree inode, so 
there should be no highmem folios from the beginning.

> 
> The original bug had two symptoms:
> 1. Unsigned len underflow when len -= copy_bytes executes (infinite loop)
> 2. The folio remains uninitialized because the copy block is skipped

So did you really hit the problem in the real world?

> 
> Fix requires two changes:
> 
> 1. Change "if (!dest)" to "if (!dest && !dest_folio)"
>     - Prevents the "counting-only" mode when dest_folio is provided
>     - Fixes the underflow/infinite loop
> 
> 2. Change "if (dest)" to "if (dest || dest_folio)"
>     - Ensures the copy block executes when dest_folio is provided
>     - Allows kmap_local_folio() to properly map the highmem folio
>     - Actually writes data to the folio
> 
> Without the second change, the highmem folio is not populated even
> though the read succeeds, causing subsequent fs-verity verification to
> operate on stale or uninitialized data.
> 
> Fixes: 884937793db5 ("btrfs: convert read_key_bytes() to take a folio")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Missing LLM disclosure.
> ---
>   fs/btrfs/verity.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 983365a73541..80bc945c7dcb 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -351,7 +351,7 @@ static int read_key_bytes(struct btrfs_inode *inode, u8 key_type, u64 offset,
>   		}
>   
>   		/* desc = NULL to just sum all the item lengths */
> -		if (!dest)
> +		if (!dest && !dest_folio)
>   			copy_end = item_end;
>   		else
>   			copy_end = min(offset + len, item_end);
> @@ -362,7 +362,7 @@ static int read_key_bytes(struct btrfs_inode *inode, u8 key_type, u64 offset,
>   		/* Offset from the start of item for copying */
>   		copy_offset = offset - key.offset;
>   
> -		if (dest) {
> +		if (dest || dest_folio) {
>   			if (dest_folio)
>   				kaddr = kmap_local_folio(dest_folio, 0);
>