Date: Tue, 6 May 2025 12:40:09 -0700 From: Kevin Bowling <kevin.bowling@kev009.com> To: Gleb Smirnoff <glebius@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 9a7d03c7df35 - main - sendfile: cover the entire sendfile operation under CURVNET_SET() Message-ID: <CAK7dMtCgHOKery82vM%2BqrwsWchMeou3EHqL=qQ_v_qF7e5RUsQ@mail.gmail.com> In-Reply-To: <202505061934.546JYexL056488@gitrepo.freebsd.org> References: <202505061934.546JYexL056488@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, May 6, 2025 at 12:34=E2=80=AFPM Gleb Smirnoff <glebius@freebsd.org>= wrote: > > The branch main has been updated by glebius: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D9a7d03c7df3536cdb5faf0848c= 6dab4a5d7bcef8 > > commit 9a7d03c7df3536cdb5faf0848c6dab4a5d7bcef8 > Author: Gleb Smirnoff <glebius@FreeBSD.org> > AuthorDate: 2025-05-06 18:15:15 +0000 > Commit: Gleb Smirnoff <glebius@FreeBSD.org> > CommitDate: 2025-05-06 19:34:26 +0000 > > sendfile: cover the entire sendfile operation under CURVNET_SET() > > There is no reason to set/restore for every single pr_send(), as it n= ever > changes. Also, cover call into pr_sendfile_wait with CURVNET_SET() f= ixing > recent regression in unix(4). > > Now we would enter sendfile_iodone() with curvnet set, when called > synchronously. Although, it is easy to tell a syncronous call from I= /O > completion, unfortunately the vnet(9) macros do not support condition= al > invocation, so just change CURVNET_SET() to CURVNET_SET_QUIET() and a= dd a > comment that we are aware of the recursion. Seeing a build error on main with this --- kern_sendfile.o --- /usr/src/sys/kern/kern_sendfile.c:788:6: error: variable 'saved_vnet' is used uninitialized whenever 'if' condition is true [ -Werror,-Wsometimes-uninitialized] 788 | if (error !=3D 0) | ^~~~~~~~~~ /usr/src/sys/kern/kern_sendfile.c:1273:2: note: uninitialized use occurs he= re 1273 | CURVNET_RESTORE(); and /usr/src/sys/kern/kern_sendfile.c:788:2: note: remove the 'if' if its condition is always false 788 | if (error !=3D 0) | ^~~~~~~~~~~~~~~ 789 | goto out; | ~~~~~~~~ > Reported-by: syzbot+7b0b20cf2c672c181d98@syzkaller.appspotmail.com > --- > sys/kern/kern_sendfile.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/sys/kern/kern_sendfile.c b/sys/kern/kern_sendfile.c > index 1fd1828c37c7..c428d80e0e1a 100644 > --- a/sys/kern/kern_sendfile.c > +++ b/sys/kern/kern_sendfile.c > @@ -285,6 +285,11 @@ sendfile_iowait(struct sf_io *sfio, const char *wmes= g) > > /* > * I/O completion callback. > + * > + * When called via I/O path, the curvnet is not set and should be obtain= ed > + * from the socket. When called synchronously from vn_sendfile(), usual= ly > + * to report error or just release the reference (all pages are valid), = then > + * curvnet shall be already set. > */ > static void > sendfile_iodone(void *arg, vm_page_t *pa, int count, int error) > @@ -365,7 +370,7 @@ sendfile_iodone(void *arg, vm_page_t *pa, int count, = int error) > ("non-ext_pgs mbuf with TLS session")); > #endif > so =3D sfio->so; > - CURVNET_SET(so->so_vnet); > + CURVNET_SET_QUIET(so->so_vnet); > if (__predict_false(sfio->error)) { > /* > * I/O operation failed. The state of data in the socket > @@ -782,6 +787,7 @@ vn_sendfile(struct file *fp, int sockfd, struct uio *= hdr_uio, > error =3D sendfile_getsock(td, sockfd, &sock_fp, &so); > if (error !=3D 0) > goto out; > + CURVNET_SET(so->so_vnet); > pr =3D so->so_proto; > > #ifdef MAC > @@ -1161,7 +1167,6 @@ prepend_header: > ("%s: mlen %u space %d hdrlen %d", > __func__, m_length(m, NULL), space, hdrlen)); > > - CURVNET_SET(so->so_vnet); > #ifdef KERN_TLS > if (tls !=3D NULL) > ktls_frame(m, tls, &tls_enq_cnt, TLS_RLTYPE_APP); > @@ -1203,8 +1208,6 @@ prepend_header: > tcp_log_sendfile(so, offset, nbytes, flags); > } > #endif > - CURVNET_RESTORE(); > - > m =3D NULL; > if (error) > goto done; > @@ -1265,9 +1268,9 @@ out: > if (tls !=3D NULL) > ktls_free(tls); > #endif > - > if (error =3D=3D ERESTART) > error =3D EINTR; > + CURVNET_RESTORE(); > > return (error); > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAK7dMtCgHOKery82vM%2BqrwsWchMeou3EHqL=qQ_v_qF7e5RUsQ>