linux-ext4 - Re: [PATCH 24/31] fuse: implement inline data file IO via iomap

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260205190206.1890644-1-clm@meta.com>
Date: Thu, 5 Feb 2026 11:01:07 -0800
From: Chris Mason <clm@...a.com>
To: "Darrick J. Wong" <djwong@...nel.org>
CC: <miklos@...redi.hu>, <joannelkoong@...il.com>, <bernd@...ernd.com>,
        <neal@...pa.dev>, <linux-ext4@...r.kernel.org>,
        <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 24/31] fuse: implement inline data file IO via iomap

"Darrick J. Wong" <djwong@...nel.org> wrote:
> From: Darrick J. Wong <djwong@...nel.org>
> 
> Implement inline data file IO by issuing FUSE_READ/FUSE_WRITE commands
> in response to an inline data mapping.

Hi everyone,

I'm trying out my AI review prompts on a few more trees, and I ran it
on the fuse-iomap-cache branch:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-cache

This was the latest email lore could find, apologies if I grabbed the wrong one.

Hopefully the review below is useful, please let me know if there are places it
has gone wildly wrong.  I kept the header to make it obvious exactly which
commit I'd run through:

commit 1263eae0852acb1359182377ad94d570701e2274
Author: Darrick J. Wong <djwong@...nel.org>

fuse: implement inline data file IO via iomap

Implement inline data file IO by issuing FUSE_READ/FUSE_WRITE commands
in response to an inline data mapping.

> diff --git a/fs/fuse/fuse_iomap.c b/fs/fuse/fuse_iomap.c
> --- a/fs/fuse/fuse_iomap.c
> +++ b/fs/fuse/fuse_iomap.c
> @@ -398,6 +398,150 @@ fuse_iomap_find_dev(struct fuse_conn *fc, const struct fuse_iomap_io *map)
>  	return ret;
>  }
>
> +/*
> + * Use the FUSE_READ command to read inline file data from the fuse server.
> + * Note that there's no file handle attached, so the fuse server must be able
> + * to reconnect to the inode via the nodeid.
> + */
> +static int fuse_iomap_inline_read(struct inode *inode, loff_t pos,
> +				  loff_t count, struct iomap *iomap)
> +{
> +	struct fuse_read_in in = {
> +		.offset = pos,
> +		.size = count,
> +	};
> +	struct fuse_inode *fi = get_fuse_inode(inode);
> +	struct fuse_mount *fm = get_fuse_mount(inode);
> +	FUSE_ARGS(args);
> +	ssize_t ret;
> +
> +	if (BAD_DATA(!iomap_inline_data_valid(iomap)))
> +		return -EFSCORRUPTED;

Does this code leak memory when the BAD_DATA check fails?  Looking at
the call chain:

fuse_iomap_set_inline()->fuse_iomap_inline_alloc() allocates
iomap->inline_data via kvzalloc.  If the iomap_inline_data_valid check
fails (i.e., iomap->length > PAGE_SIZE - offset_in_page(iomap->inline_data)),
the function returns -EFSCORRUPTED without freeing iomap->inline_data.

Other error paths in this function (around the fuse_simple_request call)
properly call fuse_iomap_inline_free(iomap) before returning errors.

The caller fuse_iomap_set_inline doesn't clean up iomap->inline_data on
error in the read path (line 538: 'if (err) return err;'), so the
allocation is leaked when a FUSE server returns an IOMAP_INLINE mapping
with length > PAGE_SIZE.

> +
> +	args.opcode = FUSE_READ;
> +	args.nodeid = fi->nodeid;
> +	args.in_numargs = 1;
> +	args.in_args[0].size = sizeof(in);
> +	args.in_args[0].value = &in;
> +	args.out_argvar = true;
> +	args.out_numargs = 1;
> +	args.out_args[0].size = count;
> +	args.out_args[0].value = iomap_inline_data(iomap, pos);
> +
> +	ret = fuse_simple_request(fm, &args);
> +	if (ret < 0) {
> +		fuse_iomap_inline_free(iomap);
> +		return ret;
> +	}
> +	/* no readahead means something bad happened */
> +	if (ret == 0) {
> +		fuse_iomap_inline_free(iomap);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}

[ ... ]

> @@ -511,8 +663,28 @@ static int fuse_iomap_end(struct inode *inode, loff_t pos, loff_t count,
>  {
>  	struct fuse_inode *fi = get_fuse_inode(inode);
>  	struct fuse_mount *fm = get_fuse_mount(inode);
> +	struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap);
> +	struct iomap *srcmap = &iter->srcmap;

Can this container_of usage corrupt memory?  Looking at
fuse_iomap_writeback_range, it calls fuse_iomap_end with &write_iomap,
which is a stack-local struct iomap variable (not embedded in an
iomap_iter):

fuse_iomap_writeback_range()
    struct iomap write_iomap = { };
    ...
    fuse_iomap_end(inode, offset, len, 0,
                   FUSE_IOMAP_OP_WRITEBACK, &write_iomap);

The container_of macro computes an invalid pointer by subtracting the
offset of the iomap member from the address, resulting in iter pointing
to garbage memory on the stack.  Subsequently, accessing iter->srcmap
reads from invalid memory, potentially causing undefined behavior or
crashes.

The iomap core calls fuse_iomap_end via fuse_iomap_ops where iomap IS
properly embedded in iomap_iter, but the direct call from
fuse_iomap_writeback_range violates this assumption.