From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id D81FF5A0265 for ; Wed, 21 Oct 2026 04:01:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202610; t=1792548102; bh=UGsnR7Zj8b1XdqkZJZbgCbIl62Lw7mhK5pWH85DqoZQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=p8xNhW7uD9UonEBC2x2SVM8rd353MCSq3QNdOd2X81K/HgS4+AkmXvixDuOvy6hUF 9gZYMthil7uqNNNmV39m4PARsLiCE7cimgNH4KWCB6ELd+Q3utHpljPpKTmGNSWArQ N5cCz1eyXzu6kCIJLN70hPInUWKZhpFJzx2+DIjxKFzrLgf+HCPZ9MPleIH5/I9By4 DZsZ38prokIvFzzfyKUF4BBAcPfgD1aKOpy9rKgdw+Eav0OGjsVjDen3lJc+wJfoZz tBjGcV20wPG2kn3mg9ra/fSmKJdw6gCQuP7IN15VM/VuLNHEDHb8TyLsVxfjwI7dxW V5040TlKSZveA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4j8XZt0z43z4wCH; Wed, 21 Oct 2026 13:01:42 +1100 (AEDT) Date: Wed, 21 Oct 2026 12:38:42 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] tcp: Avoid division by zero with small MSS values Message-ID: References: <20261016112811.4176811-1-anskuma@redhat.com> <20261021021112.385ea997@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="7naecYyHiRgSc5dl" Content-Disposition: inline In-Reply-To: <20261021021112.385ea997@elisabeth> Message-ID-Hash: PRFA7G3D35G2JGP3CNSWJEH72XPWSLFY X-Message-ID-Hash: PRFA7G3D35G2JGP3CNSWJEH72XPWSLFY X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Anshu Kumari , passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --7naecYyHiRgSc5dl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 21, 2026 at 02:11:13AM +0200, Stefano Brivio wrote: > On Fri, 16 Oct 2026 16:58:11 +0530 > Anshu Kumari wrote: >=20 > > Fuzz testing with AFL++ persistent mode found a crash >=20 > Yay, that's quite an achievement! >=20 > > in > > tcp_buf_data_from_sock(), SIGFPE (signal 8) from division > > by zero at: > >=20 > > fill_bufs =3D DIV_ROUND_UP(wnd_scaled - already_sent, mss); > >=20 > > when we split buffers on MSS boundaries, if the MSS is small > > enough (less than four, rounded down to 0). > >=20 > > A guest advertising such a small MSS indicates unusual > > behaviour because the minimum usable MSS is > > MTU - IP header - TCP header (28 bytes for the minimum > > IPv4 MTU of 68). To improve robustness: > >=20 > > - Fix MSS_SET() to round up when MSS is between 1 and 3, > > storing tap_mss as 1 (recovered as 4 by MSS_GET()). >=20 > This introduces a subtle problem though: if the MSS is between 1 and 3 > (silly but I guess legitimate), we would overflow it now. >=20 > I think we should rather adjust MSS_GET() as well, to always use 1 for > values 0 < x < 4. >=20 > > - In case of zero MSS, don't send data: tcp_buf_data_from_sock() > > now returns early. > >=20 > > This is not a security issue: the effect of a zero MSS from > > a guest input was a clean process termination (SIGFPE), not > > memory corruption or data leakage. >=20 > And, actually, a rather convenient termination because the guest might > be doing something nasty here, so much that an assert() could also be a > reasonable choice instead (even though I still think that trying to be > robust against this condition makes more sense). assert() seems dubious to me, but resetting the connection would certainly be reasonable. In fact, it might be best to send an RST immediately to a SYN with MSS of 0, since such a connection would be useless. > Anyway, now that we continue, we should really make sure we're not > introducing actual issues. Maybe it would be good to keep fuzzing for a > while with this change, and see what else (if anything) a zero or very > low MSS can actually trigger. >=20 > >=20 > > Signed-off-by: Anshu Kumari > > --- > > tcp_buf.c | 3 +++ > > tcp_conn.h | 3 ++- > > 2 files changed, 5 insertions(+), 1 deletion(-) > >=20 > > diff --git a/tcp_buf.c b/tcp_buf.c > > index 72c45412..ee8c7ce8 100644 > > --- a/tcp_buf.c > > +++ b/tcp_buf.c > > @@ -335,6 +335,9 @@ int tcp_buf_data_from_sock(const struct ctx *c, str= uct tcp_tap_conn *conn, > > struct iovec *iov; > > uint32_t seq; > > =20 > > + if (!mss) > > + return 0; > > + > > /* Set up buffer descriptors we'll fill completely and partially. */ > > fill_bufs =3D DIV_ROUND_UP(wnd_scaled - already_sent, mss); > > if (fill_bufs > TCP_FRAMES) { > > diff --git a/tcp_conn.h b/tcp_conn.h > > index e3c55341..3f5ad28b 100644 > > --- a/tcp_conn.h > > +++ b/tcp_conn.h > > @@ -51,7 +51,8 @@ struct tcp_tap_conn { > > =20 > > #define TCP_MSS_BITS 14 > > unsigned int tap_mss :TCP_MSS_BITS; > > -#define MSS_SET(conn, mss) (conn->tap_mss =3D (mss >> (16 - TCP_MSS_BI= TS))) > > +#define MSS_SET(conn, mss) \ > > + (conn->tap_mss =3D (mss) ? MAX(1, (mss) >> (16 - TCP_MSS_BITS)) : 0) >=20 > I think this would deserve a comment at this point (above the define), > clearly explaining that we are use 1 just to avoid rounding to zero for > values less than 4. >=20 > And, I guess (untested): >=20 > > #define MSS_GET(conn) (conn->tap_mss << (16 - TCP_MSS_BITS)) >=20 > #define MSS_GET(conn) \ > ((conn->tap_mss =3D=3D 1) ? 1 : (conn->tap_mss << (16 - TCP_MSS_BITS))) >=20 > ...with another comment explaining that tap_mss is stored as 1 for > any non-zero MSS value <=3D 4. >=20 > > =20 > > #define RTT_EXP_BITS 4 >=20 > --=20 > Stefano >=20 --=20 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 --7naecYyHiRgSc5dl Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmrYF5MACgkQzQJF27ox 2GerUxAAi9Q+cQ2bnAyQR5vLVuH4qpeJYnNh1uGtQg+sE/VFeHZpzfniGkvIZpfU 9pIuzBNlHsh6ymMBk8YUsooMCpjN5+rTS2CSYKhvdjFFIxWRrjVdBJEcsKJQKmDp PZ3aC4rSXXWyznsLhVZ4Hw6rCytcPxATvXYKE4vX1sRQGzHSXx8O+JiH546Ruz/J PK/V7lw+NcvBjZ5iTs5MUf/HnUqVc7N42cL1AWOMj7CZUeYMhHYb5X8N3l+L+zeZ fe187KvP55T9xGStx95xCCe1l53pODxflOxc6Ws9Gu7ND5b7XgVZVLrWOwmh8VUP QsGjk7iDX0ndto58VALw+FiDBWkrrxrqiLQ7RTuPWz8vuiso7kXEczpn7l9qRpm0 IBmEuxz8uPTji4vuEavzTgflMsrDekupDbbyILE+SsfdrwtVPzvPQTwz9+2KiD9N CdOAhgH6rrmwReGyIEC+JkfHPJDNFNiJqkKhEZbyffbMmov/QITnTnHxitel/o8+ w9Fkham4Vz6ijlGpgaApQYdU89CTSU3UDVR8IpmmOxzruYmLCtxisTJjW56LqadN t4wphxHNYk7Ah63glV2JiTsEGET7GA6fCPb1cOK/jkcWrdegZe7XmZo0SaxFj6EP uuHiB18RlV/0QkVsVQrV34y9d7nlX57keX5aqq//SE6lMEwtItM= =g65e -----END PGP SIGNATURE----- --7naecYyHiRgSc5dl--