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=p8gnQsjI; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 3C1C45A0262 for ; Mon, 17 Aug 2026 05:46:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786938368; bh=sGBrOcHtoPA0jyrehHMdBiZ+2F3VdkvC+7lswTxpacQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=p8gnQsjI9MCU8qoB1qLYgHZtdwz9jOqT4tMouV1HoWgLbN+V4yjq4f4SUbO4AAerr q8sXWa+uzCT2QCbmGM8Yul2qQHsCWrx3COma0C4Y0vqzBLFrH73b+d71dUCcHz7v6N OadHYJGlEhemleLsH2jv+N1WSFyS52A4Qe9PacVU0g5DSEEXI/WcLiVozzD+SMonJP eXPfU4odAE9vr7hB/ZGtF78hXXNNR9nqrWpWhuCjeu7VRuYPibDpfJUnRcAQBgs7pw NAMY1+HDRAvIa5u+8UCzLsyZyEvK9fEr0Zw4fMvo9V1xsNG4nUslZqtyyGdVRixIel 4wU/62PdeSV5Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hNdzN1xYqz4wBF; Mon, 17 Aug 2026 13:46:08 +1000 (AEST) Date: Mon, 17 Aug 2026 13:46:01 +1000 From: David Gibson To: Anshu Kumari Subject: Re: [PATCH 4/5] fuzz: Add AFL++ persistent mode fuzz loop Message-ID: References: <20260812072630.3235261-1-anskuma@redhat.com> <20260812072630.3235261-5-anskuma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="zuPejz26ZXcSEuV+" Content-Disposition: inline In-Reply-To: Message-ID-Hash: IVLXKWBBQNMC5MO5Z5JZT7BROQWJDX74 X-Message-ID-Hash: IVLXKWBBQNMC5MO5Z5JZT7BROQWJDX74 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: --zuPejz26ZXcSEuV+ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 14, 2026 at 04:02:58PM +0530, Anshu Kumari wrote: > On Fri, Aug 14, 2026 at 7:27=E2=80=AFAM David Gibson > wrote: >=20 > > On Wed, Aug 12, 2026 at 12:56:27PM +0530, Anshu Kumari wrote: > > > Add the AFL++ persistent mode fuzz loop to passt.c main(). > > > The loop uses __AFL_LOOP() for in-process iteration and > > > __AFL_FUZZ_TESTCASE_BUF for shared memory fuzzing. > > > > > > Each iteration: > > > - Resets deterministic clock, flow table, and epoll instance. > > > - Drains stale data from the TAP socket. > > > - Reads an epoll event from the AFL++ buffer. > > > - For TAP events: constructs a packet with fixed L2/L3/L4 > > > headers and injects it via tap_add_packet() + tap_handler(). > > > - Exchanges a turn flag with the test server for > > > bidirectional flow over the UNIX socket. > > > - Calls passt_worker() to process the event. > > > - Polls for host-side TCP events via epoll_wait(). > > > - Runs post_handler() for deferred work. > > > > > > Added the 'make fuzz' target which builds passt with > > > afl-clang-fast, -DFUZZING, -DNDEBUG, and AddressSanitizer. > > > > Stefano's concerns generally seconded (although I haven't really got > > my head around the role of the test server in either yours or his mind > > - I'll address that once I've read 5/5). > > > > The big concerns here are that to do interesting fuzzing we'll need a) > > sequences of multiple packets/packets and b) to fuzz-generate the > > headers, including malformed ones. > > > > AIUI, logically each fuzzer generated case could be run in a separate > > instance of passt: the __AFL_LOOP() stuff is an optimization to avoid > > the delay of a fresh startup on each cycle. Is that correct? >=20 > yes, without __AFL_LOOP(), AFL++ forks a fresh passt for each input. Understood. > > > Signed-off-by: Anshu Kumari > > > --- > > > Makefile | 8 +++ > > > passt.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++ > > > 2 files changed, 197 insertions(+) > > > > > > diff --git a/Makefile b/Makefile > > > index fe1df58..8e4121e 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -123,6 +123,14 @@ valgrind: BASE_CPPFLAGS +=3D -DVALGRIND > > > valgrind: BASE_CFLAGS +=3D -g > > > valgrind: all > > > > > > +FUZZ_CC ?=3D afl-clang-fast > > > + > > > +.PHONY: fuzz > > > + > > > +fuzz: > > > + $(MAKE) clean > > > + $(MAKE) CC=3D"$(FUZZ_CC)" CPPFLAGS=3D"-DFUZZING -DNDEBUG" CFLAG= S=3D"-g > > -fsanitize=3Daddress" passt > > > > I'd recommend building the fuzzing binary under a different name, to > > make accidentally using the wrong one a bit less likely. > > > > > .PHONY: clean > > > clean: > > > $(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h seccomp_pesto.h > > pasta.1 \ > > > diff --git a/passt.c b/passt.c > > > index 5054551..e026eb2 100644 > > > --- a/passt.c > > > +++ b/passt.c > > > @@ -35,6 +35,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > > > > #include "util.h" > > > #include "passt.h" > > > @@ -54,12 +55,56 @@ > > > #include "repair.h" > > > #include "netlink.h" > > > #include "epoll_ctl.h" > > > +#include "flow_table.h" > > > +#include "fuzz.h" > > > > > > #define NUM_EPOLL_EVENTS 8 > > > > > > #define TIMER_INTERVAL_ MIN(TCP_TIMER_INTERVAL, > > FWD_PORT_SCAN_INTERVAL) > > > #define TIMER_INTERVAL MIN(TIMER_INTERVAL_, > > FLOW_TIMER_INTERVAL) > > > > > > +#ifdef FUZZING > > > + > > > +/* AFL++ persistent mode / shared memory fuzzing compatibility macro= s. > > */ > > > +#ifndef __AFL_FUZZ_TESTCASE_LEN > > > + ssize_t fuzz_len; > > > + unsigned char fuzz_buf[1024 * 1024]; > > > +# define __AFL_FUZZ_TESTCASE_LEN fuzz_len > > > +# define __AFL_FUZZ_TESTCASE_BUF fuzz_buf > > > +# define __AFL_FUZZ_INIT() void sync(void) > > > +# define __AFL_LOOP(x) \ > > > + ((fuzz_len =3D read(0, fuzz_buf, sizeof(fuzz_buf))) > 0 ? 1 : 0) > > > > This macro ignores its parameter. Is that intentional? > > >=20 > Yes, this is intentional as it helps compile afl++ without afl-clang-fas= t. > more about this: > https://github.com/AFLplusplus/AFLplusplus/blob/stable/instrumentation/RE= ADME.persistent_mode.md#2-tldr Weird, ok. > > > +# define __AFL_INIT() sync() > > > +#endif > > > + > > > +#ifdef __AFL_HAVE_MANUAL_CONTROL > > > + __AFL_FUZZ_INIT(); > > > +#endif > > > + > > > +static struct fuzz_turn *fuzz_turn_ptr; > > > + > > > +/** > > > + * fuzz_turn_connect() - Map the turn flag shared memory > > > + * > > > + * Return: pointer to mapped turn flag, or NULL on failure > > > + */ > > > +static struct fuzz_turn *fuzz_turn_connect(void) > > > +{ > > > + struct fuzz_turn *t; > > > + int fd; > > > + > > > + fd =3D open(FUZZ_TURN_PATH, O_RDWR); > > > > FUZZ_TURN_PATH was defined in 1/5 but only used here, which makes > > review harder. I'd suggest moving the definition to this patch. > > >=20 > Noted !! >=20 > > > > > + if (fd < 0) > > > + return NULL; > > > + > > > + t =3D mmap(NULL, sizeof(*t), PROT_READ | PROT_WRITE, MAP_SHARED= , fd, > > 0); > > > + close(fd); > > > + > > > + return (t =3D=3D MAP_FAILED) ? NULL : t; > > > +} > > > + > > > +#endif > > > + > > > char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); > > > > > > struct ctx passt_ctx =3D { > > > @@ -282,9 +327,17 @@ static void passt_worker(void *opaque, int nfds, > > struct epoll_event *events) > > > icmp_sock_handler(c, ref, &now); > > > break; > > > case EPOLL_TYPE_VHOST_CMD: > > > +#ifdef FUZZING > > > + if (!c->vdev) > > > + break; > > > +#endif > > > > This serves a very similar purpose to the checks in 2/5, and the > > comments I had there apply here as well. If we ignore an event here, > > it means we're now on a path that's not really interesting to fuzz. > > So instead of ignoring and carrying on, it would be better to mark > > this as "program died correctly" and proceed to the next case. > > >=20 > Noted. >=20 > > > > > vu_control_handler(c->vdev, c->fd_tap, eventmas= k); > > > break; > > > case EPOLL_TYPE_VHOST_KICK: > > > +#ifdef FUZZING > > > + if (!c->vdev) > > > + break; > > > +#endif > > > vu_kick_cb(c->vdev, ref, &now); > > > break; > > > case EPOLL_TYPE_REPAIR_LISTEN: > > > @@ -450,6 +503,141 @@ int main(int argc, char **argv) > > > > > > timer_init(c, &now); > > > > > > +#ifdef FUZZING > > > + fuzz_turn_ptr =3D fuzz_turn_connect(); > > > + > > > +#define FUZZ_LOOP_ITERATIONS 10000 > > > > AFAICT, this has no effect, since __AFL_LOOP() ignores its parameter. > > > > > +#define FUZZ_DRAIN_BUF_SIZE 1600 > > > + > > > +#ifdef __AFL_HAVE_MANUAL_CONTROL > > > + __AFL_INIT(); > > > > Both the definition and use of __AFL_INIT() are conditional on > > __AFL_HAVE_MANUAL_CONTROL. Would it make more sense to define > > __AFL_INIT() as a no-op if !__AFL_HAVE_MANUAL_CONTROL to avoid a > > second #ifdef? > > > > I guess yes. Noted !! >=20 >=20 > > > +#endif > > > + { > > > + unsigned char *buf =3D __AFL_FUZZ_TESTCASE_BUF; > > > + > > > + while (__AFL_LOOP(FUZZ_LOOP_ITERATIONS)) { > > > + int len =3D __AFL_FUZZ_TESTCASE_LEN; > > > + int injected =3D 0; > > > + int pkt_len, round; > > > + struct epoll_event ev; > > > + union epoll_ref ref; > > > + int min_pkt =3D sizeof(struct ethhdr) + > > > + sizeof(struct iphdr) + > > > + sizeof(struct tcphdr); > > > + > > > + if (len < (int)sizeof(ev)) > > > + continue; > > > + > > > + /* Reset clock, flow table and epoll for each > > > + * AFL++ iteration. > > > + */ > > > + fuzz_clock_reset(); > > > + clock_gettime(CLOCK_MONOTONIC, &now); > > > + timer_init(c, &now); > > > + > > > + flow_init(); > > > > flow_init() wipes the table itself, but doesn't clean up any existing > > flows. If you're creating real external sockets, that means those > > will be leaked, which means you could well hit the file descriptor > > limit during a long fuzzing session. > > > > Also, it looks like flow_init() doesn't reset flow_first_free. > > > > > > Seeing the structure of the afl loop, I now have further thoughts on > > the assert()s you were suppressing earlier in the series. As I said, > > if we hit those we want to stop this fuzzing path - it's no longer > > interesting - but we don't want to mark it as a bug. A die() might > > accomplish that, but of course would mean restarting passt, bypassing > > the acceleration that __AFL_LOOP() is supposed to provide. > > > > Essentially what you want in those cases is to abort whatever you're > > doing and continue on to the next iteration of the AFL loop. This > > might make it one of the rare cases where setjmp() / longjmp() is a > > good idea. > > > > > + /* Recreate epoll instance */ > > > + close(c->epollfd); > > > + c->epollfd =3D epoll_create1(EPOLL_CLOEXEC); > > > + flow_epollid_register(EPOLLFD_ID_DEFAULT, > > c->epollfd); > > > + > > > + if (c->fd_tap >=3D 0) { > > > + union epoll_ref tref =3D { > > > + .type =3D EPOLL_TYPE_TAP_PASST, > > > + .fd =3D c->fd_tap > > > + }; > > > + epoll_add(c->epollfd, > > > + EPOLLIN | EPOLLRDHUP, tref); > > > + > > > + /* Drain stale socket data */ > > > + char drain[FUZZ_DRAIN_BUF_SIZE]; > > > + while (recv(c->fd_tap, drain, > > sizeof(drain), > > > + MSG_DONTWAIT) > 0); > > > > You could use MSG_TRUNC here to avoid the need for a drain buffer. > > > > > + } > > > + > > > + /* Read epoll event from AFL++ buffer */ > > > + memcpy(&ev, buf, sizeof(ev)); > > > + ref =3D *((union epoll_ref *)&ev.data.u64); > > > + > > > + /* Set recv payload in AFL++ shared memory */ > > > + fuzz_recv_data =3D buf + FUZZ_RECV_OFF; > > > + fuzz_recv_data_len =3D > > > + (len > FUZZ_RECV_OFF + FUZZ_RECV_MAX) > > > + ? FUZZ_RECV_MAX > > > + : ((len > FUZZ_RECV_OFF) > > > + ? len - FUZZ_RECV_OFF := 0); > > > + > > > + /* Inject fuzz packet for TAP events */ > > > + if (ref.type =3D=3D EPOLL_TYPE_TAP_PASST || > > > + ref.type =3D=3D EPOLL_TYPE_TAP_PASTA) { > > > + struct iov_tail data; > > > + struct ethhdr *eh; > > > + struct iphdr *iph; > > > + struct tcphdr *th; > > > + > > > + tap_flush_pools(); > > > + memset(pkt_buf, 0, min_pkt); > > > + > > > + pkt_len =3D len - (int)sizeof(ev); > > > > How does this differ from fuzz_recv_data_len? > > >=20 > pkt_len has the size of the TAP packet injected. >=20 > fuzz_recv_data_len contains the size of the recv payload available to the > determinstic > fuzz_recv()/fuzz_recvmsg() wrappers. It starts at offset 12 and can go up= to > 64KB. Ok, but they both have the same value of (len - sizeof(ev)). The fuzz_recv_data_len case checks some more edge cases and uses different defines, but it will mostly work out to the same thing. That seems odd. > > > > > + if (pkt_len > 0) > > > + memcpy(pkt_buf, buf + sizeof(ev= ), > > > + pkt_len); > > > + if (pkt_len < min_pkt) > > > + pkt_len =3D min_pkt; > > > + > > > + /* construct ethernet header */ > > > + eh =3D (struct ethhdr *)pkt_buf; > > > + memcpy(eh->h_dest, c->our_tap_mac, > > ETH_ALEN); > > > + memcpy(eh->h_source, c->guest_mac, > > ETH_ALEN); > > > + eh->h_proto =3D htons(ETH_P_IP); > > > + > > > + /* construct IPv4 header */ > > > + iph =3D (struct iphdr *)(pkt_buf + > > sizeof(*eh)); > > > + iph->version =3D 4; > > > + iph->ihl =3D 5; > > > + iph->protocol =3D IPPROTO_TCP; > > > + iph->saddr =3D c->ip4.addr.s_addr; > > > + iph->daddr =3D c->ip4.guest_gw.s_addr; > > > + iph->tot_len =3D htons(pkt_len - > > sizeof(*eh)); > > > + > > > + /* Fix TCP Header */ > > > + th =3D (struct tcphdr *)(pkt_buf + > > sizeof(*eh) + > > > + > > sizeof(*iph)); > > > + th->dest =3D htons(9999); > > > + if (th->doff < 5) > > > + th->doff =3D 5; > > > > As Stefano also points out, this is constructing a fixed version of > > exactly the things we most want to fuzz. > > > > > + data =3D IOV_TAIL_FROM_BUF(pkt_buf, pkt= _len, > > 0); > > > + tap_add_packet(c, &data, &now); > > > + tap_handler(c, &now); > > > + injected =3D 1; > > > + } > > > + > > > + /* Turn exchange -- only if data was sent */ > > > + if (injected && fuzz_turn_ptr) { > > > + __atomic_store_n(&fuzz_turn_ptr->turn, = 1, > > > + __ATOMIC_RELEASE); > > > + while > > (__atomic_load_n(&fuzz_turn_ptr->turn, > > > + __ATOMIC_ACQUIRE)= !=3D > > 0); > > > + } > > > > I don't really understand what this 'turn' thing is doing. > > >=20 > "turn" flag is being used to synchronized the frame exchange between passt > and fuzz-server over the UNIX socket. I figured, but can you elaborate on how exactly it does that. > > > + > > > + passt_worker(c, 1, &ev); > > > + > > > + /* Process host-side TCP events */ > > > + for (round =3D 0; round < 4; round++) { > > > + nfds =3D epoll_wait(c->epollfd, events, > > > + NUM_EPOLL_EVENTS, 0); > > > + if (nfds <=3D 0) > > > + break; > > > + passt_worker(c, nfds, events); > > > + } > > > + > > > + post_handler(c, &now); > > > > post_handler() is already called from passt_worker(), why do we need > > another call? > > > > > + } > > > + } > > > + return 0; > > > +#else > > > loop: > > > /* NOLINTBEGIN(bugprone-branch-clone): intervals can be the sam= e */ > > > /* cppcheck-suppress [duplicateValueTernary, unmatchedSuppressi= on] > > */ > > > @@ -461,4 +649,5 @@ loop: > > > passt_worker(c, nfds, events); > > > > > > goto loop; > > > +#endif /* FUZZING */ > > > } > > > -- > > > 2.55.0 > > > > > > > -- > > David Gibson (he or they) | I'll have my music baroque, and my co= de > > david AT gibson.dropbear.id.au | minimalist, thank you, not the other = way > > | around. > > http://www.ozlabs.org/~dgibson > > >=20 >=20 > --=20 > Anshu --=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 --zuPejz26ZXcSEuV+ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmqCg+gACgkQzQJF27ox 2GfTWA//V0q7rKXeMNYlKF+VMxuDcSL1JcDTlsPYIoQiwrpnkgL/9wh+vVzQmwnp v9wtpzyB+FOpidxFxTcs4d6RN+/Pmthu9FfZ28yvXueC71N47QQQIE8weo6qOQta Hh3i1oW8zw4VrnRLT38RcF5ax8bPO9xacEVQ/XQqIdPa3yzPJnITOTILinGGdOVm O7Xfk7ai1v9fXcRrIjlfHB/RRFHC2K5gKzYdq2ABSepDz0wfNjRX3konXBXA6bWc /17mtVTzKlTgZ+HoV5r2FTJHFOH9Kzxwh4co8L2OXNYkyJyc9OQgNbkM/UZZjKX+ H2SS3v0ilzWec20UVQmB8fXkmlIBaKRD079O+jAiyb7i8IWwGC14SUx/tDMioaFW DpMpxGoaQVzduWQ300VqMC6h/aV6NNQgBjEkEIAg+n+VfGCGxD8i1ZSciAs5taIC KGuofn2teDl3bHmxMmW0lO1+YlK0bqp8jxN9uLqvvMT9qimwn07TXBq3tXu3guSw RwwYoT0o49OftGCW8BalrZ0TTYhFthOnAfGY45E3tQzGOBgR5pVHEEv49nBgHqf0 HiGXnDrW8tzHVTdQ+7Cq9I3MAxM/sFNANsstJ/Msq204yrCopqgeBYxt0FG8lPZa 7KFTSrkvId5KZQUGbaDXqxBCFbKVWsUiQg5SouOogeJg8/wNXX8= =j/v4 -----END PGP SIGNATURE----- --zuPejz26ZXcSEuV+--