linux-kernel - Re: [PATCH v6 35/43] btrfs: make btrfs_ref_to

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260208150339.3021113-1-clm@meta.com>
Date: Sun, 8 Feb 2026 07:02:41 -0800
From: Chris Mason <clm@...a.com>
To: Daniel Vacek <neelx@...e.com>
CC: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        Eric Biggers
	<ebiggers@...nel.org>,
        "Theodore Y. Ts'o" <tytso@....edu>, Jaegeuk Kim
	<jaegeuk@...nel.org>,
        Jens Axboe <axboe@...nel.dk>, David Sterba
	<dsterba@...e.com>,
        <linux-block@...r.kernel.org>, <linux-fscrypt@...r.kernel.org>,
        <linux-btrfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 35/43] btrfs: make btrfs_ref_to_path handle encrypted filenames

Daniel Vacek <neelx@...e.com> wrote:
> From: Josef Bacik <josef@...icpanda.com>
> 
> We use this helper for inode-resolve and path resolution in send, so
> update this helper to properly decrypt any encrypted names it finds.
> 

Hi everyone,

I'm running the btrfs for-next branch, along with the encryption patch set
through my AI review prompts:

https://github.com/masoncl/review-prompts

I'm working on some btrfs specific instructions, but hopefully these reviews
are already useful.  I've kept the headers to make it easier to double check
that I'm actually replying to the right email, but please let me know if any
parts of this are incorrect:

commit 473bbb12052f731a4e2ce3c48e7d7e9f2510528b
Author: Josef Bacik <josef@...icpanda.com>

btrfs: make btrfs_ref_to_path handle encrypted filenames

This commit updates btrfs_ref_to_path() to handle encrypted filenames by
introducing a new copy_resolved_iref_to_buf() helper that can decrypt
names when the filesystem has encryption enabled.

> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 9bb406f7dd302..577c3ef87791a 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c

[ ... ]

> @@ -2107,6 +2108,39 @@ int btrfs_find_one_extref(struct btrfs_root *root, u64 inode_objectid,
>  	return ret;
>  }
>
> +static int copy_resolved_iref_to_buf(struct btrfs_root *fs_root,
> +				     struct extent_buffer *eb, char *dest,
> +				     u64 parent, unsigned long name_off,
> +				     u32 name_len, s64 *bytes_left)
> +{
> +	struct btrfs_fs_info *fs_info = fs_root->fs_info;
> +	struct fscrypt_str fname = FSTR_INIT(NULL, 0);
> +	int ret;
> +
> +	/* No encryption, just copy the name in. */
> +	if (!btrfs_fs_incompat(fs_info, ENCRYPT)) {
> +		*bytes_left -= name_len;
> +		if (*bytes_left >= 0)
> +			read_extent_buffer(eb, dest + *bytes_left, name_off, name_len);
> +		return 0;
> +	}
> +
> +	ret = fscrypt_fname_alloc_buffer(BTRFS_NAME_LEN, &fname);
> +	if (ret)
> +		return ret;
> +
> +	ret = btrfs_decrypt_name(fs_root, eb, name_off, name_len, parent, &fname);
> +	if (ret)
> +		goto out;
> +
> +	*bytes_left -= fname.len;
> +	if (*bytes_left >= 0)
> +		memcpy(dest + *bytes_left, fname.name, fname.len);
> +out:
> +	fscrypt_fname_free_buffer(&fname);
> +	return ret;
> +}
> +
>  /*
>   * this iterates to turn a name (from iref/extref) into a full filesystem path.
>   * Elements of the path are separated by '/' and the path is guaranteed to be
> @@ -2138,10 +2172,10 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root, struct btrfs_path *path,
>  		dest[bytes_left] = '\0';
>
>  	while (1) {
> -		bytes_left -= name_len;
> -		if (bytes_left >= 0)
> -			read_extent_buffer(eb, dest + bytes_left,
> -					   name_off, name_len);
> +		ret = copy_resolved_iref_to_buf(fs_root, eb, dest, parent,
> +						name_off, name_len, &bytes_left);
> +		if (ret)
> +			break;
>  		if (eb != eb_in) {
>  			if (!path->skip_locking)
>  				btrfs_tree_read_unlock(eb);

Does this code leak the extent buffer when copy_resolved_iref_to_buf()
fails? Looking at the loop structure in btrfs_ref_to_path(), when
copy_resolved_iref_to_buf() returns an error and we break at line 2178,
the cleanup code that follows never executes:

    if (eb != eb_in) {
        if (!path->skip_locking)
            btrfs_tree_read_unlock(eb);
        free_extent_buffer(eb);
    }

This can happen in subsequent iterations where eb != eb_in (the extent
buffer was detached from path->nodes[0] and set to NULL in the previous
iteration). The cleanup at the end of the function (btrfs_release_path()
at line 2216) cannot free eb since it's no longer in the path.

This could be triggered when decryption fails in copy_resolved_iref_to_buf()
due to btrfs_iget() error, fscrypt_fname_alloc_buffer() ENOMEM, or
fscrypt_prepare_readdir() failure on an encrypted filesystem.

Before this patch, the name copy operation (read_extent_buffer) could not
fail, so the cleanup always happened before any error check.