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

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

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

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?

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

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

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

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

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

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

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

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