Fuzz testing with AFL++ persistent mode found that tcp_buf_data_from_sock() terminates the process with SIGFPE (signal 8) from division by zero at: fill_bufs = DIV_ROUND_UP(wnd_scaled - already_sent, mss); when MSS is small enough (less than four) to round down to 0 during the bit-shift in MSS_SET(). The termination itself is clean (not memory corruption or data leakage), but we can handle this condition more gracefully. We considered resetting connections with a zero or very low MSS, but concluded this is not RFC-compliant: RFC 9293, section 3.1 (MUST-7), states: "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." However, this applies only to option *lengths*, not *values* — no standard RFC defines a minimum MSS value. Section 3.5.2 further states: "A reset must not be sent if it is not clear that this is the case." Very low MSS values also have legitimate uses in microcontroller applications with limited buffers, and a zero MSS could represent a unidirectional connection. Rounding a low MSS upward is also not an option: emitting segments larger than the peer advertised violates the effective send MSS requirement in RFC 9293, section 3.7.1. Instead, preserve the actual MSS: - Adjust MSS_SET() to clamp non-zero values below 4 to 1, avoiding the round-to-zero in bit-field storage. - Adjust MSS_GET() to return 1 when tap_mss is 1, so we never inflate the MSS beyond what the peer advertised. - When MSS is zero, don't send data: tcp_buf_data_from_sock() returns early. The peer will eventually determine the connection is unusable and reset it. Signed-off-by: Anshu Kumari --- v2: - added comment for both MSS_SET() and MSS_GET() macro. - fixed/modified MSS_GET(). - fixed commit message by explaining why we are not resetting the connection. NOTE: let the fuzzer run for more than a day with this fix but till now, haven't found any issue. --- tcp_buf.c | 3 +++ tcp_conn.h | 10 ++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) 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..3173ddb9 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -51,8 +51,14 @@ 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_GET(conn) (conn->tap_mss << (16 - TCP_MSS_BITS)) + + /* Clamp non-zero MSS < 4 to 1 to avoid rounding to zero */ +#define MSS_SET(conn, mss) \ + (conn->tap_mss = (mss) ? MAX(1, (mss) >> (16 - TCP_MSS_BITS)) : 0) + + /* tap_mss is stored as 1 for any non-zero MSS value <= 4 */ +#define MSS_GET(conn) \ + ((conn->tap_mss == 1) ? 1 : (conn->tap_mss << (16 - TCP_MSS_BITS))) #define RTT_EXP_BITS 4 unsigned int rtt_exp :RTT_EXP_BITS; -- 2.55.0