From 119a45f5f64c5c1391acf1df334842e14845b91a Mon Sep 17 00:00:00 2001 From: Sam Schweigel Date: Thu, 4 Dec 2025 17:54:15 -0800 Subject: [PATCH 1/3] Handle large read/write/sendfile to File; return actual bytes written The return value for the `jl_fs_write/read/sendfile` functions is an `int`, even through they all take `size_t` length arguments, resulting in potentially truncated counts for large file operations. Instead, on success (including partial read/write), we should return the `uv_fs_t.result` field. I have added a test for this but have left it disabled because it allocates a 5 GiB Vector to read into. One consequence of the truncated return values is that we previously mangled large files while copying them with sendfile. If the return value is truncated to a random negative value, it manifests as #30723 or #39868. If it is truncated to a positive value, sendfile will create a very large destination file with thousands of repeated sections of the source file, causing #56537. The bug will only manifest if LLVM decides to zero-extend the return value. This also changes `jl_fs_write` to return the actual number of bytes written, rather than the number of bytes it was asked to write. If a write returns EAGAIN (usually because the file is O_NONBLOCK/O_NODELAY), we never see the actual number of bytes written. --- base/filesystem.jl | 14 +++++++------- src/jl_uv.c | 22 +++++++++++++++------- test/file.jl | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/base/filesystem.jl b/base/filesystem.jl index 36d60f1d3318a..834c5c10f49a4 100644 --- a/base/filesystem.jl +++ b/base/filesystem.jl @@ -219,7 +219,7 @@ function sendfile(dst::File, src::File, src_offset::Int64, bytes::Int) check_open(dst) check_open(src) while true - result = ccall(:jl_fs_sendfile, Int32, (OS_HANDLE, OS_HANDLE, Int64, Csize_t), + result = ccall(:jl_fs_sendfile, Cssize_t, (OS_HANDLE, OS_HANDLE, Int64, Csize_t), src.handle, dst.handle, src_offset, bytes) uv_error("sendfile", result) nsent = result @@ -232,10 +232,10 @@ end function unsafe_write(f::File, buf::Ptr{UInt8}, len::UInt, offset::Int64=Int64(-1)) check_open(f) - err = ccall(:jl_fs_write, Int32, (OS_HANDLE, Ptr{UInt8}, Csize_t, Int64), + ret = ccall(:jl_fs_write, Cssize_t, (OS_HANDLE, Ptr{UInt8}, Csize_t, Int64), f.handle, buf, len, offset) - uv_error("write", err) - return len + uv_error("write", ret) + return ret end write(f::File, c::UInt8) = write(f, Ref{UInt8}(c)) @@ -265,7 +265,7 @@ end function read(f::File, ::Type{UInt8}) check_open(f) p = Ref{UInt8}() - ret = ccall(:jl_fs_read, Int32, (OS_HANDLE, Ptr{Cvoid}, Csize_t), + ret = ccall(:jl_fs_read, Cssize_t, (OS_HANDLE, Ptr{Cvoid}, Csize_t), f.handle, p, 1) uv_error("read", ret) @assert ret <= sizeof(p) == 1 @@ -298,7 +298,7 @@ read(f::File, ::Type{T}) where {T<:AbstractChar} = T(read(f, Char)) # fallback function unsafe_read(f::File, p::Ptr{UInt8}, nel::UInt) check_open(f) - ret = ccall(:jl_fs_read, Int32, (OS_HANDLE, Ptr{Cvoid}, Csize_t), + ret = ccall(:jl_fs_read, Cssize_t, (OS_HANDLE, Ptr{Cvoid}, Csize_t), f.handle, p, nel) uv_error("read", ret) ret == nel || throw(EOFError()) @@ -314,7 +314,7 @@ function readbytes!(f::File, b::MutableDenseArrayType{UInt8}, nb=length(b)) if length(b) < nr resize!(b, nr) end - ret = ccall(:jl_fs_read, Int32, (OS_HANDLE, Ptr{Cvoid}, Csize_t), + ret = ccall(:jl_fs_read, Cssize_t, (OS_HANDLE, Ptr{Cvoid}, Csize_t), f.handle, b, nr) uv_error("read", ret) return ret diff --git a/src/jl_uv.c b/src/jl_uv.c index e41b896320693..bdf08c75d90eb 100644 --- a/src/jl_uv.c +++ b/src/jl_uv.c @@ -583,8 +583,8 @@ JL_DLLEXPORT int jl_fs_rename(const char *src_path, const char *dst_path) return ret; } -JL_DLLEXPORT int jl_fs_sendfile(uv_os_fd_t src_fd, uv_os_fd_t dst_fd, - int64_t in_offset, size_t len) +JL_DLLEXPORT ssize_t jl_fs_sendfile(uv_os_fd_t src_fd, uv_os_fd_t dst_fd, + int64_t in_offset, size_t len) { uv_fs_t req; JL_SIGATOMIC_BEGIN(); @@ -592,7 +592,7 @@ JL_DLLEXPORT int jl_fs_sendfile(uv_os_fd_t src_fd, uv_os_fd_t dst_fd, in_offset, len, NULL); uv_fs_req_cleanup(&req); JL_SIGATOMIC_END(); - return ret; + return req.result ? req.result : ret; } JL_DLLEXPORT int jl_fs_hardlink(char *path, char *new_path) @@ -635,7 +635,15 @@ JL_DLLEXPORT int jl_fs_access(char *path, int mode) return ret; } -JL_DLLEXPORT int jl_fs_write(uv_os_fd_t handle, const char *data, size_t len, +/* + * A note about the uv_fs_* functions that return a count of bytes written: + * uv_fs_t.result is a ssize_t, but the function returns int. The result field + * needs to be checked so we can distinguish a real error from a count that + * overflows an int, but we can't use it directly---some error conditions will + * leave it set to 0 but return an error. + */ + +JL_DLLEXPORT ssize_t jl_fs_write(uv_os_fd_t handle, const char *data, size_t len, int64_t offset) JL_NOTSAFEPOINT { jl_task_t *ct = jl_get_current_task(); @@ -654,10 +662,10 @@ JL_DLLEXPORT int jl_fs_write(uv_os_fd_t handle, const char *data, size_t len, jl_io_loop = uv_default_loop(); int ret = uv_fs_write(unused_uv_loop_arg, &req, handle, buf, 1, offset, NULL); uv_fs_req_cleanup(&req); - return ret; + return req.result ? req.result : ret; } -JL_DLLEXPORT int jl_fs_read(uv_os_fd_t handle, char *data, size_t len) +JL_DLLEXPORT ssize_t jl_fs_read(uv_os_fd_t handle, char *data, size_t len) { uv_fs_t req; uv_buf_t buf[1]; @@ -665,7 +673,7 @@ JL_DLLEXPORT int jl_fs_read(uv_os_fd_t handle, char *data, size_t len) buf[0].len = len; int ret = uv_fs_read(unused_uv_loop_arg, &req, handle, buf, 1, -1, NULL); uv_fs_req_cleanup(&req); - return ret; + return req.result ? req.result : ret; } JL_DLLEXPORT int jl_fs_close(uv_os_fd_t handle) diff --git a/test/file.jl b/test/file.jl index 92e85125c8451..4e53ee9d65663 100644 --- a/test/file.jl +++ b/test/file.jl @@ -2200,3 +2200,36 @@ end end @test Base.infer_return_type(stat, (String,)) == Base.Filesystem.StatStruct + +# Partial write() to File should return the actual number of bytes written +@test let p = Pipe() + # Create a non-blocking pipe that will fill and return EAGAIN + Base.link_pipe!(p; writer_supports_async=true) + try + f = Base.Filesystem.File(Base._fd(p.in)) + n_read, n_write = 0, 0 + @sync begin + @async begin + n_write = write(f, rand(UInt8, 1000000)) + close(p.in) + end + n_read = length(read(p)) + end + n_write == n_read + finally + close(p) + end +end + +# Very large read() shouldn't return EOFError because the return value was truncated. +@test_skip let + FS = Base.Filesystem + f = FS.open("/dev/urandom", FS.JL_O_RDONLY) + try + v = Vector{UInt8}(undef, 5 * 2^30) # 5 GiB (larger than INT32_MAX) + read!(f, v) + true + finally + close(f) + end +end From 49f2ad75c3796b2ba0d8fefae10f6cd4d945d02a Mon Sep 17 00:00:00 2001 From: Sam Schweigel Date: Fri, 5 Dec 2025 09:08:10 -0800 Subject: [PATCH 2/3] Use async reader in partial write() test --- test/file.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/file.jl b/test/file.jl index 4e53ee9d65663..cd1632461deab 100644 --- a/test/file.jl +++ b/test/file.jl @@ -2204,7 +2204,7 @@ end # Partial write() to File should return the actual number of bytes written @test let p = Pipe() # Create a non-blocking pipe that will fill and return EAGAIN - Base.link_pipe!(p; writer_supports_async=true) + Base.link_pipe!(p; writer_supports_async=true, reader_supports_async=true) try f = Base.Filesystem.File(Base._fd(p.in)) n_read, n_write = 0, 0 From e3f29f1db76102ac7a0d518972f560f4f1dc67b5 Mon Sep 17 00:00:00 2001 From: Sam Schweigel Date: Fri, 5 Dec 2025 11:47:28 -0800 Subject: [PATCH 3/3] Don't run the partial write() test on Windows --- test/file.jl | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/test/file.jl b/test/file.jl index cd1632461deab..2c792bc5a65a6 100644 --- a/test/file.jl +++ b/test/file.jl @@ -2202,22 +2202,24 @@ end @test Base.infer_return_type(stat, (String,)) == Base.Filesystem.StatStruct # Partial write() to File should return the actual number of bytes written -@test let p = Pipe() - # Create a non-blocking pipe that will fill and return EAGAIN - Base.link_pipe!(p; writer_supports_async=true, reader_supports_async=true) - try - f = Base.Filesystem.File(Base._fd(p.in)) - n_read, n_write = 0, 0 - @sync begin - @async begin - n_write = write(f, rand(UInt8, 1000000)) - close(p.in) +if !Sys.iswindows() + @test let p = Pipe() + # Create a non-blocking pipe that will fill and return EAGAIN + Base.link_pipe!(p; writer_supports_async=true, reader_supports_async=true) + try + f = Base.Filesystem.File(Base._fd(p.in)) + n_read, n_write = 0, 0 + @sync begin + @async begin + n_write = write(f, rand(UInt8, 1000000)) + close(p.in) + end + n_read = length(read(p)) end - n_read = length(read(p)) + n_write == n_read + finally + close(p) end - n_write == n_read - finally - close(p) end end