Re: git: dfad790c8cca - main - sendfile: stop abusing kern_writev()

On Tue, Jul 07, 2026 at 09:47:35PM +0000, Konstantin Belousov wrote:
> The branch main has been updated by kib:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=dfad790c8ccad05ff603ceaa5b2efe4205b38e1c
> 
> commit dfad790c8ccad05ff603ceaa5b2efe4205b38e1c
> Author:     Konstantin Belousov <kib@FreeBSD.org>
> AuthorDate: 2026-07-04 02:29:56 +0000
> Commit:     Konstantin Belousov <kib@FreeBSD.org>
> CommitDate: 2026-07-07 21:47:18 +0000
> 
>     sendfile: stop abusing kern_writev()
>     
>     Provide convenient wrapper kern_filewrite() around fo_write().
>     Switch to use it in vn_sendfile().  This allows to avoid duplicate
>     fget() when we already have the reference to the file, which creates a
>     correctness race with the userspace.  Also td_retval[0] clearing hack
>     can be removed.
>     
>     Reviewed by:    glebius, markj
>     Sponsored by:   The FreeBSD Foundation
>     MFC after:      1 week
>     Differential revision:  https://reviews.freebsd.org/D58035
> ---
>  sys/kern/kern_sendfile.c | 25 +++++++++++++------------
>  sys/kern/sys_generic.c   | 41 ++++++++++++++++++++++++++++++-----------
>  sys/sys/syscallsubr.h    |  2 ++
>  3 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/sys/kern/kern_sendfile.c b/sys/kern/kern_sendfile.c
> index cf9716560c07..1a534210ca0c 100644
> --- a/sys/kern/kern_sendfile.c
> +++ b/sys/kern/kern_sendfile.c
> @@ -1168,14 +1168,17 @@ prepend_header:
>  	}
>  
>  	/*
> -	 * Send trailers. Wimp out and use writev(2).
> +	 * Send trailers.
>  	 */
>  	if (trl_uio != NULL) {
> +		ssize_t cnt;
> +
>  		SOCK_IO_SEND_UNLOCK(so);
>  		CURVNET_RESTORE();
> -		error = kern_writev(td, sockfd, trl_uio);
> +		error = kern_filewrite(td, sockfd, sock_fp, trl_uio,
> +		    (off_t)-1, 0, &cnt);
>  		if (error == 0)
> -			sbytes += td->td_retval[0];
> +			sbytes += cnt;
>  		goto out;
>  	}
>  
> @@ -1183,15 +1186,8 @@ done:
>  	SOCK_IO_SEND_UNLOCK(so);
>  	CURVNET_RESTORE();
>  out:
> -	/*
> -	 * If there was no error we have to clear td->td_retval[0]
> -	 * because it may have been set by writev.
> -	 */
> -	if (error == 0) {
> -		td->td_retval[0] = 0;
> -		if (sbytes > 0 && vp != NULL)
> -			INOTIFY(vp, IN_ACCESS);
> -	}
> +	if (error == 0 && sbytes > 0 && vp != NULL)
> +		INOTIFY(vp, IN_ACCESS);
>  	if (sent != NULL) {
>  		(*sent) = sbytes;
>  	}
> @@ -1260,6 +1256,11 @@ sendfile(struct thread *td, struct sendfile_args *uap, int compat)
>  			    &trl_uio);
>  			if (error != 0)
>  				goto out;
> +			if (trl_uio->uio_rw != UIO_WRITE) {

cloneuio() does not set uio_rw.  sendfile() was relying on kern_writev()
to do that.  A GENERIC-KMSAN kernel panics when running the test suite:

panic: MSan: Uninitialized malloc memory from copyinuio+0x11d
cpuid = 5
time = 1783537112
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0xa7/frame 0xfffffe00764d7840
vpanic() at vpanic+0x538/frame 0xfffffe00764d79c0
panic() at panic+0x1dd/frame 0xfffffe00764d7ad0
__msan_warning() at __msan_warning+0x25d/frame 0xfffffe00764d7c20
sendfile() at sendfile+0xcb6/frame 0xfffffe00764d7d50
amd64_syscall() at amd64_syscall+0x837/frame 0xfffffe00764d7f30
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe00764d7f30
--- syscall (393, FreeBSD ELF64, sendfile), rip = 0x21aad2ea3b0a, rsp = 0x21aad1d706c8, rbp = 0x21aad1d70760 ---
KDB: enter: panic