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

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:

...

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