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 (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E935E5A0274 for ; Fri, 23 Oct 2026 05:28:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202610; t=1792726129; bh=8Dhv8I3yGw2I0FPKzpz3VdH73rKDyVTzKD4eYOSLdMw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=W9eY4sEy0PYB6VxEe3PW2Z49bt7ONk9CjwN9/AdPhuRHkdALDhqxbnjwicwp30oPR 6DtUNji3xpMkiINyY9iJY4t/SCjEo1ktibq8yDSpOlMYB06rKsTFrYzOpocKC22sZK VLn+DyhnsQC4Yy82Ddsw8P9LtGzkdCV4ylKMJPKVJNO+7P7XawZZpr/ziqTX8uOnBv 0icmi89l07TabbiCIJ8lYKE2ycqGprlK+Pg3MUSivTR5t1EYve4zFaYTl3rXpCevIp +vjIyHzgqY/SsmNjmCtUiKw3vqxGdhSKXo3CFxKsYeghVskh8l1RYj9IgZqcBRHEKr u1bkdVmBaH3Hg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4j9pQT0HwLz4w9g; Fri, 23 Oct 2026 14:28:49 +1100 (AEDT) Date: Fri, 23 Oct 2026 14:28:41 +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> <20261021104809.0db3d744@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bvWdKfUiOSNEIIEt" Content-Disposition: inline In-Reply-To: <20261021104809.0db3d744@elisabeth> Message-ID-Hash: INNTHBBWTAKIGIZRG2P2ZLK2FO2G6WXO X-Message-ID-Hash: INNTHBBWTAKIGIZRG2P2ZLK2FO2G6WXO 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: --bvWdKfUiOSNEIIEt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 21, 2026 at 10:48:09AM +0200, Stefano Brivio wrote: > On Wed, 21 Oct 2026 12:38:42 +1100 > David Gibson wrote: >=20 > > 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 > > >=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 > > >=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 > > >=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). =20 > >=20 > > 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. >=20 > We pondered with Anshu about that for a long while (and a summary of > this should probably be part of the commit message): it was actually > my favourite solution when we started looking into it, but then > realised that: >=20 > 1. from a use case perspective: >=20 > a. very low MSSes are not uncommon in microcontroller applications > (let's say you have just 100-200 bytes available to process at a > given time, and short commands, or want the peer to split messages > / commands for you onto specific boundaries) >=20 > b. what about 0 though? It might be a "probe" connection, and that > would be legitimate (we looked into all possible RFCs and couldn't > find a single sentence preventing that). It might also be meant as > a unidirectional connection >=20 > 2. from a RFC perspective: >=20 > a. from RFC 9293, section 3.1: "TCP implementations MUST be prepared > to handle an illegal option length (e.g., zero); a suggested > procedure is to reset the connection and log the error cause > (MUST-7)." >=20 > But this is just about illegal option *lengths*, there's no such > thing as an illegal option *value*. And not even RFC 9293 sets a > minimum legal or expected value for the MSS >=20 > b. again from RFC 9293, 3.5.2: >=20 > "A reset must not be sent if it is not clear that this is the case." >=20 > This requirement seems to be overridden here and there in the same > RFC, but in very specific ways. >=20 > From this we concluded that it's not really compliant to reset the > connection in this case. Ah, good points. >=20 > The next concern was going for transparency as much as we could: what's > the minimum for TCP_MAXSEG we can advertise to the peer? It was January > 2000, and the definition of TCP_MIN_MSS as 88 bytes appeared in Linux > 2.3.41pre2. We looked up related discussions but couldn't find anything, > except for: >=20 > https://unix.stackexchange.com/questions/22410/minimal-tcp-mss-in-linux >=20 > and there's a rather insightful answer there, based on the _maximum_ > possible size of headers. On the other hand, we read and read, and > nowhere it's written that all IP implementations must support the > maximum size for TCP headers. If a segment is too big, it can be > dropped. >=20 > So, good, we can't reflect this 0 MSS to the peer. The peer might send > us data. What can we do at this point? The best we could come up with > is: pile it up. Don't send it (because we would overflow a 0 MSS if we > send anything). >=20 > The sender will figure it out. If it wasn't meant to be that way, they > will reset the connection, but we don't want to take that > responsibility. Right. I mean, passt already decouples the MSS from one side to the other, so that we can take advantage of the large guest-side MTU. Given that, it seems like it's an already established break from transparency. --=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 --bvWdKfUiOSNEIIEt Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmra1GQACgkQzQJF27ox 2GeaXA//YIM/R5yaPCFzAea4Z/X124oOFXhLOG8W72spgU/X79GOeqrs20zt7CaT ScA2xjMwhfjulLywsCYUUxiLJO+VrUJUSYzhao7d1gMwMePMJiYnA9F5/FSk0YLO 0/n/wsJUCEMcp4ugTfGJKlF99mhkRHC1HeaCgSLvkMbqlph7jDvtsR7+mIbF2kSP oQ9ddHVzPJH3U/gtqj07Z3E2je4dwd2m1UeFs8vLtcmna/Fnfwu45B4vzLVP1J0k wh/N7Y+9X+Vbvw4FdtCrq7CzHG/kgLIg44k/gsBtJVvsxIBVYx1ew3ucG81m66Ru hj1ynWKOT5UUI/xJkEKHZD0QxZiUDrNvoblryIUQS/o9dnDhrFWZz6ckGj1Dod2O hpPCfFENvLT+wzCEBJWzTvQFP6HBb/sKqIZlDG11D6bfZjl2jPKjZyzPQW2HMLPH HF4NeerYhqfyrmqhKAj9dcLGBXAdOQrB+3wTaOvBZoyDZTq+RYjQs7TlEwDi453F GMpAtv9BpiPFMFQlDDyWZrH3j+SGrLQ5lZpMf1q7VoNKjUw29Bn5YKz3+FV0Aqy+ M1QMSklyLtUNaFfMSjLckOpHMoHCP6qA3crhzJFD8TTzQbAoteXEhUrBYYxCwp1i PSdB+cthFsWMpSeBP7GnJH/AdhJr+PqbJ8S2rnFx8kPXUC9QX4Y= =VleO -----END PGP SIGNATURE----- --bvWdKfUiOSNEIIEt--