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

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

[Anshu Kumari]

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 <anskuma@redhat.com>
---
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