Re: [PATCH v3 7/8] tcp: make tcp_rst_no_conn() respond with correct MAC address

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3008 bytes --]

On Sun, Jun 29, 2025 at 01:13:46PM -0400, Jon Maloy wrote:
> tcp_rst_no_conn() needs to identify and specify which source MAC
> address to use when sending an RST to the guest. This is because
> it doesn't have access to any flow structure where this address
> could be fetched.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v3: - Adapted to the signature change in nl_mac_get() function, so that
>       we now consider only the template interface when checking the
>       ARP/NDP table.
> ---
>  tcp.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 3ecf9e8..8c502ea 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -309,6 +309,7 @@
>  #include "tcp_internal.h"
>  #include "tcp_buf.h"
>  #include "tcp_vu.h"
> +#include "netlink.h"
>  
>  #ifndef __USE_MISC
>  /* From Linux UAPI, missing in netinet/tcp.h provided by musl */
> @@ -1888,17 +1889,29 @@ static void tcp_rst_no_conn(const struct ctx *c, int af,
>  			    const struct tcphdr *th, size_t l4len)
>  {
>  	struct iov_tail payload = IOV_TAIL(NULL, 0, 0);
> +	unsigned char src_mac[ETH_ALEN];
> +	union inany_addr tgt;
>  	struct tcphdr *rsth;
>  	char buf[USHRT_MAX];
>  	uint32_t psum = 0;
>  	size_t rst_l2len;
> +	int ifi;
>  
>  	/* Don't respond to RSTs without a connection */
>  	if (th->rst)
>  		return;
>  
> +	/* Respond with true MAC address if remote host is on
> +	 * the template interface's network segment
> +	 */
> +	ifi = af == AF_INET ? c->ifi4 : c->ifi6;
> +	memcpy(src_mac, c->our_tap_mac, ETH_ALEN);
> +	inany_from_af(&tgt, af, daddr);
> +	if (!inany_nat(c, &tgt))
> +		nl_mac_get(nl_sock, &tgt, ifi, src_mac);

As with all these cases, this will fail if it's the first time we've
contacted the peer.  That's probably harmless in this case... but
similarly it's also probably harmless to always use the standard MAC
like we were before.  If we get here, we've basically got a garbage
packet from the guest - does it really need to know the real MAC of
the host it might have been contacting if things were in a much saner
state than they apparently are.

> +
>  	if (af == AF_INET) {
> -		struct iphdr *ip4h = tap_push_l2h(c, buf, NULL, ETH_P_IP);
> +		struct iphdr *ip4h = tap_push_l2h(c, buf, src_mac, ETH_P_IP);
>  		const struct in_addr *rst_src = daddr;
>  		const struct in_addr *rst_dst = saddr;
>  
> @@ -1908,7 +1921,7 @@ static void tcp_rst_no_conn(const struct ctx *c, int af,
>  					      *rst_src, *rst_dst);
>  
>  	} else {
> -		struct ipv6hdr *ip6h = tap_push_l2h(c, buf, NULL, ETH_P_IPV6);
> +		struct ipv6hdr *ip6h = tap_push_l2h(c, buf, src_mac, ETH_P_IPV6);
>  		const struct in6_addr *rst_src = daddr;
>  		const struct in6_addr *rst_dst = saddr;
>  

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]