Re: [PATCH 4/5] fuzz: Add AFL++ persistent mode fuzz loop

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 17137 bytes --]

On Fri, Aug 14, 2026 at 04:02:58PM +0530, Anshu Kumari wrote:
> On Fri, Aug 14, 2026 at 7:27 AM David Gibson <david@gibson.dropbear.id.au>
> 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 <anskuma@redhat.com>
> > > ---
> > >  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 <sys/prctl.h>
> > >  #include <netinet/if_ether.h>
> > >  #include <libgen.h>
> > > +#include <netinet/tcp.h>
> > >
> > >  #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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]