From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=fail (p=quarantine dis=none) header.from=redhat.com Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id B71DC5A0265 for ; Wed, 21 Oct 2026 02:11:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1792541478; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RfP4Ug2ZHQmgKA+uZqtBWoFYAPMldTxHYNtxDvqA/hc=; b=GsLlJ6CuqNAXiSGiZLFpBHy2NqmkXr+b2cfBNLP4B1uIdAeemR9B+XTFgZcoKtjdmHLm3W +rZmJZ3Jub7QGKJFZw1WGU8QtTxPQVAc4xViZzFWu+QjbLS9XbnjqUpjOzMTmZ+PPDFP/C Ej2OKufz0SJ7LsA1Y3KplPLGyj/uCDk= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-651-NTtcNSkPNnGHjAnydrV_kg-1; Wed, 21 Oct 2026 00:11:16 +0000 X-MC-Unique: NTtcNSkPNnGHjAnydrV_kg-1 X-Mimecast-MFC-AGG-ID: NTtcNSkPNnGHjAnydrV_kg_1792541475 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-48c6a1d4034so143399f8f.0 for ; Tue, 20 Oct 2026 17:11:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792541475; x=1793146275; h=date:content-transfer-encoding:content-type:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=RfP4Ug2ZHQmgKA+uZqtBWoFYAPMldTxHYNtxDvqA/hc=; b=so1F84kO9K9m3KvghLa0+tQrUrhNet40fMx5snFDI8MhVh+s4HDV8tccZ7ZZ9J3VTo j2ZeBbtN+XkmPoyf8/gp5qLcW9/vgC+fxKbh6OFPR6t1aRC0P970b+KeI2GiYTaUc+29 /D+4gLYRrnrzpJ7nJA0PZIVGjQA1KvtUvSZZD9Gt38qzytt/M1gTPQU6QgHBhRWFCngH 7jlKIiP3iajHsLYmWK1mkzcNFW0LHmxagefBrjXH5QMa/M8vOabUVWNby85G0Q+mDmxA Sk1pxnKGVJup/NCiAIYDUvIT1KlfKz7iHJFnKCrfKeW96GpvUImv7B0T2SgeL+SjZpUQ 1uUA== X-Gm-Message-State: AFq9FYKb3eGhtEXtYRmO5wnpOexCSs2+lRqBSBWXaQkRE0hb2xgi90k4 Q18FRoFrHtJwLc25i2ysOiAAOSQhAv/G1e4RGGCA5hidd6NeH+gM4Wo0Q2xVRrw2aHtbp1VBdMn +PctboQPkIjv2pQpVAD7ssjgK6HhJgIxkrxur6WaR2W2aBY2FZ7UXFdwDihX18XtNS3SHGLoEeN VkkoxPdKEdxlBmZwsSGZBXZaDxh8X9+1/pRWZF X-Gm-Gg: AYBFou1JyQLYv0VxUxl4Hej3f8mfsKoE8ydg61P1lg+21bndx4FDxYPW35sjKEgYfN8 osJaPpx1G9LkbpgHgiiz07sJIM10PsJrqoZttwBDPMdppZTqLqhXz++M005fqFaibhmsBeBKrh0 oRaWgvjqq01D0Ba70SOEOrFiMjhrx6AUqD+rD5I3I9tW3y5m/MHzXNjL6zsDhb/Ow7hypMwqGVJ 0IGCnA3cQaRJxIeTNELo2q/o43wuv2dlWsByzGCAONWHKlaD0Aakw7e+Q41e8kF0W9+ewghVwsE jr+wxAX6puZB0qvunX5GqKRsmqgiSv3r2rKLv8bJPoJGSmixHmWn4Mze4NqGzmN4w0xsr8NvNa7 FMgrNYYP0+1DPx8F3snPa0V1XDz0= X-Received: by 2002:a05:6000:40cd:b0:48d:db77:d124 with SMTP id ffacd0b85a97d-48e26430969mr731547f8f.6.1792541475414; Tue, 20 Oct 2026 17:11:15 -0700 (PDT) X-Received: by 2002:a05:6000:40cd:b0:48d:db77:d124 with SMTP id ffacd0b85a97d-48e26430969mr731511f8f.6.1792541474935; Tue, 20 Oct 2026 17:11:14 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-48e258213cesm1702214f8f.11.2026.10.20.17.11.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Oct 2026 17:11:13 -0700 (PDT) From: Stefano Brivio To: Anshu Kumari Subject: Re: [PATCH] tcp: Avoid division by zero with small MSS values Message-ID: <20261021021112.385ea997@elisabeth> In-Reply-To: <20261016112811.4176811-1-anskuma@redhat.com> References: <20261016112811.4176811-1-anskuma@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 21 Oct 2026 02:11:13 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: eOpAUkw7WetyUIYmxXQ9qt-flEULTnFK2IpjwPh2T8E_1792541475 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: GFDIRFFLP77I4Y6MEQO7GFOP5XY5K7WA X-Message-ID-Hash: GFDIRFFLP77I4Y6MEQO7GFOP5XY5K7WA X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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