On Fri, Aug 14, 2026 at 04:02:58PM +0530, Anshu Kumari wrote: > On Fri, Aug 14, 2026 at 7:27 AM David Gibson > wrote: > > > 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? > > 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 += -DVALGRIND > > > valgrind: BASE_CFLAGS += -g > > > valgrind: all > > > > > > +FUZZ_CC ?= afl-clang-fast > > > + > > > +.PHONY: fuzz > > > + > > > +fuzz: > > > + $(MAKE) clean > > > + $(MAKE) CC="$(FUZZ_CC)" CPPFLAGS="-DFUZZING -DNDEBUG" CFLAGS="-g > > -fsanitize=address" 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 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 = read(0, fuzz_buf, sizeof(fuzz_buf))) > 0 ? 1 : 0) > > > > This macro ignores its parameter. Is that intentional? > > > > Yes, this is intentional as it helps compile afl++ without afl-clang-fast. > more about this: > https://github.com/AFLplusplus/AFLplusplus/blob/stable/instrumentation/README.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 = 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. > > > > Noted !! > > > > > > + if (fd < 0) > > > + return NULL; > > > + > > > + t = mmap(NULL, sizeof(*t), PROT_READ | PROT_WRITE, MAP_SHARED, fd, > > 0); > > > + close(fd); > > > + > > > + return (t == MAP_FAILED) ? NULL : t; > > > +} > > > + > > > +#endif > > > + > > > char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); > > > > > > struct ctx passt_ctx = { > > > @@ -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. > > > > Noted. > > > > > > 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) > > > > > > timer_init(c, &now); > > > > > > +#ifdef FUZZING > > > + fuzz_turn_ptr = 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 !! > > > > > +#endif > > > + { > > > + unsigned char *buf = __AFL_FUZZ_TESTCASE_BUF; > > > + > > > + while (__AFL_LOOP(FUZZ_LOOP_ITERATIONS)) { > > > + int len = __AFL_FUZZ_TESTCASE_LEN; > > > + int injected = 0; > > > + int pkt_len, round; > > > + struct epoll_event ev; > > > + union epoll_ref ref; > > > + int min_pkt = 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 = epoll_create1(EPOLL_CLOEXEC); > > > + flow_epollid_register(EPOLLFD_ID_DEFAULT, > > c->epollfd); > > > + > > > + if (c->fd_tap >= 0) { > > > + union epoll_ref tref = { > > > + .type = EPOLL_TYPE_TAP_PASST, > > > + .fd = 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 = *((union epoll_ref *)&ev.data.u64); > > > + > > > + /* Set recv payload in AFL++ shared memory */ > > > + fuzz_recv_data = buf + FUZZ_RECV_OFF; > > > + fuzz_recv_data_len = > > > + (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 == EPOLL_TYPE_TAP_PASST || > > > + ref.type == 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 = len - (int)sizeof(ev); > > > > How does this differ from fuzz_recv_data_len? > > > > pkt_len has the size of the TAP packet injected. > > 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 upto > 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 = min_pkt; > > > + > > > + /* construct ethernet header */ > > > + eh = (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 = htons(ETH_P_IP); > > > + > > > + /* construct IPv4 header */ > > > + iph = (struct iphdr *)(pkt_buf + > > sizeof(*eh)); > > > + iph->version = 4; > > > + iph->ihl = 5; > > > + iph->protocol = IPPROTO_TCP; > > > + iph->saddr = c->ip4.addr.s_addr; > > > + iph->daddr = c->ip4.guest_gw.s_addr; > > > + iph->tot_len = htons(pkt_len - > > sizeof(*eh)); > > > + > > > + /* Fix TCP Header */ > > > + th = (struct tcphdr *)(pkt_buf + > > sizeof(*eh) + > > > + > > sizeof(*iph)); > > > + th->dest = htons(9999); > > > + if (th->doff < 5) > > > + th->doff = 5; > > > > As Stefano also points out, this is constructing a fixed version of > > exactly the things we most want to fuzz. > > > > > + data = IOV_TAIL_FROM_BUF(pkt_buf, pkt_len, > > 0); > > > + tap_add_packet(c, &data, &now); > > > + tap_handler(c, &now); > > > + injected = 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) != > > 0); > > > + } > > > > I don't really understand what this 'turn' thing is doing. > > > > "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 = 0; round < 4; round++) { > > > + nfds = epoll_wait(c->epollfd, events, > > > + NUM_EPOLL_EVENTS, 0); > > > + if (nfds <= 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); > > > > > > goto loop; > > > +#endif /* FUZZING */ > > > } > > > -- > > > 2.55.0 > > > > > > > -- > > 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 > > > > > -- > Anshu -- 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