From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dkim=fail reason="key not found in DNS" header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202312 header.b=fwuo6Z4W; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C75B35A0275 for ; Wed, 14 Aug 2024 12:03:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1723629814; bh=YKlXDVyH9s/oiis4Ws093jaSAJu/e+JTg4XfB8e7vtk=; h=From:To:Cc:Subject:Date:From; b=fwuo6Z4W+jCgOn/uYnNi6uW5P0Y2RTMpVe9HvJjmT/t3XDnWBfXUNhbDh6UOhABQR NbjCYyQrHDIzIrFdPmG/QBnM8/q1MbfQMZ9SaUEGC1G1SQwMg47PdwRQ9xVkShjjF3 K6DYIdn6BWipoW9TAh/GNJCrLQx3PQ39g66k8pwG9+UbChQe7R7kTI9aEIiYxorXEy yNVf2XxVtel3LbBMouL4Jaz1Gob/maOzdzWgNcxHbq3OEhvnxhsfLen88T+NQn/Da/ RY7rwMGVHonbte1iVU+YT4NPWjZXFLbD1HMt2rgzfOtuq1IuxnD9Bq/B1e9Mgiz7yL LZOUlcs7BNxUw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WkP3B4pTsz4x5G; Wed, 14 Aug 2024 20:03:34 +1000 (AEST) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH] flow: Don't crash if guest attempts to connect to port 0 Date: Wed, 14 Aug 2024 20:03:33 +1000 Message-ID: <20240814100333.706977-1-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.46.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 642NXVURSO2IVHQXB4RTPOA5L26LVETY X-Message-ID-Hash: 642NXVURSO2IVHQXB4RTPOA5L26LVETY 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: Matt Hamilton , David Gibson 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: Using a zero port on TCP or UDP is dubious, and we can't really deal with forwarding such a flow within the constraints of the socket API. Hence we ASSERT()ed that we had non-zero ports in flow_hash(). The intention was to make sure that the protocol code sanitizes such ports before completing a flow entry. Unfortunately, flow_hash() is also called on new packets to see if they have an existing flow, so the unsanitized guest packet can crash passt with the assert. Correct this by moving the assert from flow_hash() to flow_sidx_hash() which is only used on entries already in the table, not on unsanitized data. Signed-off-by: David Gibson --- flow.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/flow.c b/flow.c index 687e9fd0..93b687dc 100644 --- a/flow.c +++ b/flow.c @@ -561,12 +561,6 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif, { struct siphash_state state = SIPHASH_INIT(c->hash_secret); - /* For the hash table to work, we need complete endpoint information, - * and at least a forwarding port. - */ - ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) && - side->eport != 0 && side->fport != 0); - inany_siphash_feed(&state, &side->faddr); inany_siphash_feed(&state, &side->eaddr); @@ -586,8 +580,16 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif, static uint64_t flow_sidx_hash(const struct ctx *c, flow_sidx_t sidx) { const struct flow_common *f = &flow_at_sidx(sidx)->f; - return flow_hash(c, FLOW_PROTO(f), - f->pif[sidx.sidei], &f->side[sidx.sidei]); + const struct flowside *side = &f->side[sidx.sidei]; + uint8_t pif = f->pif[sidx.sidei]; + + /* For the hash table to work, entries must have complete endpoint + * information, and at least a forwarding port. + */ + ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) && + side->eport != 0 && side->fport != 0); + + return flow_hash(c, FLOW_PROTO(f), pif, side); } /** -- 2.46.0