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

[Anshu Kumari <anskuma@redhat.com>]

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

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.

>
> >
> > 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


> > +# 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.


>
> > +                             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.


>
> > +
> > +                     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

[-- Attachment #2: Type: text/html, Size: 21574 bytes --]