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

older
[PATCH v2] feat: Pass open files...

Anshu Kumari

16 Oct 2026 16 Oct '26

1:28 p.m.

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

Show replies by date

Stefano Brivio

21 Oct 21 Oct

2:11 a.m.

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

3:38 a.m.

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

Stefano Brivio

10:48 a.m.

On Wed, 21 Oct 2026 12:38:42 +1100 David Gibson 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 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

Age (days ago)

Last active (days ago)

List overview

Download

3 comments

3 participants

participants (3)

Anshu Kumari
David Gibson
Stefano Brivio