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

From: Mark Johnston <markj_at_freebsd.org>
Date: Wed, 08 Jul 2026 22:57:21 UTC
On Wed, Jul 08, 2026 at 11:23:27PM +0300, Konstantin Belousov wrote:
> On Wed, Jul 08, 2026 at 03:36:44PM -0400, Mark Johnston wrote:
> > On Wed, Jul 08, 2026 at 10:23:06PM +0300, Konstantin Belousov wrote:
> > > On Wed, Jul 08, 2026 at 02:59:47PM -0400, Mark Johnston wrote:
> > > > On Tue, Jul 07, 2026 at 09:47:35PM +0000, Konstantin Belousov wrote:
> > > > >  			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
> > > 
> > > The following should be enough, am I correct?
> > 
> > I am re-running the test suite to see.
> > 
> > I am also not sure about the offset.  dofilewrite() sets
> > auio->uio_offset = offset.  kern_filewrite() takes an offset parameter
> > and does nothing with it.
> > 
> > copyinuio() initializes it to -1, so KMSAN will not report a problem,
> > but it looks wrong to me.
> 
> The streaming sockets are kind of append-only, so uio_offset should
> be irrelevant for them.  But indeed kern_filewrite() should not take
> explicit offset, it was a mistake to keep the argument after the split.

> 
> commit 605bd06dd7fca162967ce0f99bcbd31d982b1a37
> Author: Konstantin Belousov <kib@FreeBSD.org>
> Date:   Wed Jul 8 23:21:26 2026 +0300
> 
>     kern_filewrite(): remove unused argument offset
>     
>     Noted by:       markj
>     Fixes:  dfad790c8cca ("sendfile: stop abusing kern_writev()")
> 

Seems ok.  I am surprised the compiler didn't catch this.  It seems we
need to explicitly pass -Wunused-parameter, and if I do this there are
many places that need to be patched.

There is another bug, the comparison `auio->uio_resid != cnt` in
dofilewrite() is no longer correct.  It needs to be `cnt != 0`.