Re: [PATCH v3 12/15] fuse_service: bound argc and arg len read from the args memfd

[Bernd Schubert <bernd@bsbernd.com>]

On 9/30/26 15:11, Bernd Schubert via B4 Relay wrote:
> From: Keerthana KT <keerthana@labs.digiscrypt.com>
> 
> fuse_service_append_args() takes the argument count and each argument
> length straight from the args memfd, which this file already treats as
> untrusted (see the SO_PASSRIGHTS guard against a malicious mount
> helper). Both fields are uint32_t and feed allocation math with no
> bound: calloc(memfd_args.argc + existing_args->argc, ...) wraps in
> unsigned arithmetic and undersizes the argv array, while
> calloc(1, memfd_arg.len + 1) wraps to a zero-size buffer when len is
> UINT32_MAX, which the following pread() then overflows.
> 
> Nothing bounded the memfd itself, so cap it on both sides with a new
> FUSE_SERVICE_MAX_ARGV_SIZE. The mount helper refuses to write a string
> that would push the file past the cap, and the server fstat()s the file
> and refuses to parse one larger than it. The file size then bounds the
> rest: argc cannot exceed the number of iovecs that fit between the
> header and the strings, and no string can be longer than the file
> holding it. An argc of zero is rejected as well, because only the first
> loop iteration assigns argv[0].
> 
> Signed-off-by: Keerthana KT <keerthana@labs.digiscrypt.com>
> Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
> ---
>  include/fuse_service_priv.h | 10 ++++++++++
>  lib/fuse_service.c          | 40 ++++++++++++++++++++++++++++++++++++++++
>  util/mount_service.c        | 10 ++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/include/fuse_service_priv.h b/include/fuse_service_priv.h
> index 988f7c9251c8..84e6948d771c 100644
> --- a/include/fuse_service_priv.h
> +++ b/include/fuse_service_priv.h
> @@ -23,6 +23,16 @@ struct fuse_service_memfd_argv {
>  
>  #define FUSE_SERVICE_MAX_CMD_SIZE	(65536)
>  
> +/*
> + * Upper bound on the whole argv memfd, as opposed to FUSE_SERVICE_MAX_CMD_SIZE
> + * which bounds one socket command.  Both sides check it: the mount helper
> + * refuses to write past it, and the fuse server refuses to parse a file larger
> + * than it.  Generous next to any real mount(8) invocation, but small enough
> + * that the counts and lengths the server reads out of the file cannot overflow
> + * the allocation math they feed.
> + */
> +#define FUSE_SERVICE_MAX_ARGV_SIZE	(sysconf(_SC_ARG_MAX))
> +
>  #define FUSE_SERVICE_ARGS_MAGIC		0x41524753	/* ARGS */
>  
>  /* mount.service sends a hello to the server and it replies */
> diff --git a/lib/fuse_service.c b/lib/fuse_service.c
> index 0a05b3fbc1f2..af08e919eebb 100644
> --- a/lib/fuse_service.c
> +++ b/lib/fuse_service.c
> @@ -629,8 +629,10 @@ int fuse_service_append_args(struct fuse_service *sf,
>  	struct fuse_args new_args = {
>  		.allocated = 1,
>  	};
> +	struct stat statbuf;
>  	char *str = NULL;
>  	off_t memfd_pos = 0;
> +	off_t max_argc;
>  	ssize_t received;
>  	unsigned int i;
>  	int ret;
> @@ -656,6 +658,34 @@ int fuse_service_append_args(struct fuse_service *sf,
>  	memfd_args.argc = htonl(memfd_args.argc);
>  	memfd_pos += sizeof(memfd_args);
>  
> +	ret = fstat(sf->argvfd, &statbuf);
> +	if (ret) {
> +		int error = errno;
> +
> +		fuse_log(FUSE_LOG_ERR, "fuse: service args file stat: %s\n",
> +			 strerror(error));
> +		return -error;
> +	}
> +	if (statbuf.st_size > FUSE_SERVICE_MAX_ARGV_SIZE) {
> +		fuse_log(FUSE_LOG_ERR, "fuse: service args file too large\n");
> +		return -EBADMSG;
> +	}
> +
> +	/*
> +	 * The array of argv iovecs sits between the header and the strings, so
> +	 * the file size bounds argc.  Reject a count the file cannot hold: the
> +	 * sum below is computed in unsigned arithmetic and would otherwise wrap
> +	 * and undersize the array.  argc 0 is rejected as well, because only
> +	 * the first loop iteration fills argv[0].
> +	 */
> +	max_argc = (statbuf.st_size - (off_t)sizeof(memfd_args)) /
> +		   (off_t)sizeof(struct fuse_service_memfd_arg);
> +	if (memfd_args.argc == 0 || memfd_args.argc > max_argc) {
> +		fuse_log(FUSE_LOG_ERR, "fuse: service args file argc %u invalid\n",
> +			 memfd_args.argc);
> +		return -EBADMSG;
> +	}
> +
>  	/* Allocate a new array of argv string pointers */
>  	new_args.argv = calloc(memfd_args.argc + existing_args->argc,
>  			       sizeof(char *));
> @@ -722,6 +752,16 @@ int fuse_service_append_args(struct fuse_service *sf,
>  		memfd_arg.len = htonl(memfd_arg.len);
>  		memfd_pos += sizeof(memfd_arg);
>  
> +		/* memfd_arg sanity check */
> +		if (memfd_arg.pos > (uint64_t)statbuf.st_size ||
> +		    memfd_arg.len >= (uint64_t)statbuf.st_size - memfd_arg.pos) {
> +			fuse_log(FUSE_LOG_ERR,
> +				 "fuse: service args file argv[%u] pos %u len %u out of range\n",
> +				 i, memfd_arg.pos, memfd_arg.len);
> +			ret = -EBADMSG;
> +			goto out_new_args;
> +		}
> +

I should first let tests complete and then post it, tests found an off-by-one issue, due to ">=" instead
of ">". Corrected in the libfuse PR with 

diff --git a/lib/fuse_service.c b/lib/fuse_service.c
index af08e919eebb..05aeb46d17fa 100644
--- a/lib/fuse_service.c
+++ b/lib/fuse_service.c
@@ -754,10 +754,10 @@ int fuse_service_append_args(struct fuse_service *sf,
 
                /* memfd_arg sanity check */
                if (memfd_arg.pos > (uint64_t)statbuf.st_size ||
-                   memfd_arg.len >= (uint64_t)statbuf.st_size - memfd_arg.pos) {
+                   memfd_arg.len > (uint64_t)statbuf.st_size - memfd_arg.pos) {
                        fuse_log(FUSE_LOG_ERR,
-                                "fuse: service args file argv[%u] pos %u len %u out of range\n",
-                                i, memfd_arg.pos, memfd_arg.len);
+                                "fuse: service args file argv[%u] pos %u len %u file size %jd out of range\n",
+                                i, memfd_arg.pos, memfd_arg.len, (intmax_t)statbuf.st_size);
                        ret = -EBADMSG;
                        goto out_new_args;




>  		/* read arg string from file */
>  		str = calloc(1, memfd_arg.len + 1);
>  		if (!str) {
> diff --git a/util/mount_service.c b/util/mount_service.c
> index 84e9d831ce03..18e0481a07d2 100644
> --- a/util/mount_service.c
> +++ b/util/mount_service.c
> @@ -442,6 +442,16 @@ static int mount_service_capture_arg(const struct mount_service *mo,
>  	};
>  	ssize_t written;
>  
> +	/*
> +	 * string_pos already covers the header and the whole array, so this
> +	 * bounds the entire memfd.  The server rejects anything larger.
> +	 */
> +	if (*string_pos + (off_t)string_len > FUSE_SERVICE_MAX_ARGV_SIZE) {
> +		fprintf(stderr, "%s: memfd argv[%u] exceeds %ld byte limit\n",
> +			mo->msgtag, args->argc, (long)FUSE_SERVICE_MAX_ARGV_SIZE);
> +		return -1;
> +	}
> +
>  	written = pwrite(mo->argvfd, string, string_len, *string_pos);
>  	if (written < 0) {
>  		fprintf(stderr, "%s: memfd argv write: %s\n",
>