From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202608 header.b=s+Z0aeRb; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 825665A0269 for ; Thu, 13 Aug 2026 07:04:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786597482; bh=NCUtUq8dAxMvpIwHZlJNr6/0vC9IYEm915zn5AOFZh0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=s+Z0aeRb7zb/CPn5F/mQkxkdQXh3YtRWzURihXfoX4VXaeoN8nT5EC1p748U8NcHx e/4TxNcCPtaPxrzpIhFRZzmnKooec2QdY/vhaAWJsgbN3h9VVaDWbJWYzC1oYKZm4z X4r2SevB3cuPw/ewob63cAHb8b2g6ctKNql8OzyHgeX43WrqrO0dtDc0xk4gL7BmzU t3ViZPnZ8kdrMYGLBqmoZwcecsAeyWP3v+hd/M7CYD8FCf5B9KUrTh9I0EQ4IbVWkk ukuar9SA2cxaOt7BW3ILMER9OWZxYuhmx++g5GwnffPZ6DSO8J8OzvKSl7SYRJNFBH rPVtPSYETs7aw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hLCvt38RHz4w1f; Thu, 13 Aug 2026 15:04:42 +1000 (AEST) Date: Thu, 13 Aug 2026 14:45:41 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH 2/5] fuzz: Add flow type guards for fuzzing stability Message-ID: References: <20260812072630.3235261-1-anskuma@redhat.com> <20260812072630.3235261-3-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="YK93iLm4tKiz0zA8" Content-Disposition: inline In-Reply-To: <20260812072630.3235261-3-anskuma@redhat.com> Message-ID-Hash: DERLXTZT3DPNFEZ5VPGAFUW3FXQ6LKE2 X-Message-ID-Hash: DERLXTZT3DPNFEZ5VPGAFUW3FXQ6LKE2 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: sbrivio@redhat.com, passt-dev@passt.top, aerosound161@gmail.com, abdobngad@gmail.com, lvivier@redhat.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --YK93iLm4tKiz0zA8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 12, 2026 at 12:56:25PM +0530, Anshu Kumari wrote: > Under FUZZING, AFL++ can inject arbitrary epoll event types > from its shared memory buffer. When an event references a flow > table entry whose type doesn't match the handler, the existing > assert() crashes the process eventually masking the real bugs. This makes sense to me up until the "eventually masking the real bugs". IIUC the way AFL works is that it will use the coverage feedback to adjust its input - so even if many inputs result in a early crash due to mismatching types, it should be able to discover inputs that get past that into more interesting territory. Is it just a question of trying to filter our all the uninteresting crashes from flow type mismatches? Or to put it another way, distinguishing between bugs and correct abnormal exits due to something going wrong? > If there is no flow at the start of fuzzing then also we are > just returning early instead of hitting crashes. I don't really understand what you mean by this. > Allowing the > fuzzer to explore other code path. >=20 > Replace assert() with NULL returns in the flow-lookup functions > when compiled with -DFUZZING: >=20 > - tcp.c: conn_at_sidx(), tcp_timer_handler(), tcp_sock_handler() > - tcp_splice.c: conn_at_sidx(), tcp_splice_sock_handler() > - udp.c: udp_sock_handler(), udp_sock_to_sock(), > udp_buf_sock_to_tap(), udp_sock_fwd() error path > - udp_flow.c: udp_at_sidx() > - icmp.c: ping_at_sidx(), icmp_sock_handler() So, in looking at this, I'm bearing in mind the distinction between what should be a die() and what should be an assert(). Generally if something external did something wrong we can't cope with, it's a die() - and that's not a bug. If we did something wrong it's an assert() and that is a bug. The tricky case is where the "external" thing is the kernel. We obviously have to trust it to some extent, so if we get something we never expect to get from a kernel interace is that a die() or an assert(). The reasoning for these being assert()s is that if we get something unexpected here, by far the most likely cause is that we did something wrong when we set the epoll reference, rather than the kernel returning garbage. Fuzzing, at least with the current draft setup is breaking that assumption because the simulated kernel *is* returning garbage a lot of the time. So, we either need to constrain generation of epoll events to "plausible" ones, or safely ignore bad events. AIUI this patch is taking the second approach. That basic approach makes sense to me, but I'm not loving the way it works out in practice. It's fairly ugly and invasive, for starters. But, hat concerns me more though, is that at the points you're removing the assert()s it's not very obvious what new other paths the early returns will trigger. Because those are paths we'll never reach in real workloads, they're also not that interesting to fuzz. I can think of two possible approaches that might be this nicer. 1) IIUC the way the coverage driven fuzzing works, in the assert() cases we don't really need to carry on looking for other bugs. It should be ok to exit immediately as long as that is flagged as "we exited because something went wrong externally" not "we exited because we hit a bug". The coverage driven engine should then be able to go back and generate new inputs that explore other paths. I don't really know the interfaces used to communicate with AFL++, but could we replace these assert()s with say fuzz_assert(), which still exits, but signals to the fuzzer that this exit is not a bug? Is changing the abort() to a die() #ifdef FUZZING enough to do that? 2) We could move epoll event validation to immediately after the epoll_wait(). We esentially want to filter generated epoll events to plausible ones - at least meaning that the returned reference is one of the ones we epoll_add()ed. In one way, that's awkward, because we have to have a single point with all the validation logic, which might vary across various different branches. On the other hand, it makes the invasiveness much more localised, and it's clearer what a validation failure will do, whether that's exit in a non-bug way, or just ignore this event and go on to the next one. > Signed-off-by: Anshu Kumari > --- > icmp.c | 14 +++++++++++++- > tcp.c | 19 ++++++++++++++++++- > tcp_splice.c | 10 ++++++++++ > udp.c | 29 +++++++++++++++++++++++++++-- > udp_flow.c | 5 +++++ > 5 files changed, 73 insertions(+), 4 deletions(-) >=20 > diff --git a/icmp.c b/icmp.c > index 0fe2366..cdfa253 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -39,6 +39,7 @@ > #include "icmp.h" > #include "flow_table.h" > #include "epoll_ctl.h" > +#include "fuzz.h" > =20 > #define ICMP_ECHO_TIMEOUT 60 /* s, timeout for ICMP socket activity */ > #define ICMP_NUM_IDS (1U << 16) > @@ -58,7 +59,12 @@ static struct icmp_ping_flow *ping_at_sidx(flow_sidx_t= sidx) > if (!flow) > return NULL; > =20 > +#ifdef FUZZING > + if (flow->f.type !=3D FLOW_PING4 && flow->f.type !=3D FLOW_PING6) > + return NULL; > +#else > assert(flow->f.type =3D=3D FLOW_PING4 || flow->f.type =3D=3D FLOW_PING6= ); > +#endif > return &flow->ping; > } > =20 > @@ -72,7 +78,13 @@ void icmp_sock_handler(const struct ctx *c, union epol= l_ref ref, > const struct timespec *now) > { > struct icmp_ping_flow *pingf =3D ping_at_sidx(ref.flowside); > - const struct flowside *ini =3D &pingf->f.side[INISIDE]; > + const struct flowside *ini; > + > +#ifdef FUZZING > + if (!pingf) > + return; > +#endif > + ini =3D &pingf->f.side[INISIDE]; > union sockaddr_inany sr; > socklen_t sl =3D sizeof(sr); > char buf[USHRT_MAX]; > diff --git a/tcp.c b/tcp.c > index 3b78d2e..612c884 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -316,6 +316,7 @@ > #include "tcp_buf.h" > #include "tcp_vu.h" > #include "epoll_ctl.h" > +#include "fuzz.h" > =20 > /* > * The size of TCP header (including options) is given by doff (Data Off= set) > @@ -456,7 +457,12 @@ static struct tcp_tap_conn *conn_at_sidx(flow_sidx_t= sidx) > if (!flow) > return NULL; > =20 > +#ifdef FUZZING > + if (flow->f.type !=3D FLOW_TCP) > + return NULL; > +#else > assert(flow->f.type =3D=3D FLOW_TCP); > +#endif > return &flow->tcp; > } > =20 > @@ -2681,7 +2687,14 @@ void tcp_timer_handler(const struct ctx *c, union = epoll_ref ref, > const struct timespec *now) > { > struct itimerspec check_armed =3D { { 0 }, { 0 } }; > - struct tcp_tap_conn *conn =3D &FLOW(ref.flow)->tcp; > + struct tcp_tap_conn *conn; > + > +#ifdef FUZZING > + if (ref.flow >=3D FLOW_MAX || > + FLOW(ref.flow)->f.type !=3D FLOW_TCP) > + return; > +#endif > + conn =3D &FLOW(ref.flow)->tcp; > =20 > assert(!c->no_tcp); > assert(conn->f.type =3D=3D FLOW_TCP); > @@ -2752,6 +2765,10 @@ void tcp_sock_handler(const struct ctx *c, union e= poll_ref ref, > { > struct tcp_tap_conn *conn =3D conn_at_sidx(ref.flowside); > =20 > +#ifdef FUZZING > + if (!conn) > + return; > +#endif > assert(!c->no_tcp); > assert(pif_at_sidx(ref.flowside) !=3D PIF_TAP); > =20 > diff --git a/tcp_splice.c b/tcp_splice.c > index 4b01f1a..005ecd1 100644 > --- a/tcp_splice.c > +++ b/tcp_splice.c > @@ -105,7 +105,12 @@ static struct tcp_splice_conn *conn_at_sidx(flow_sid= x_t sidx) > if (!flow) > return NULL; > =20 > +#ifdef FUZZING > + if (flow->f.type !=3D FLOW_TCP_SPLICE) > + return NULL; > +#else > assert(flow->f.type =3D=3D FLOW_TCP_SPLICE); > +#endif > return &flow->tcp_splice; > } > =20 > @@ -594,6 +599,11 @@ void tcp_splice_sock_handler(struct ctx *c, union ep= oll_ref ref, > struct tcp_splice_conn *conn =3D conn_at_sidx(ref.flowside); > unsigned evsidei =3D ref.flowside.sidei; > =20 > +#ifdef FUZZING > + if (!conn) > + return; > +#endif > + > assert(conn->f.type =3D=3D FLOW_TCP_SPLICE); > =20 > if (conn->events =3D=3D SPLICE_CLOSED) > diff --git a/udp.c b/udp.c > index 505e554..9431353 100644 > --- a/udp.c > +++ b/udp.c > @@ -118,6 +118,7 @@ > #include "udp_internal.h" > #include "udp_vu.h" > #include "epoll_ctl.h" > +#include "fuzz.h" > =20 > #define UDP_MAX_FRAMES 32 /* max # of frames to receive at once */ > =20 > @@ -807,9 +808,15 @@ static void udp_sock_to_sock(const struct ctx *c, in= t from_s, int n, > const struct flowside *toside =3D flowside_at_sidx(tosidx); > const struct udp_flow *uflow =3D udp_at_sidx(tosidx); > uint8_t topif =3D pif_at_sidx(tosidx); > - int to_s =3D uflow->s[tosidx.sidei]; > + int to_s; > int i; > =20 > +#ifdef FUZZING > + if (!uflow) > + return; > +#endif > + to_s =3D uflow->s[tosidx.sidei]; > + > if ((n =3D udp_sock_recv(c, from_s, udp_mh_recv, n)) <=3D 0) > return; > =20 > @@ -836,9 +843,15 @@ static void udp_buf_sock_to_tap(const struct ctx *c,= int s, int n, > { > const struct flowside *toside =3D flowside_at_sidx(tosidx); > struct udp_flow *uflow =3D udp_at_sidx(tosidx); > - uint8_t *omac =3D uflow->f.tap_omac; > + uint8_t *omac; > int i; > =20 > +#ifdef FUZZING > + if (!uflow) > + return; > +#endif > + omac =3D uflow->f.tap_omac; > + > if ((n =3D udp_sock_recv(c, s, udp_mh_recv, n)) <=3D 0) > return; > =20 > @@ -901,10 +914,18 @@ void udp_sock_fwd(const struct ctx *c, int s, int r= ule_hint, > } else if (flow_sidx_valid(tosidx)) { > struct udp_flow *uflow =3D udp_at_sidx(tosidx); > =20 > +#ifdef FUZZING > + if (!uflow) { > + discard =3D true; > + continue; > + } > +#endif > + > flow_err_ratelimit( > uflow, now, > "No support for forwarding UDP from %s to %s", > pif_name(frompif), pif_name(topif)); > + > discard =3D true; > } else { > warn_ratelimit(now, "Discarding datagram without flow"); > @@ -949,6 +970,10 @@ void udp_sock_handler(const struct ctx *c, union epo= ll_ref ref, > { > struct udp_flow *uflow =3D udp_at_sidx(ref.flowside); > =20 > +#ifdef FUZZING > + if (!uflow) > + return; > +#endif > assert(!c->no_udp && uflow); > =20 > if (events & EPOLLERR) { > diff --git a/udp_flow.c b/udp_flow.c > index f59649f..6c5b010 100644 > --- a/udp_flow.c > +++ b/udp_flow.c > @@ -31,7 +31,12 @@ struct udp_flow *udp_at_sidx(flow_sidx_t sidx) > if (!flow) > return NULL; > =20 > +#ifdef FUZZING > + if (flow->f.type !=3D FLOW_UDP) > + return NULL; > +#else > assert(flow->f.type =3D=3D FLOW_UDP); > +#endif > return &flow->udp; > } > =20 > --=20 > 2.55.0 >=20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --YK93iLm4tKiz0zA8 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmp9S+YACgkQzQJF27ox 2Gcu5RAAlUXaGY0tsDI0yKYo0mfF+A/YvGefBGLoJep4ZmvsdX98malKKXvZRYB/ NivuBL8f9AwQqDgQo8H07Jf5MZcBziLJmaD9/paEtllvarVSFxVVO+90be5Gg30i kwZDpMCKvuJuBuBw5w6o0eAe3Ckk8Xz7cHfWuFhMKj3OKROdjQd+v3Xa4accucwr Kr+VhS5DiX1dWf5tBTB/iwBA7DYCTr5r55vnjgspcD+ysb79QVedWWzfZNMGgsup E154hwueB+0HCTHsH3dnaM8JO5szXKNoDeU+CYKslA4/P0Z67+xyTOXo2Un64I1l 7bO9LBzcXxoMt8BJwiXZE1x6eQjrdplVkBUDN43f7M+sci6/9/B3hxRM6sxO5i7K H1Xk/fa2uLWKUrW3WTKtkm0Wwr004BNtTlktSosvbdvunwvxvezljpIrO55KCw3M eIoxx+iA+D+bDvwwWIbsgB+1dvBFL6WqqVbzu/mFIEu7sy2RKMAIlEW+cxdeb0fY UipyDGnWAOZEAmiuwCtWwYFbCSFxEaxkMMwz2Iy6NSLmb+/SgaguOG7Y7kF6gN9j vR/Ozlrf5XVDPIhyW1aoJCTrDftIfNjqlPC8jOsWCPOAiSt8ofzRHFtA7bnzph8l cg1eLX7ehZgj/Bhu0jMGwEkT3l2HrVyYMhKm0o4ZudTQyncX7ng= =ydZq -----END PGP SIGNATURE----- --YK93iLm4tKiz0zA8--