From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 2FF6B5A028B for ; Thu, 17 Nov 2022 06:59:22 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4NCTkl4TBFz4xyL; Thu, 17 Nov 2022 16:59:11 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1668664751; bh=Mh+oxFINjiVfgHz/EEFtRLddRC1vsSrg6DvHQRfRhNI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=APt0ulMBDFkMtSnekxEa+6JETUrUXlKFB6Hdyq5Yu9qxI5CSUQ1UD8EuyzkgCtW2L v7kubwiLy3Qy75qfWE9p/9V2QctBm7AqmEGLc+WuigQYKfHa777PvY7WPdBCaqdkTK J60quBKg4OZa3unyyiDY/sB6VmOgKVSygVKzv6Ds= From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v2 25/32] tcp: Fix small errors in tcp_seq_init() time handling Date: Thu, 17 Nov 2022 16:59:01 +1100 Message-Id: <20221117055908.2782981-26-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221117055908.2782981-1-david@gibson.dropbear.id.au> References: <20221117055908.2782981-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: IYINJUJ6UM4QGIV4IOGKGKFVZJS3F24A X-Message-ID-Hash: IYINJUJ6UM4QGIV4IOGKGKFVZJS3F24A X-MailFrom: dgibson@gandalf.ozlabs.org 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: David Gibson X-Mailman-Version: 3.3.3 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: It looks like tcp_seq_init() is supposed to advance the sequence number by one every 32ns. However we only right shift the ns part of the timespec not the seconds part, meaning that we'll advance by an extra 32 steps on each second. I don't know if that's exploitable in any way, but it doesn't appear to be the intent, nor what RFC 6528 suggests. In addition, we convert from seconds to nanoseconds with a multiplication by '1E9'. In C '1E9' is a floating point constant, forcing a conversion to floating point and back for what should be an integer calculation (confirmed with objdump and Makefile default compiler flags). Spell out 1000000000 in full to avoid that. Signed-off-by: David Gibson --- tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index dabcbc4..559e271 100644 --- a/tcp.c +++ b/tcp.c @@ -1957,8 +1957,8 @@ static void tcp_seq_init(const struct ctx *c, struct tcp_tap_conn *conn, seq = siphash_36b((uint8_t *)&in, c->tcp.hash_secret); - ns = now->tv_sec * 1E9; - ns += now->tv_nsec >> 5; /* 32ns ticks, overflows 32 bits every 137s */ + /* 32ns ticks, overflows 32 bits every 137s */ + ns = (now->tv_sec * 1000000000 + now->tv_nsec) >> 5; conn->seq_to_tap = seq + ns; } -- 2.38.1