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

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

[Anshu Kumari]

Fuzz testing with AFL++ persistent mode found a crash 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()).
  - 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.

Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
 tcp_buf.c  | 3 +++
 tcp_conn.h | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

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, struct tcp_tap_conn *conn,
 	struct iovec *iov;
 	uint32_t seq;
 
+	if (!mss)
+		return 0;
+
 	/* Set up buffer descriptors we'll fill completely and partially. */
 	fill_bufs = 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 {
 
 #define TCP_MSS_BITS			14
 	unsigned int	tap_mss		:TCP_MSS_BITS;
-#define MSS_SET(conn, mss)	(conn->tap_mss = (mss >> (16 - TCP_MSS_BITS)))
+#define MSS_SET(conn, mss)						\
+	(conn->tap_mss = (mss) ? MAX(1, (mss) >> (16 - TCP_MSS_BITS)) : 0)
 #define MSS_GET(conn)		(conn->tap_mss << (16 - TCP_MSS_BITS))
 
 #define RTT_EXP_BITS			4
-- 
2.55.0

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

[Stefano Brivio]

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

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.

> 
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
>  tcp_buf.c  | 3 +++
>  tcp_conn.h | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> 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, struct tcp_tap_conn *conn,
>  	struct iovec *iov;
>  	uint32_t seq;
>  
> +	if (!mss)
> +		return 0;
> +
>  	/* Set up buffer descriptors we'll fill completely and partially. */
>  	fill_bufs = 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 {
>  
>  #define TCP_MSS_BITS			14
>  	unsigned int	tap_mss		:TCP_MSS_BITS;
> -#define MSS_SET(conn, mss)	(conn->tap_mss = (mss >> (16 - TCP_MSS_BITS)))
> +#define MSS_SET(conn, mss)						\
> +	(conn->tap_mss = (mss) ? MAX(1, (mss) >> (16 - TCP_MSS_BITS)) : 0)

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.

And, I guess (untested):

>  #define MSS_GET(conn)		(conn->tap_mss << (16 - TCP_MSS_BITS))

#define MSS_GET(conn)							\
	((conn->tap_mss == 1) ? 1 : (conn->tap_mss << (16 - TCP_MSS_BITS)))

...with another comment explaining that tap_mss is stored as 1 for
any non-zero MSS value <= 4.

>  
>  #define RTT_EXP_BITS			4

-- 
Stefano

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

[David Gibson]

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

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.

> 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.
> 
> > 
> > Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> > ---
> >  tcp_buf.c  | 3 +++
> >  tcp_conn.h | 3 ++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > 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, struct tcp_tap_conn *conn,
> >  	struct iovec *iov;
> >  	uint32_t seq;
> >  
> > +	if (!mss)
> > +		return 0;
> > +
> >  	/* Set up buffer descriptors we'll fill completely and partially. */
> >  	fill_bufs = 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 {
> >  
> >  #define TCP_MSS_BITS			14
> >  	unsigned int	tap_mss		:TCP_MSS_BITS;
> > -#define MSS_SET(conn, mss)	(conn->tap_mss = (mss >> (16 - TCP_MSS_BITS)))
> > +#define MSS_SET(conn, mss)						\
> > +	(conn->tap_mss = (mss) ? MAX(1, (mss) >> (16 - TCP_MSS_BITS)) : 0)
> 
> 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.
> 
> And, I guess (untested):
> 
> >  #define MSS_GET(conn)		(conn->tap_mss << (16 - TCP_MSS_BITS))
> 
> #define MSS_GET(conn)							\
> 	((conn->tap_mss == 1) ? 1 : (conn->tap_mss << (16 - TCP_MSS_BITS)))
> 
> ...with another comment explaining that tap_mss is stored as 1 for
> any non-zero MSS value <= 4.
> 
> >  
> >  #define RTT_EXP_BITS			4
> 
> -- 
> Stefano
> 

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

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

[Stefano Brivio]

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.

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.

-- 
Stefano