Created attachment 247381 [details] Fix handling of truncation in vn_generic_copy_file_range() This patch fixes the truncation problem. It will be committed to main and MFC'd soon.
When vn_generic_copy_file_range() is given a large len argument, but the file represented by infd is smaller than the file represented by outfd, the file represented by outfd can be truncated when it should not do so. This was caused by a calculation that uses "len" but not the actual size of the file repesented by infd when len extends past the infd file's EOF. When copy_file_range(2) was first developed, if *inoffp + len exceeded the infd file's EOF an error was returned. This changed when the Linux implementation's semantics changed to allow this, introducing this bug. The attachment patch fixes the problem and will be committed/MFC'd soon.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=2319ca6a01816f7fc85d623097c639f239e18c6a commit 2319ca6a01816f7fc85d623097c639f239e18c6a Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2023-12-31 23:55:24 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2023-12-31 23:55:24 +0000 vfs_vnops.c: Fix vn_generic_copy_file_range() for truncation When copy_file_range(2) was first being developed, *inoffp + len had to be <= infile_size or an error was returned. This semantic (as defined by Linux) changed to allow *inoffp + len to be greater than infile_size and the copy would end at *inoffp + infile_size. Unfortunately, the code that decided if the outfd should be truncated in length did not get updated for this semantics change. As such, if a copy_file_range(2) is done, where infile_size - *inoffp is less that outfile_size but len is large, the outfd file is truncated when it should not be. (The semantics for this for Linux is to not truncate outfd in this case.) This patch fixes the problem. I believe the calculation is safe for all non-negative values of outsize, *outoffp, *inoffp and insize, which should be ok, since they are all guaranteed to be non-negative. Note that this bug is not observed over NFSv4.2, since it truncates len to infile_size - *inoffp. PR: 276045 Reviewed by: asomers, kib MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D43258 sys/kern/vfs_vnops.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=2f3ff6fe1a69ad5736e0d1d1575678c846e04402 commit 2f3ff6fe1a69ad5736e0d1d1575678c846e04402 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2023-12-31 23:55:24 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-01-03 01:22:26 +0000 vfs_vnops.c: Fix vn_generic_copy_file_range() for truncation When copy_file_range(2) was first being developed, *inoffp + len had to be <= infile_size or an error was returned. This semantic (as defined by Linux) changed to allow *inoffp + len to be greater than infile_size and the copy would end at *inoffp + infile_size. Unfortunately, the code that decided if the outfd should be truncated in length did not get updated for this semantics change. As such, if a copy_file_range(2) is done, where infile_size - *inoffp is less that outfile_size but len is large, the outfd file is truncated when it should not be. (The semantics for this for Linux is to not truncate outfd in this case.) This patch fixes the problem. I believe the calculation is safe for all non-negative values of outsize, *outoffp, *inoffp and insize, which should be ok, since they are all guaranteed to be non-negative. Note that this bug is not observed over NFSv4.2, since it truncates len to infile_size - *inoffp. PR: 276045 (cherry picked from commit 2319ca6a01816f7fc85d623097c639f239e18c6a) sys/kern/vfs_vnops.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e7044084cf813bfb66cbea8e9278895b26eda5d2 commit e7044084cf813bfb66cbea8e9278895b26eda5d2 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2023-12-31 23:55:24 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-01-03 01:34:31 +0000 vfs_vnops.c: Fix vn_generic_copy_file_range() for truncation When copy_file_range(2) was first being developed, *inoffp + len had to be <= infile_size or an error was returned. This semantic (as defined by Linux) changed to allow *inoffp + len to be greater than infile_size and the copy would end at *inoffp + infile_size. Unfortunately, the code that decided if the outfd should be truncated in length did not get updated for this semantics change. As such, if a copy_file_range(2) is done, where infile_size - *inoffp is less that outfile_size but len is large, the outfd file is truncated when it should not be. (The semantics for this for Linux is to not truncate outfd in this case.) This patch fixes the problem. I believe the calculation is safe for all non-negative values of outsize, *outoffp, *inoffp and insize, which should be ok, since they are all guaranteed to be non-negative. Note that this bug is not observed over NFSv4.2, since it truncates len to infile_size - *inoffp. PR: 276045 (cherry picked from commit 2319ca6a01816f7fc85d623097c639f239e18c6a) sys/kern/vfs_vnops.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
The patch has been committed to main and MFC'd. I have left this PR in progress, in case an errata for FreeBSD14.0 is deemed appropriate.
(In reply to Rick Macklem from comment #6) The stable/13 patch bails out as there is no 'outsize' variable: cc -target aarch64-unknown-freebsd13.2 --sysroot=/usr/obj/usr/src/arm64.aarch64/tmp -B/usr/obj/usr/src/arm64.aarch64/tmp/usr/bin -c -O2 -pipe -fno-strict-aliasing -g -nostdinc -I. -I/usr/src/sys -I/usr/src/sys/contrib/ck/include -I/usr/src/sys/contrib/libfdt -I/usr/src/sys/contrib/device-tree/include -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -fno-common -DLINUX_DTS_VERSION=\""5.9"\" -mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 -mstack-protector-guard-offset=0 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -MD -MF.depend.vfs_vnops.o -MTvfs_vnops.o -fdebug-prefix-map=./machine=/usr/src/sys/arm64/include -mgeneral-regs-only -ffixed-x18 -ffreestanding -fwrapv -fstack-protector -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wcast-qual -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__ -Wmissing-include-dirs -fdiagnostics-show-option -Wno-unknown-pragmas -Wno-error=tautological-compare -Wno-error=empty-body -Wno-error=parentheses-equality -Wno-error=unused-function -Wno-error=pointer-sign -Wno-error=shift-negative-value -Wno-address-of-packed-member -Wno-error=array-parameter -Wno-error=deprecated-non-prototype -Wno-error=strict-prototypes -Wno-error=unused-but-set-variable -Wno-error=unused-but-set-variable -Wno-format-zero-length -std=iso9899:1999 -Werror /usr/src/sys/kern/vfs_vnops.c /usr/src/sys/kern/vfs_vnops.c:3361:7: error: use of undeclared identifier 'outsize'; did you mean 'blksize'? outsize <= *outoffp + (inva.va_size - *inoffp)) { ^~~~~~~ blksize /usr/src/sys/kern/vfs_vnops.c:3301:9: note: 'blksize' declared here u_long blksize; ^ 1 error generated. *** [vfs_vnops.o] Error code 1 make[2]: stopped in /usr/obj/usr/src/arm64.aarch64/sys/GENERIC 1 error make[2]: stopped in /usr/obj/usr/src/arm64.aarch64/sys/GENERIC make[1]: stopped in /usr/src make: stopped in /usr/src
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=71cfc19e024bd68caf6bd2a4ec2e2e4fececf12d commit 71cfc19e024bd68caf6bd2a4ec2e2e4fececf12d Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2024-01-03 15:40:15 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-01-03 15:40:15 +0000 vfs_vnops.c: Fix cherry-pick of e7044084cf81 Oops, my bad. When I did the cherry-pick of e7044084cf81 I missed changing outsize to va.va_size. This direct commit fixes it. PR: 276045 sys/kern/vfs_vnops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Rick, do you happen to have a test program handy which reproduces this? It would be nice to have it in the regression test suite.
Created attachment 247804 [details] Test for this trncation bug This little program tests to see if this truncation bug exists.
I have no idea how the test suite works, but I have created an attachment with a little program that tests for the bug.
Created attachment 247805 [details] Test for this truncation bug This version of the little test program checks for the output file having correct data as well as correct size after the copy_file_range(2) loop.