[PATCH] tcp, udp: Don't initialise IPv6/IPv4 sockets if IPv4/IPv6 are not enabled

[PATCH] tcp, udp: Don't initialise IPv6/IPv4 sockets if IPv4/IPv6 are not enabled

[Stefano Brivio]

If we disable a given IP version automatically (no corresponding
default route on host) or administratively (--ipv4-only or
--ipv6-only options), we don't initialise related buffers and
services (DHCP for IPv4, NDP and DHCPv6 for IPv6). The "tap"
handlers will also ignore packets with a disabled IP version.

However, in commit 3c6ae625101a ("conf, tcp, udp: Allow address
specification for forwarded ports") I happily changed socket
initialisation functions to take AF_UNSPEC meaning "any enabled
IP version", but I forgot to add checks back for the "enabled"
part.

Reported by Paul: on a host without default IPv6 route, but IPv6
enabled, connect, using IPv6, to a port handled by pasta, which
tries to send data to a tap device without initialised buffers
for that IP version and exits because the resulting write() fails.

Simpler way to reproduce: pasta -6 and inbound IPv4 connection, or
pasta -4 and inbound IPv6 connection.

Reported-by: Paul Holzinger <pholzing@redhat.com>
Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tcp.c | 4 ++--
 udp.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tcp.c b/tcp.c
index 713248f..d043123 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3213,9 +3213,9 @@ static void tcp_sock_init6(const struct ctx *c, int ns,
 void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 		   const void *addr, const char *ifname, in_port_t port)
 {
-	if (af == AF_INET || af == AF_UNSPEC)
+	if ((af == AF_INET  || af == AF_UNSPEC) && c->ifi4)
 		tcp_sock_init4(c, ns, addr, ifname, port);
-	if (af == AF_INET6 || af == AF_UNSPEC)
+	if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
 		tcp_sock_init6(c, ns, addr, ifname, port);
 }
 
diff --git a/udp.c b/udp.c
index 42a17a7..ff7f993 100644
--- a/udp.c
+++ b/udp.c
@@ -1129,7 +1129,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 					    c->udp.fwd_in.f.delta[port]);
 	}
 
-	if (af == AF_INET || af == AF_UNSPEC) {
+	if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
 		if (!addr && c->mode == MODE_PASTA)
 			bind_addr = &c->ip4.addr;
 		else
@@ -1162,7 +1162,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 		}
 	}
 
-	if (af == AF_INET6 || af == AF_UNSPEC) {
+	if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
 		if (!addr && c->mode == MODE_PASTA)
 			bind_addr = &c->ip6.addr;
 		else
-- 
@@ -1129,7 +1129,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 					    c->udp.fwd_in.f.delta[port]);
 	}
 
-	if (af == AF_INET || af == AF_UNSPEC) {
+	if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
 		if (!addr && c->mode == MODE_PASTA)
 			bind_addr = &c->ip4.addr;
 		else
@@ -1162,7 +1162,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 		}
 	}
 
-	if (af == AF_INET6 || af == AF_UNSPEC) {
+	if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
 		if (!addr && c->mode == MODE_PASTA)
 			bind_addr = &c->ip6.addr;
 		else
-- 
2.35.1

Re: [PATCH] tcp, udp: Don't initialise IPv6/IPv4 sockets if IPv4/IPv6 are not enabled

[David Gibson]

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

On Wed, Nov 09, 2022 at 06:38:08PM +0100, Stefano Brivio wrote:
> If we disable a given IP version automatically (no corresponding
> default route on host) or administratively (--ipv4-only or
> --ipv6-only options), we don't initialise related buffers and
> services (DHCP for IPv4, NDP and DHCPv6 for IPv6). The "tap"
> handlers will also ignore packets with a disabled IP version.
> 
> However, in commit 3c6ae625101a ("conf, tcp, udp: Allow address
> specification for forwarded ports") I happily changed socket
> initialisation functions to take AF_UNSPEC meaning "any enabled
> IP version", but I forgot to add checks back for the "enabled"
> part.
> 
> Reported by Paul: on a host without default IPv6 route, but IPv6
> enabled, connect, using IPv6, to a port handled by pasta, which
> tries to send data to a tap device without initialised buffers
> for that IP version and exits because the resulting write() fails.
> 
> Simpler way to reproduce: pasta -6 and inbound IPv4 connection, or
> pasta -4 and inbound IPv6 connection.
> 
> Reported-by: Paul Holzinger <pholzing@redhat.com>
> Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Heh, I also noticed this while working on the dual stack socket stuff,
but didn't get around to fixing it yet.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>


> ---
>  tcp.c | 4 ++--
>  udp.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 713248f..d043123 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -3213,9 +3213,9 @@ static void tcp_sock_init6(const struct ctx *c, int ns,
>  void tcp_sock_init(const struct ctx *c, int ns, sa_family_t af,
>  		   const void *addr, const char *ifname, in_port_t port)
>  {
> -	if (af == AF_INET || af == AF_UNSPEC)
> +	if ((af == AF_INET  || af == AF_UNSPEC) && c->ifi4)
>  		tcp_sock_init4(c, ns, addr, ifname, port);
> -	if (af == AF_INET6 || af == AF_UNSPEC)
> +	if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
>  		tcp_sock_init6(c, ns, addr, ifname, port);
>  }
>  
> diff --git a/udp.c b/udp.c
> index 42a17a7..ff7f993 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -1129,7 +1129,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
>  					    c->udp.fwd_in.f.delta[port]);
>  	}
>  
> -	if (af == AF_INET || af == AF_UNSPEC) {
> +	if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
>  		if (!addr && c->mode == MODE_PASTA)
>  			bind_addr = &c->ip4.addr;
>  		else
> @@ -1162,7 +1162,7 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
>  		}
>  	}
>  
> -	if (af == AF_INET6 || af == AF_UNSPEC) {
> +	if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
>  		if (!addr && c->mode == MODE_PASTA)
>  			bind_addr = &c->ip6.addr;
>  		else

-- 
David Gibson			| 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 --]