Re: [PATCH] tcp: Avoid division by zero with small MSS values

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

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

On Wed, Oct 21, 2026 at 10:48:09AM +0200, Stefano Brivio wrote:
> On Wed, 21 Oct 2026 12:38:42 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Oct 21, 2026 at 02:11:13AM +0200, Stefano Brivio wrote:
> > > On Fri, 16 Oct 2026 16:58:11 +0530
> > > Anshu Kumari <anskuma@redhat.com> wrote:
> > >   
> > > > Fuzz testing with AFL++ persistent mode found a crash  
> > > 
> > > Yay, that's quite an achievement!
> > >   
> > > > in
> > > > tcp_buf_data_from_sock(), SIGFPE (signal 8) from division
> > > > by zero at:
> > > > 
> > > >   fill_bufs = DIV_ROUND_UP(wnd_scaled - already_sent, mss);
> > > > 
> > > > when we split buffers on MSS boundaries, if the MSS is small
> > > > enough (less than four, rounded down to 0).
> > > > 
> > > > 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:
> > > > 
> > > >   - Fix MSS_SET() to round up when MSS is between 1 and 3,
> > > >     storing tap_mss as 1 (recovered as 4 by MSS_GET()).  
> > > 
> > > This introduces a subtle problem though: if the MSS is between 1 and 3
> > > (silly but I guess legitimate), we would overflow it now.
> > > 
> > > I think we should rather adjust MSS_GET() as well, to always use 1 for
> > > values 0 < x < 4.
> > >   
> > > >   - In case of zero MSS, don't send data: tcp_buf_data_from_sock()
> > > >     now returns early.
> > > > 
> > > > 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.  
> > > 
> > > 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.
> 
> 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:
> 
> 1. from a use case perspective:
> 
>   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)
> 
>   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
> 
> 2. from a RFC perspective:
> 
>   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)."
> 
>      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
> 
>   b. again from RFC 9293, 3.5.2:
> 
>      "A reset must not be sent if it is not clear that this is the case."
> 
>      This requirement seems to be overridden here and there in the same
>      RFC, but in very specific ways.
> 
> From this we concluded that it's not really compliant to reset the
> connection in this case.

Ah, good points.

> 
> 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:
> 
>   https://unix.stackexchange.com/questions/22410/minimal-tcp-mss-in-linux
> 
> 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.
> 
> 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).
> 
> 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.

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