public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Anshu Kumari <anskuma@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: sbrivio@redhat.com, passt-dev@passt.top, aerosound161@gmail.com,
	abdobngad@gmail.com, lvivier@redhat.com
Subject: Re: [PATCH 4/5] fuzz: Add AFL++ persistent mode fuzz loop
Date: Mon, 17 Aug 2026 14:43:40 +0530	[thread overview]
Message-ID: <CADJNnVL_-KDkV0xgk4qstqNi19bLqe=HoZUxThmEsd-5jctHeQ@mail.gmail.com> (raw)
In-Reply-To: <aoKD6FAfb6hm8D1z@zatzit>

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

On Mon, Aug 17, 2026 at 9:16 AM David Gibson <david@gibson.dropbear.id.au>
wrote:

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

turn = 0 (passt's turn)
turn = 1 (test-server's turn)

-> Once passt inject the TAP packet, it sets "turn = 1" using atomic store
which tells
    the test-server that passt has sent something and now it can perform
the read opr (as
    turn flag is mmap'd by both passt and fuzz-server).

-> passt then does spin-wait until it's turn != 0.

-> similary test-server is spinning on it's own,  checking if (turn == 1).
When it sees 1,
    it does recv() on the UNIX socket to read the frame passt sent,
    generates a protocol response (e.g., ARP reply, SYN-ACK),
    sends it back via send() on the same socket, then sets turn = 0.

-> Once passt sees turn = 0, it exit its spin-wait and continue processing.

Without this turn flag, there was a timing problem: passt could call
passt_worker()
before test-server had a chance to read the outbound frame and send it's
response back.
The turn flag guarantees the response is available before passt tries to
process it.


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


-- 
Anshu

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

  reply	other threads:[~2026-08-17  9:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  7:26 [PATCH 0/5] Add AFL++ fuzzing support for passt Anshu Kumari
2026-08-12  7:26 ` [PATCH 1/5] fuzz: Add deterministic wrappers for system calls Anshu Kumari
2026-08-13  3:46   ` David Gibson
2026-08-12  7:26 ` [PATCH 2/5] fuzz: Add flow type guards for fuzzing stability Anshu Kumari
2026-08-13  4:45   ` David Gibson
2026-08-12  7:26 ` [PATCH 3/5] fuzz: Bypass isolation and adapt sockets for AFL++ Anshu Kumari
2026-08-13  5:04   ` David Gibson
2026-08-12  7:26 ` [PATCH 4/5] fuzz: Add AFL++ persistent mode fuzz loop Anshu Kumari
2026-08-13  6:34   ` Stefano Brivio
2026-08-14  1:55   ` David Gibson
2026-08-14 10:32     ` Anshu Kumari
2026-08-17  3:46       ` David Gibson
2026-08-17  9:13         ` Anshu Kumari [this message]
2026-08-12  7:26 ` [PATCH 5/5] fuzz: Add test server for bidirectional protocol fuzzing Anshu Kumari
2026-08-13  7:53   ` Stefano Brivio
2026-08-14  5:40     ` David Gibson
2026-08-14  7:35       ` Stefano Brivio
2026-08-14 10:02         ` David Gibson
2026-08-14 11:55           ` Stefano Brivio
2026-08-17  6:58             ` David Gibson
2026-08-17 20:09               ` Stefano Brivio
2026-08-18  5:24                 ` David Gibson
2026-08-14  5:50   ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADJNnVL_-KDkV0xgk4qstqNi19bLqe=HoZUxThmEsd-5jctHeQ@mail.gmail.com' \
    --to=anskuma@redhat.com \
    --cc=abdobngad@gmail.com \
    --cc=aerosound161@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).