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=V2q0RooL; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 8C7705A0262 for ; Fri, 14 Aug 2026 03:57:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786672636; bh=S71PoslPna4rLnykdTj1Y0Ydkvd6/M4qb1jEuHGUEpU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V2q0RooLTZ6TcVFFSNF7a8C+k1dFGRUxYWZt9yrVSjzg1D8MvLzWhduwDfSFYFViZ iv9UwPSWB1/PoAJrPDsBkKyPvzcfwxaaAVPE+bGnqRsV+jb7kfEuthadxJiwMLrYPF Z0UfmpxRb+ID92ZgSWuUU4eu3hMkLDisBPHGTHgL5t/uWQiMDn9BKCr/Mbr2XTNnLZ 8H0nAqphKqZdlpiBUybedAWcTFOn2S2r1ayoxv1ZOVJTZrWDu0OZLR8+P4rJrHoIF3 4bsBJJzPYn/7RkXe601ccWd+pLIMP9mwqvjnBVluFqQ37ZbHqDrEJtBKzpKWoh6qln YVOBfE+Og5T4w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hLlj800xvz4wHr; Fri, 14 Aug 2026 11:57:15 +1000 (AEST) Date: Fri, 14 Aug 2026 11:55:51 +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="bFgmPyfPncRfJXK8" Content-Disposition: inline In-Reply-To: <20260812072630.3235261-5-anskuma@redhat.com> Message-ID-Hash: 43PWSGYQ5FDIEUGJFLDDL5TUSNBRXMAE X-Message-ID-Hash: 43PWSGYQ5FDIEUGJFLDDL5TUSNBRXMAE 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: --bFgmPyfPncRfJXK8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > 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 > Signed-off-by: Anshu Kumari > --- > Makefile | 8 +++ > passt.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 197 insertions(+) >=20 > 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 > =20 > +FUZZ_CC ?=3D afl-clang-fast > + > +.PHONY: fuzz > + > +fuzz: > + $(MAKE) clean > + $(MAKE) CC=3D"$(FUZZ_CC)" CPPFLAGS=3D"-DFUZZING -DNDEBUG" CFLAGS=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 > =20 > #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" > =20 > #define NUM_EPOLL_EVENTS 8 > =20 > #define TIMER_INTERVAL_ MIN(TCP_TIMER_INTERVAL, FWD_PORT_SCAN_INTERVAL) > #define TIMER_INTERVAL MIN(TIMER_INTERVAL_, FLOW_TIMER_INTERVAL) > =20 > +#ifdef FUZZING > + > +/* AFL++ persistent mode / shared memory fuzzing compatibility macros. */ > +#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? > +# 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. > + 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))); > =20 > struct ctx passt_ctx =3D { > @@ -282,9 +327,17 @@ static void passt_worker(void *opaque, int nfds, str= uct 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. > vu_control_handler(c->vdev, c->fd_tap, eventmask); > 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) > =20 > timer_init(c, &now); > =20 > +#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? > +#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? > + 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. > + > + 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 same */ > /* cppcheck-suppress [duplicateValueTernary, unmatchedSuppression] */ > @@ -461,4 +649,5 @@ loop: > passt_worker(c, nfds, events); > =20 > goto loop; > +#endif /* FUZZING */ > } > --=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 --bFgmPyfPncRfJXK8 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmp+dZsACgkQzQJF27ox 2GeKOQ//bhUZ4qI/F0WvX6LGrJFlFlrZNNLL31ToW/yQzAKD58BsuF4EO1N1O87U iKhX71QCObVfm1N5B9gpJMIrMcxsYDKdkUbPa1gEugI9/AySeXqK0ldo7VPy47QS f3AiPSUYpaNzSUAF8nhhffEdxm+Sq3U0bzXE+2AO6/AIc3us6WdSfBLfHwuUjZKg mhYOGq5M9M2BU4T5TDnN3JXs1SWprVJxzXb009hQcx7z1kDFoG39qHd2Iqeu/sIX EVSjIb+cv2AJUQCL6GwbGoWyH/oIwJKCbxkfB41s6Y9OzpQKFmLGOwQFF61CRBRb YAeorzbPOHqSUV+l9C9YuvT1spmm/JAkcjEEDBT0f++Wdj03CgsnCxAqRXf2P9ze 7L3YjdoXZPlId0TUhYWvncxryubynUmxDf255n4KwpHJjC6fxBoiFNWB5o2SBepr y5NvcUn3CzG6aH8BELYLKj+hqMlt7zbIsZItl3FzvMK49ojbXMV+JxTr9sSRyYq6 gyao50lHjC3dU19A0TdNUhk2BHzVAdP4q47hrGyyp35XdAf3a7/9i9TAIo/HaAxl ABtTIuQiHS1MjxyYYzpJf2znM2sdv7B4BbPDy1fYCvC9iY5z9Vo2RnXR1aRAHH5D 5qmjh5lchNXECEFqPCFg/jth2JYloWOAc6Ok72c9JE2ZoL8Egq4= =Mx17 -----END PGP SIGNATURE----- --bFgmPyfPncRfJXK8--