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 --]
next prev parent 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).