sendfile(2): rework compat implementation
ClosedPublic
Actions

Authored by brooks on Aug 20 2026, 9:16 AM.

Details

Reviewers

kib
jhb
glebius
markj

Group Reviewers

cheri

Commits

rGc1e733258770: sendfile(2): rework compat implementation

Summary

Rename the sendfile() function to kern_sendfile(), expand the arguments in sendfile_args, and extend with two function pointer arguments to copy in the header/trailer structure and the create uio's for the header and trailer as required. Use this to allow the removal of freebsd32_do_sendfile() which was a nearly identical duplicate of sendfile() with attendant maintenance cost.

Effort: CHERI upstreaming
Sponsored by: Innovate UK

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 76139
Build 73022: arc lint + arc unit

Event Timeline

brooks created this revision.Aug 20 2026, 9:16 AM

Herald added subscribers: olce, imp. · View Herald TranscriptAug 20 2026, 9:16 AM

brooks requested review of this revision.Aug 20 2026, 9:16 AM

Harbormaster completed remote builds in B75779: Diff 184445.Aug 20 2026, 9:16 AM

Herald added a reviewer: cheri. · View Herald TranscriptAug 20 2026, 9:16 AM

Herald added a reviewer: cheri. · View Herald Transcript

Merging the implementations will avoid future instances of oversights like the one fixed in 4b17776d9afd0009ac8547126c59c97eda0f3fc9 as well as avoiding a third copy of this code from freebsd64 when it lands.

markj added inline comments.Aug 20 2026, 1:36 PM

sys/compat/freebsd32/freebsd32_misc.c
2201	Should the first parameter just be `const void *`? Otherwise it looks like we're calling this function via an incompatible function pointer. I'm not sure if that's UB.
2212
sys/kern/kern_sendfile.c
1250	Can we handle this in a COMPAT_FREEBSD4-specific copyinuio callback? That seems cleaner, and then you can drop the `compat` flag.

emaste added a subscriber: emaste.Aug 20 2026, 1:38 PM

Why passing struct sendile_args to kern_sendfile()? This is not how all other kern_* wrappers are done, and IMO is worse than flattening the args. You have to explicitly initialize the literal anyway, which makes it equivalent to list the function's argument in the call.

freebsd32_copyin_hdtr: take a void *, fix trailer count
make copyinuio_t take a void * (but don't update the functions yet)

Harbormaster completed remote builds in B76139: Diff 185018.Aug 25 2026, 9:56 AM

In D59034#1354663, @kib wrote:

Why passing struct sendile_args to kern_sendfile()? This is not how all other kern_* wrappers are done, and IMO is worse than flattening the args. You have to explicitly initialize the literal anyway, which makes it equivalent to list the function's argument in the call.

It's the lowest churn change since that's how sendfile() worked, but I could flatten the arguments (or use a different structure that would benefit slightly from default initialization.)

sys/compat/freebsd32/freebsd32_misc.c
2201	I can't imagine a compiler where it mattered (particularly since it's only used by copyin, which is assembly). A type-aware ABI is certainly possible, but would be a bit crazy. In this case I've followed the pattern of copyinuio.
sys/kern/kern_sendfile.c
1250	I don't see an easy way to do this as we're not modifying the uio, but instead updating `nbytes`. I guess I could do something like: if (update_nbytes != NULL) update_nbytes(&nbytes, uio); That would at least move it out of the main code path at the cost of having to expose the callback globally for the compat code. On a somewhat related note, I can't help but wonder if we should be returning EINVAL for `uap->nbytes < hdr_uio->uio_resid`.

markj added inline comments.Aug 25 2026, 5:58 PM

sys/kern/kern_sendfile.c
1250	Couldn't you could provide a sendfile-specific copyinuio wrapper which takes the `uap` as a parameter and updates `nbytes`? On a somewhat related note, I can't help but wonder if we should be returning EINVAL for uap->nbytes < hdr_uio->uio_resid. In the compat case you mean? The `uap->nbytes == 0` case is special, so you'd have to exclude that at least, but it seems wrong to make the compat code stricter in any case.

brooks added inline comments.Aug 26 2026, 12:39 PM

sys/kern/kern_sendfile.c
1250	Couldn't you could provide a sendfile-specific copyinuio wrapper which takes the `uap` as a parameter and updates `nbytes`? I've created D59198 which does roughly that. If it's what you're thinking of, I'll squash into this one.

markj accepted this revision as: markj.Aug 26 2026, 1:33 PM

This revision is now accepted and ready to land.Aug 26 2026, 1:33 PM

markj added inline comments.Aug 26 2026, 1:34 PM

sys/kern/kern_sendfile.c
1250	I left a comment on that review. I'm okay with this change as it is.

In D59034#1357245, @brooks wrote:

In D59034#1354663, @kib wrote:

Why passing struct sendile_args to kern_sendfile()? This is not how all other kern_* wrappers are done, and IMO is worse than flattening the args. You have to explicitly initialize the literal anyway, which makes it equivalent to list the function's argument in the call.

It's the lowest churn change since that's how sendfile() worked, but I could flatten the arguments (or use a different structure that would benefit slightly from default initialization.)