[PATCH v2 0/3] fuse copy_file_range() fixes

[PATCH v2 0/3] fuse copy_file_range() fixes

[Miklos Szeredi]

Fix 32 bit overflow issues.

v2:
  - check for return value not exceeding requested size [1/3]
  - split patch: limit copy size for old API [2/3], add new API [3/3]

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
Miklos Szeredi (3):
  fuse: check if copy_file_range() returns larger than requested size
  fuse: prevent overflow in copy_file_range return value
  fuse: add COPY_FILE_RANGE_64 that allows large copies

 fs/fuse/file.c            | 39 +++++++++++++++++++++++++++++++--------
 fs/fuse/fuse_i.h          |  3 +++
 include/uapi/linux/fuse.h | 12 +++++++++++-
 3 files changed, 45 insertions(+), 9 deletions(-)

-- 
2.49.0

[PATCH v2 1/3] fuse: check if copy_file_range() returns larger than requested size

[Miklos Szeredi]

Just like write(), copy_file_range() should check if the return value is
less or equal to the requested number of bytes.

Reported-by: Chunsheng Luo <luochunsheng@ustc.edu>
Closes: https://lore.kernel.org/all/20250807062425.694-1-luochunsheng@ustc.edu/
Fixes: 88bc7d5097a1 ("fuse: add support for copy_file_range()")
Cc: <stable@vger.kernel.org> # v4.20
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/file.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 5525a4520b0f..45207a6bb85f 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3026,6 +3026,9 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
 		fc->no_copy_file_range = 1;
 		err = -EOPNOTSUPP;
 	}
+	if (!err && outarg.size > len)
+		err = -EIO;
+
 	if (err)
 		goto out;
 
-- 
2.49.0

[PATCH v2 2/3] fuse: prevent overflow in copy_file_range return value

[Miklos Szeredi]

The FUSE protocol uses struct fuse_write_out to convey the return value of
copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
interface supports a 64-bit size copies.

Currently the number of bytes copied is silently truncated to 32-bit, which
may result in poor performance or even failure to copy in case of
truncation to zero.

Reported-by: Florian Weimer <fweimer@redhat.com>
Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@oldenburg.str.redhat.com/
Fixes: 88bc7d5097a1 ("fuse: add support for copy_file_range()")
Cc: <stable@vger.kernel.org> # v4.20
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 45207a6bb85f..4adcf09d4b01 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2960,7 +2960,7 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
 		.nodeid_out = ff_out->nodeid,
 		.fh_out = ff_out->fh,
 		.off_out = pos_out,
-		.len = len,
+		.len = min_t(size_t, len, UINT_MAX & PAGE_MASK),
 		.flags = flags
 	};
 	struct fuse_write_out outarg;
-- 
2.49.0

[PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Miklos Szeredi]

The FUSE protocol uses struct fuse_write_out to convey the return value of
copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
interface supports a 64-bit size copies and there's no reason why copies
should be limited to 32-bit.

Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
number of bytes copied is returned in a 64-bit value.

If the fuse server does not support COPY_FILE_RANGE_64, fall back to
COPY_FILE_RANGE.

Reported-by: Florian Weimer <fweimer@redhat.com>
Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@oldenburg.str.redhat.com/
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/file.c            | 44 ++++++++++++++++++++++++++++-----------
 fs/fuse/fuse_i.h          |  3 +++
 include/uapi/linux/fuse.h | 12 ++++++++++-
 3 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 4adcf09d4b01..867b5fde1237 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2960,10 +2960,12 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
 		.nodeid_out = ff_out->nodeid,
 		.fh_out = ff_out->fh,
 		.off_out = pos_out,
-		.len = min_t(size_t, len, UINT_MAX & PAGE_MASK),
+		.len = len,
 		.flags = flags
 	};
 	struct fuse_write_out outarg;
+	struct fuse_copy_file_range_out outarg_64;
+	u64 bytes_copied;
 	ssize_t err;
 	/* mark unstable when write-back is not used, and file_out gets
 	 * extended */
@@ -3013,33 +3015,51 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
 	if (is_unstable)
 		set_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
 
-	args.opcode = FUSE_COPY_FILE_RANGE;
+	args.opcode = FUSE_COPY_FILE_RANGE_64;
 	args.nodeid = ff_in->nodeid;
 	args.in_numargs = 1;
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
 	args.out_numargs = 1;
-	args.out_args[0].size = sizeof(outarg);
-	args.out_args[0].value = &outarg;
+	args.out_args[0].size = sizeof(outarg_64);
+	args.out_args[0].value = &outarg_64;
+	if (fc->no_copy_file_range_64) {
+fallback:
+		/* Fall back to old op that can't handle large copy length */
+		args.opcode = FUSE_COPY_FILE_RANGE;
+		args.out_args[0].size = sizeof(outarg);
+		args.out_args[0].value = &outarg;
+		inarg.len = len = min_t(size_t, len, UINT_MAX & PAGE_MASK);
+	}
 	err = fuse_simple_request(fm, &args);
 	if (err == -ENOSYS) {
-		fc->no_copy_file_range = 1;
-		err = -EOPNOTSUPP;
+		if (fc->no_copy_file_range_64) {
+			fc->no_copy_file_range = 1;
+			err = -EOPNOTSUPP;
+		} else {
+			fc->no_copy_file_range_64 = 1;
+			goto fallback;
+		}
 	}
-	if (!err && outarg.size > len)
-		err = -EIO;
-
 	if (err)
 		goto out;
 
+	bytes_copied = fc->no_copy_file_range_64 ?
+		outarg.size : outarg_64.bytes_copied;
+
+	if (bytes_copied > len) {
+		err = -EIO;
+		goto out;
+	}
+
 	truncate_inode_pages_range(inode_out->i_mapping,
 				   ALIGN_DOWN(pos_out, PAGE_SIZE),
-				   ALIGN(pos_out + outarg.size, PAGE_SIZE) - 1);
+				   ALIGN(pos_out + bytes_copied, PAGE_SIZE) - 1);
 
 	file_update_time(file_out);
-	fuse_write_update_attr(inode_out, pos_out + outarg.size, outarg.size);
+	fuse_write_update_attr(inode_out, pos_out + bytes_copied, bytes_copied);
 
-	err = outarg.size;
+	err = bytes_copied;
 out:
 	if (is_unstable)
 		clear_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ec248d13c8bf..42f0aff83ce0 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -850,6 +850,9 @@ struct fuse_conn {
 	/** Does the filesystem support copy_file_range? */
 	unsigned no_copy_file_range:1;
 
+	/** Does the filesystem support copy_file_range_64? */
+	unsigned no_copy_file_range_64:1;
+
 	/* Send DESTROY request */
 	unsigned int destroy:1;
 
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 122d6586e8d4..94621f68a5cc 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -235,6 +235,10 @@
  *
  *  7.44
  *  - add FUSE_NOTIFY_INC_EPOCH
+ *
+ *  7.45
+ *  - add FUSE_COPY_FILE_RANGE_64
+ *  - add struct fuse_copy_file_range_out
  */
 
 #ifndef _LINUX_FUSE_H
@@ -270,7 +274,7 @@
 #define FUSE_KERNEL_VERSION 7
 
 /** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 44
+#define FUSE_KERNEL_MINOR_VERSION 45
 
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
@@ -657,6 +661,7 @@ enum fuse_opcode {
 	FUSE_SYNCFS		= 50,
 	FUSE_TMPFILE		= 51,
 	FUSE_STATX		= 52,
+	FUSE_COPY_FILE_RANGE_64	= 53,
 
 	/* CUSE specific operations */
 	CUSE_INIT		= 4096,
@@ -1148,6 +1153,11 @@ struct fuse_copy_file_range_in {
 	uint64_t	flags;
 };
 
+/* For FUSE_COPY_FILE_RANGE_64 */
+struct fuse_copy_file_range_out {
+	uint64_t	bytes_copied;
+};
+
 #define FUSE_SETUPMAPPING_FLAG_WRITE (1ull << 0)
 #define FUSE_SETUPMAPPING_FLAG_READ (1ull << 1)
 struct fuse_setupmapping_in {
-- 
2.49.0

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Joanne Koong]

On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> The FUSE protocol uses struct fuse_write_out to convey the return value of
> copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> interface supports a 64-bit size copies and there's no reason why copies
> should be limited to 32-bit.
>
> Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> number of bytes copied is returned in a 64-bit value.
>
> If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> COPY_FILE_RANGE.

Is it unacceptable to add a union in struct fuse_write_out that
accepts a uint64_t bytes_copied?
struct fuse_write_out {
    union {
        struct {
            uint32_t size;
            uint32_t padding;
        };
        uint64_t bytes_copied;
    };
};

Maybe a little ugly but that seems backwards-compatible to me and
would prevent needing a new FUSE_COPY_FILE_RANGE64.

>
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@oldenburg.str.redhat.com/
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/file.c            | 44 ++++++++++++++++++++++++++++-----------
>  fs/fuse/fuse_i.h          |  3 +++
>  include/uapi/linux/fuse.h | 12 ++++++++++-
>  3 files changed, 46 insertions(+), 13 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 4adcf09d4b01..867b5fde1237 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -3013,33 +3015,51 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
>         if (is_unstable)
>                 set_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
>
> -       args.opcode = FUSE_COPY_FILE_RANGE;
> +       args.opcode = FUSE_COPY_FILE_RANGE_64;
>         args.nodeid = ff_in->nodeid;
>         args.in_numargs = 1;
>         args.in_args[0].size = sizeof(inarg);
>         args.in_args[0].value = &inarg;
>         args.out_numargs = 1;
> -       args.out_args[0].size = sizeof(outarg);
> -       args.out_args[0].value = &outarg;
> +       args.out_args[0].size = sizeof(outarg_64);
> +       args.out_args[0].value = &outarg_64;
> +       if (fc->no_copy_file_range_64) {
> +fallback:
> +               /* Fall back to old op that can't handle large copy length */
> +               args.opcode = FUSE_COPY_FILE_RANGE;
> +               args.out_args[0].size = sizeof(outarg);
> +               args.out_args[0].value = &outarg;
> +               inarg.len = len = min_t(size_t, len, UINT_MAX & PAGE_MASK);
> +       }
>         err = fuse_simple_request(fm, &args);
>         if (err == -ENOSYS) {
> -               fc->no_copy_file_range = 1;
> -               err = -EOPNOTSUPP;
> +               if (fc->no_copy_file_range_64) {

Maybe clearer here to do the if check on the args.opcode? Then this
could just be
if (args.opcode == FUSE_COPY_FILE_RANGE) {

which imo is a lot easier to follow.

> +                       fc->no_copy_file_range = 1;
> +                       err = -EOPNOTSUPP;
> +               } else {
> +                       fc->no_copy_file_range_64 = 1;
> +                       goto fallback;
> +               }
>         }
> -       if (!err && outarg.size > len)
> -               err = -EIO;
> -
>         if (err)
>                 goto out;
>
> +       bytes_copied = fc->no_copy_file_range_64 ?
> +               outarg.size : outarg_64.bytes_copied;
> +
> +       if (bytes_copied > len) {
> +               err = -EIO;
> +               goto out;
> +       }
> +
>         truncate_inode_pages_range(inode_out->i_mapping,
>                                    ALIGN_DOWN(pos_out, PAGE_SIZE),
> -                                  ALIGN(pos_out + outarg.size, PAGE_SIZE) - 1);
> +                                  ALIGN(pos_out + bytes_copied, PAGE_SIZE) - 1);
>
>         file_update_time(file_out);
> -       fuse_write_update_attr(inode_out, pos_out + outarg.size, outarg.size);
> +       fuse_write_update_attr(inode_out, pos_out + bytes_copied, bytes_copied);
>
> -       err = outarg.size;
> +       err = bytes_copied;
>  out:
>         if (is_unstable)
>                 clear_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 122d6586e8d4..94621f68a5cc 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -1148,6 +1153,11 @@ struct fuse_copy_file_range_in {
>         uint64_t        flags;
>  };
>
> +/* For FUSE_COPY_FILE_RANGE_64 */
> +struct fuse_copy_file_range_out {

imo having the 64 in the struct name more explicitly makes it clearer
to the server which one they're supposed to use, eg struct
fuse_copy_file_range64_out

Thanks,
Joanne
> +       uint64_t        bytes_copied;
> +};
> +

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Miklos Szeredi]

On Wed, 13 Aug 2025 at 19:09, Joanne Koong <joannelkoong@gmail.com> wrote:

> Is it unacceptable to add a union in struct fuse_write_out that
> accepts a uint64_t bytes_copied?
> struct fuse_write_out {
>     union {
>         struct {
>             uint32_t size;
>             uint32_t padding;
>         };
>         uint64_t bytes_copied;
>     };
> };
>
> Maybe a little ugly but that seems backwards-compatible to me and
> would prevent needing a new FUSE_COPY_FILE_RANGE64.

It would still require a feature flag and negotiation via FUSE_INIT.
I find adding a new op cleaner.


> > -               err = -EOPNOTSUPP;
> > +               if (fc->no_copy_file_range_64) {
>
> Maybe clearer here to do the if check on the args.opcode? Then this
> could just be
> if (args.opcode == FUSE_COPY_FILE_RANGE) {

Okay.

> > +/* For FUSE_COPY_FILE_RANGE_64 */
> > +struct fuse_copy_file_range_out {
>
> imo having the 64 in the struct name more explicitly makes it clearer
> to the server which one they're supposed to use, eg struct
> fuse_copy_file_range64_out

Makes sense.

Thanks,
Miklos

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Florian Weimer]

* Joanne Koong:

> On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>>
>> The FUSE protocol uses struct fuse_write_out to convey the return value of
>> copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
>> interface supports a 64-bit size copies and there's no reason why copies
>> should be limited to 32-bit.
>>
>> Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
>> number of bytes copied is returned in a 64-bit value.
>>
>> If the fuse server does not support COPY_FILE_RANGE_64, fall back to
>> COPY_FILE_RANGE.
>
> Is it unacceptable to add a union in struct fuse_write_out that
> accepts a uint64_t bytes_copied?
> struct fuse_write_out {
>     union {
>         struct {
>             uint32_t size;
>             uint32_t padding;
>         };
>         uint64_t bytes_copied;
>     };
> };
>
> Maybe a little ugly but that seems backwards-compatible to me and
> would prevent needing a new FUSE_COPY_FILE_RANGE64.

Even with a capability flag, it encourages the presence of bugs that
manifest only on big-endian systems.

Thanks,
Florian

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Joanne Koong]

On Wed, Aug 13, 2025 at 12:21 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Joanne Koong:
>
> > On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >>
> >> The FUSE protocol uses struct fuse_write_out to convey the return value of
> >> copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> >> interface supports a 64-bit size copies and there's no reason why copies
> >> should be limited to 32-bit.
> >>
> >> Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> >> number of bytes copied is returned in a 64-bit value.
> >>
> >> If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> >> COPY_FILE_RANGE.
> >
> > Is it unacceptable to add a union in struct fuse_write_out that
> > accepts a uint64_t bytes_copied?
> > struct fuse_write_out {
> >     union {
> >         struct {
> >             uint32_t size;
> >             uint32_t padding;
> >         };
> >         uint64_t bytes_copied;
> >     };
> > };
> >
> > Maybe a little ugly but that seems backwards-compatible to me and
> > would prevent needing a new FUSE_COPY_FILE_RANGE64.
>
> Even with a capability flag, it encourages the presence of bugs that
> manifest only on big-endian systems.
>

Interesting, can you explain how? size would always be accessed
directly through write_out->size (instead of extracted from the upper
32 bits of bytes_copied), so wouldn't the compiler handle the correct
memory access?


Thanks,
Joanne

> Thanks,
> Florian
>

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Florian Weimer]

* Joanne Koong:

> On Wed, Aug 13, 2025 at 12:21 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Joanne Koong:
>>
>> > On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>> >>
>> >> The FUSE protocol uses struct fuse_write_out to convey the return value of
>> >> copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
>> >> interface supports a 64-bit size copies and there's no reason why copies
>> >> should be limited to 32-bit.
>> >>
>> >> Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
>> >> number of bytes copied is returned in a 64-bit value.
>> >>
>> >> If the fuse server does not support COPY_FILE_RANGE_64, fall back to
>> >> COPY_FILE_RANGE.
>> >
>> > Is it unacceptable to add a union in struct fuse_write_out that
>> > accepts a uint64_t bytes_copied?
>> > struct fuse_write_out {
>> >     union {
>> >         struct {
>> >             uint32_t size;
>> >             uint32_t padding;
>> >         };
>> >         uint64_t bytes_copied;
>> >     };
>> > };
>> >
>> > Maybe a little ugly but that seems backwards-compatible to me and
>> > would prevent needing a new FUSE_COPY_FILE_RANGE64.
>>
>> Even with a capability flag, it encourages the presence of bugs that
>> manifest only on big-endian systems.
>
> Interesting, can you explain how? size would always be accessed
> directly through write_out->size (instead of extracted from the upper
> 32 bits of bytes_copied), so wouldn't the compiler handle the correct
> memory access?

Incorrectly using the size member when bytes_copied is intended and vice
versa would mostly work on little-endian architectures (so hard to spot
during testing), but break spectacularly on big-endian architectures.

Thanks,
Florian

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Darrick J. Wong]

On Wed, Aug 13, 2025 at 10:03:17AM -0700, Joanne Koong wrote:
> On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > The FUSE protocol uses struct fuse_write_out to convey the return value of
> > copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> > interface supports a 64-bit size copies and there's no reason why copies
> > should be limited to 32-bit.
> >
> > Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> > number of bytes copied is returned in a 64-bit value.
> >
> > If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> > COPY_FILE_RANGE.
> 
> Is it unacceptable to add a union in struct fuse_write_out that
> accepts a uint64_t bytes_copied?
> struct fuse_write_out {
>     union {
>         struct {
>             uint32_t size;
>             uint32_t padding;
>         };
>         uint64_t bytes_copied;
>     };
> };
> 
> Maybe a little ugly but that seems backwards-compatible to me and
> would prevent needing a new FUSE_COPY_FILE_RANGE64.

I wonder, does fuse_args::out_argvar==1 imply that you could create a
new 64-bit fuse_write64_out:

struct fuse_write64_out {
	uint64_t size;
	uint64_t padding;
};

and then fuse_copy_file_range declares a union:

union fuse_cfr_out {
	struct fuse_write_out out;
	struct fuse_write64_out out64;
};

passes that into fuse_args:

	union fuse_cfr_out outarg;

	args.out_argvar = 1;
	args.out_numargs = 1;
	args.out_args[0].size = sizeof(outarg);
	args.out_args[0].value = &outarg;

and then we can switch on the results:

	if (args.out_args[0].size == sizeof(fuse_write64_out))
		/* 64-bit return */
	else if (args.out_args[0].size == sizeof(fuse_write_out))
		/* 32-bit return */
	else
		/* error */

I guess the problem is that userspace has to know that the kernel will
accept a fuse_write64_out, because on an old kernel it'll get -EINVAL
and ... then what?  I think an error return ends the request and the
fuse server can't just try again with fuse_write_out.

<shrug> Maybe I'm speculating stupi^Wwildly. ;)

--D

> >
> > Reported-by: Florian Weimer <fweimer@redhat.com>
> > Closes: https://lore.kernel.org/all/lhuh5ynl8z5.fsf@oldenburg.str.redhat.com/
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >  fs/fuse/file.c            | 44 ++++++++++++++++++++++++++++-----------
> >  fs/fuse/fuse_i.h          |  3 +++
> >  include/uapi/linux/fuse.h | 12 ++++++++++-
> >  3 files changed, 46 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 4adcf09d4b01..867b5fde1237 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3013,33 +3015,51 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,
> >         if (is_unstable)
> >                 set_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
> >
> > -       args.opcode = FUSE_COPY_FILE_RANGE;
> > +       args.opcode = FUSE_COPY_FILE_RANGE_64;
> >         args.nodeid = ff_in->nodeid;
> >         args.in_numargs = 1;
> >         args.in_args[0].size = sizeof(inarg);
> >         args.in_args[0].value = &inarg;
> >         args.out_numargs = 1;
> > -       args.out_args[0].size = sizeof(outarg);
> > -       args.out_args[0].value = &outarg;
> > +       args.out_args[0].size = sizeof(outarg_64);
> > +       args.out_args[0].value = &outarg_64;
> > +       if (fc->no_copy_file_range_64) {
> > +fallback:
> > +               /* Fall back to old op that can't handle large copy length */
> > +               args.opcode = FUSE_COPY_FILE_RANGE;
> > +               args.out_args[0].size = sizeof(outarg);
> > +               args.out_args[0].value = &outarg;
> > +               inarg.len = len = min_t(size_t, len, UINT_MAX & PAGE_MASK);
> > +       }
> >         err = fuse_simple_request(fm, &args);
> >         if (err == -ENOSYS) {
> > -               fc->no_copy_file_range = 1;
> > -               err = -EOPNOTSUPP;
> > +               if (fc->no_copy_file_range_64) {
> 
> Maybe clearer here to do the if check on the args.opcode? Then this
> could just be
> if (args.opcode == FUSE_COPY_FILE_RANGE) {
> 
> which imo is a lot easier to follow.
> 
> > +                       fc->no_copy_file_range = 1;
> > +                       err = -EOPNOTSUPP;
> > +               } else {
> > +                       fc->no_copy_file_range_64 = 1;
> > +                       goto fallback;
> > +               }
> >         }
> > -       if (!err && outarg.size > len)
> > -               err = -EIO;
> > -
> >         if (err)
> >                 goto out;
> >
> > +       bytes_copied = fc->no_copy_file_range_64 ?
> > +               outarg.size : outarg_64.bytes_copied;
> > +
> > +       if (bytes_copied > len) {
> > +               err = -EIO;
> > +               goto out;
> > +       }
> > +
> >         truncate_inode_pages_range(inode_out->i_mapping,
> >                                    ALIGN_DOWN(pos_out, PAGE_SIZE),
> > -                                  ALIGN(pos_out + outarg.size, PAGE_SIZE) - 1);
> > +                                  ALIGN(pos_out + bytes_copied, PAGE_SIZE) - 1);
> >
> >         file_update_time(file_out);
> > -       fuse_write_update_attr(inode_out, pos_out + outarg.size, outarg.size);
> > +       fuse_write_update_attr(inode_out, pos_out + bytes_copied, bytes_copied);
> >
> > -       err = outarg.size;
> > +       err = bytes_copied;
> >  out:
> >         if (is_unstable)
> >                 clear_bit(FUSE_I_SIZE_UNSTABLE, &fi_out->state);
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 122d6586e8d4..94621f68a5cc 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -1148,6 +1153,11 @@ struct fuse_copy_file_range_in {
> >         uint64_t        flags;
> >  };
> >
> > +/* For FUSE_COPY_FILE_RANGE_64 */
> > +struct fuse_copy_file_range_out {
> 
> imo having the 64 in the struct name more explicitly makes it clearer
> to the server which one they're supposed to use, eg struct
> fuse_copy_file_range64_out
> 
> Thanks,
> Joanne
> > +       uint64_t        bytes_copied;
> > +};
> > +
> 

Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Joanne Koong]

On Thu, Aug 14, 2025 at 10:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Aug 13, 2025 at 10:03:17AM -0700, Joanne Koong wrote:
> > On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> > >
> > > The FUSE protocol uses struct fuse_write_out to convey the return value of
> > > copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> > > interface supports a 64-bit size copies and there's no reason why copies
> > > should be limited to 32-bit.
> > >
> > > Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> > > number of bytes copied is returned in a 64-bit value.
> > >
> > > If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> > > COPY_FILE_RANGE.
> >
> > Is it unacceptable to add a union in struct fuse_write_out that
> > accepts a uint64_t bytes_copied?
> > struct fuse_write_out {
> >     union {
> >         struct {
> >             uint32_t size;
> >             uint32_t padding;
> >         };
> >         uint64_t bytes_copied;
> >     };
> > };
> >
> > Maybe a little ugly but that seems backwards-compatible to me and
> > would prevent needing a new FUSE_COPY_FILE_RANGE64.
>
> I wonder, does fuse_args::out_argvar==1 imply that you could create a
> new 64-bit fuse_write64_out:
>
> struct fuse_write64_out {
>         uint64_t size;
>         uint64_t padding;
> };
>
> and then fuse_copy_file_range declares a union:
>
> union fuse_cfr_out {
>         struct fuse_write_out out;
>         struct fuse_write64_out out64;
> };
>
> passes that into fuse_args:
>
>         union fuse_cfr_out outarg;
>
>         args.out_argvar = 1;
>         args.out_numargs = 1;
>         args.out_args[0].size = sizeof(outarg);
>         args.out_args[0].value = &outarg;
>
> and then we can switch on the results:
>
>         if (args.out_args[0].size == sizeof(fuse_write64_out))
>                 /* 64-bit return */
>         else if (args.out_args[0].size == sizeof(fuse_write_out))
>                 /* 32-bit return */
>         else
>                 /* error */
>
> I guess the problem is that userspace has to know that the kernel will
> accept a fuse_write64_out, because on an old kernel it'll get -EINVAL
> and ... then what?  I think an error return ends the request and the
> fuse server can't just try again with fuse_write_out.
>

I think this would also need the feature flag sent in the init call
which Miklos didn't like


> <shrug> Maybe I'm speculating stupi^Wwildly. ;)
>
> --D
>