From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 3737D5A026F for ; Wed, 27 Dec 2023 21:25:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1703708712; 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=7kxk/DtjTcNdEDfJJPcjqXnRY8mPxxGgUF7z0am7dJU=; b=JUY89gW26infEu4Ki/voBnWPoLn0gQJl0dxLCa/CoH3qJsWk+ZyqrGLhXVQPOxHIY1yuwn ydLo9ofR7OtdPIFDZiHfxUVzr2XnX+hHY19M1Hmi6pSmRFx2CoABZtiT5G1DIVhLgdSAY9 +NNy4tTkzGJ1nHQ23bAXt/p6jZYWXZ4= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-160-Opv6TTtPNFirzz2sOn1v-w-1; Wed, 27 Dec 2023 15:25:10 -0500 X-MC-Unique: Opv6TTtPNFirzz2sOn1v-w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EAF261C04B5A; Wed, 27 Dec 2023 20:25:09 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 234061C060AF; Wed, 27 Dec 2023 20:25:08 +0000 (UTC) Date: Wed, 27 Dec 2023 21:25:06 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 1/8] tcp: Fix address type for tcp_sock_init_af() Message-ID: <20231227212506.75eb41c7@elisabeth> In-Reply-To: <20231207143140.1851378-2-david@gibson.dropbear.id.au> References: <20231207143140.1851378-1-david@gibson.dropbear.id.au> <20231207143140.1851378-2-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: Z7PX4ZKJXPIJF564U6JT3SSLHLVZO4P6 X-Message-ID-Hash: Z7PX4ZKJXPIJF564U6JT3SSLHLVZO4P6 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, 8 Dec 2023 01:31:33 +1100 David Gibson wrote: > This takes a struct in_addr * (i.e. an IPv4 address), although it's > explicitly supposed to handle IPv6 as well. Both its caller and sock_l4() > which it calls use a void * for the address, which can be either an in_addr > or an in6_addr. > > We get away with this, because we don't do anything with the pointer other > than transfer it from the caller to sock_l4(), but it's misleading. And > quite possibly technically UB, because C is like that. > > Signed-off-by: David Gibson > --- > tcp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tcp.c b/tcp.c > index f506cfd..bda95b2 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -2905,7 +2905,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events) > * Return: fd for the new listening socket, negative error code on failure > */ > static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port, > - const struct in_addr *addr, const char *ifname) > + const void *addr, const char *ifname) This is obviously correct. However, after a lot of thinking: (gcc) optimisations based on Type-Based Alias Analysis, which we don't disable on this path, could, now, happily defer filling 'addr' with inet_pton() in conf_ports() to a point *after* the tcp_sock_init() call. Without this patch, at least 32 bits must be updated before the call. It might sound like a joke because... it actually is. But look at what we had to do for the functions in checksum.c. We pass const void *buf, and anything that buf points to can be updated (with TBAA) after the call. I don't see any conceptual difference between this case and those functions. Anyway, that won't reasonably happen here, and in any case this would have been broken for IPv6, so I'll go ahead and apply this. But, eventually, I think we should switch all these usages to union inany_addr *. -- Stefano