Page MenuHomeFreeBSD

copy_file_range: Fix overlap checking
ClosedPublic

Authored by markj on Apr 5 2025, 4:40 PM.
Tags
None
Referenced Files
F132445124: D49674.id153183.diff
Fri, Oct 17, 12:24 AM
Unknown Object (File)
Fri, Oct 10, 4:26 PM
Unknown Object (File)
Sat, Oct 4, 12:47 AM
Unknown Object (File)
Fri, Oct 3, 5:25 PM
Unknown Object (File)
Fri, Oct 3, 5:11 AM
Unknown Object (File)
Fri, Oct 3, 5:01 AM
Unknown Object (File)
Fri, Oct 3, 4:56 AM
Unknown Object (File)
Sat, Sep 27, 6:20 AM
Subscribers

Details

Summary

The check for range overlap did not correctly handle negative offests,
as the addition inoff + len is promoted to an unsigned type. Moreover,
the range-locking code did not check for overflow.

Add some checks to make sure the ranges are valid. vn_copy_file_range()
has weaker checks: it also returns an error for a negative offset, but
overflow results in the copy length being truncated. Is this behaviour
required for compatibility with Linux?

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Apr 5 2025, 4:40 PM
sys/kern/vfs_syscalls.c
5087

The "len" argument is often just SSIZE_MAX to
copy to EOF on the input file, so I think truncation
of "off + len" to OFF_MAX (or whatever it is called?)
when it overflows is correct.

Truncate lengths rather than returning an error.

markj marked an inline comment as done.Apr 5 2025, 8:46 PM
sys/kern/vfs_syscalls.c
5087

I think this is ok.

However, rangelocking should handle the
full uint64_t file size, I think? (The argument
is vm_ooffset_t, which appears to be uint64_t.)

If so, since len is already clipped to SSIZE_MAX
above here in the code, this clipping should not
be needed, I think?

sys/kern/vfs_syscalls.c
5087

You're right. I was also concerned about overflow in the check immediately below in the invp == outvp case, but clamping len to SSIZE_MAX is sufficient there too.

Remove unnecessary clamping.

This revision is now accepted and ready to land.Apr 7 2025, 12:28 PM
This revision was automatically updated to reflect the committed changes.