[PATCH] arp, tap, util: Don't use perror() after seccomp filter is installed

[PATCH] arp, tap, util: Don't use perror() after seccomp filter is installed

[Stefano Brivio]

If stderr is closed, after we fork to background, glibc's
implementation of perror() will try to re-open it by calling dup(),
upon which the seccomp filter causes the process to terminate,
because dup() is not included in the list of allowed syscalls.

Replace perror() calls that might happen after isolation_postfork().
We could probably replace all of them, but early ones need a bit more
attention as we have to check whether log.c functions work in early
stages.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 arp.c  | 6 ++++--
 tap.c  | 6 +++---
 util.c | 6 +++---
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arp.c b/arp.c
index 141d43f..930b9ea 100644
--- a/arp.c
+++ b/arp.c
@@ -24,6 +24,7 @@
 #include <string.h>
 
 #include "util.h"
+#include "log.h"
 #include "arp.h"
 #include "dhcp.h"
 #include "passt.h"
@@ -43,6 +44,7 @@ int arp(const struct ctx *c, const struct pool *p)
 	struct arphdr *ah;
 	struct arpmsg *am;
 	size_t len;
+	int ret;
 
 	eh = packet_get(p, 0, 0,			 sizeof(*eh), NULL);
 	ah = packet_get(p, 0, sizeof(*eh),		 sizeof(*ah), NULL);
@@ -81,8 +83,8 @@ int arp(const struct ctx *c, const struct pool *p)
 	memcpy(eh->h_dest,	eh->h_source,	sizeof(eh->h_dest));
 	memcpy(eh->h_source,	c->mac,		sizeof(eh->h_source));
 
-	if (tap_send(c, eh, len) < 0)
-		perror("ARP: send");
+	if ((ret = tap_send(c, eh, len)) < 0)
+		warn("ARP: send: %s", strerror(ret));
 
 	return 1;
 }
diff --git a/tap.c b/tap.c
index abeff25..b7ac996 100644
--- a/tap.c
+++ b/tap.c
@@ -899,7 +899,7 @@ static void tap_sock_unix_init(struct ctx *c)
 	int i;
 
 	if (fd < 0) {
-		perror("UNIX socket");
+		err("UNIX socket: %s", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
 
@@ -920,7 +920,7 @@ static void tap_sock_unix_init(struct ctx *c)
 
 		ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
 		if (ex < 0) {
-			perror("UNIX domain socket check");
+			err("UNIX domain socket check: %s", strerror(errno));
 			exit(EXIT_FAILURE);
 		}
 
@@ -944,7 +944,7 @@ static void tap_sock_unix_init(struct ctx *c)
 	}
 
 	if (i == UNIX_SOCK_MAX) {
-		perror("UNIX socket bind");
+		err("UNIX socket bind: %s", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
 
diff --git a/util.c b/util.c
index 514bd44..be102e3 100644
--- a/util.c
+++ b/util.c
@@ -125,7 +125,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 		fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
 
 	if (fd < 0) {
-		perror("L4 socket");
+		warn("L4 socket: %s", strerror(errno));
 		return -1;
 	}
 
@@ -193,7 +193,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 	}
 
 	if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
-		perror("TCP socket listen");
+		warn("TCP socket listen: %s", strerror(errno));
 		close(fd);
 		return -1;
 	}
@@ -201,7 +201,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 	ev.events = EPOLLIN;
 	ev.data.u64 = ref.u64;
 	if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
-		perror("L4 epoll_ctl");
+		warn("L4 epoll_ctl: %s", strerror(errno));
 		return -1;
 	}
 
-- 
@@ -125,7 +125,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 		fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
 
 	if (fd < 0) {
-		perror("L4 socket");
+		warn("L4 socket: %s", strerror(errno));
 		return -1;
 	}
 
@@ -193,7 +193,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 	}
 
 	if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
-		perror("TCP socket listen");
+		warn("TCP socket listen: %s", strerror(errno));
 		close(fd);
 		return -1;
 	}
@@ -201,7 +201,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
 	ev.events = EPOLLIN;
 	ev.data.u64 = ref.u64;
 	if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
-		perror("L4 epoll_ctl");
+		warn("L4 epoll_ctl: %s", strerror(errno));
 		return -1;
 	}
 
-- 
2.35.1

Re: [PATCH] arp, tap, util: Don't use perror() after seccomp filter is installed

[David Gibson]

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

On Tue, Nov 15, 2022 at 02:24:00AM +0100, Stefano Brivio wrote:
> If stderr is closed, after we fork to background, glibc's
> implementation of perror() will try to re-open it by calling dup(),
> upon which the seccomp filter causes the process to terminate,
> because dup() is not included in the list of allowed syscalls.
> 
> Replace perror() calls that might happen after isolation_postfork().
> We could probably replace all of them, but early ones need a bit more
> attention as we have to check whether log.c functions work in early
> stages.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  arp.c  | 6 ++++--
>  tap.c  | 6 +++---
>  util.c | 6 +++---
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arp.c b/arp.c
> index 141d43f..930b9ea 100644
> --- a/arp.c
> +++ b/arp.c
> @@ -24,6 +24,7 @@
>  #include <string.h>
>  
>  #include "util.h"
> +#include "log.h"
>  #include "arp.h"
>  #include "dhcp.h"
>  #include "passt.h"
> @@ -43,6 +44,7 @@ int arp(const struct ctx *c, const struct pool *p)
>  	struct arphdr *ah;
>  	struct arpmsg *am;
>  	size_t len;
> +	int ret;
>  
>  	eh = packet_get(p, 0, 0,			 sizeof(*eh), NULL);
>  	ah = packet_get(p, 0, sizeof(*eh),		 sizeof(*ah), NULL);
> @@ -81,8 +83,8 @@ int arp(const struct ctx *c, const struct pool *p)
>  	memcpy(eh->h_dest,	eh->h_source,	sizeof(eh->h_dest));
>  	memcpy(eh->h_source,	c->mac,		sizeof(eh->h_source));
>  
> -	if (tap_send(c, eh, len) < 0)
> -		perror("ARP: send");
> +	if ((ret = tap_send(c, eh, len)) < 0)
> +		warn("ARP: send: %s", strerror(ret));
>  
>  	return 1;
>  }
> diff --git a/tap.c b/tap.c
> index abeff25..b7ac996 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -899,7 +899,7 @@ static void tap_sock_unix_init(struct ctx *c)
>  	int i;
>  
>  	if (fd < 0) {
> -		perror("UNIX socket");
> +		err("UNIX socket: %s", strerror(errno));
>  		exit(EXIT_FAILURE);
>  	}
>  
> @@ -920,7 +920,7 @@ static void tap_sock_unix_init(struct ctx *c)
>  
>  		ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
>  		if (ex < 0) {
> -			perror("UNIX domain socket check");
> +			err("UNIX domain socket check: %s", strerror(errno));
>  			exit(EXIT_FAILURE);
>  		}
>  
> @@ -944,7 +944,7 @@ static void tap_sock_unix_init(struct ctx *c)
>  	}
>  
>  	if (i == UNIX_SOCK_MAX) {
> -		perror("UNIX socket bind");
> +		err("UNIX socket bind: %s", strerror(errno));
>  		exit(EXIT_FAILURE);
>  	}
>  
> diff --git a/util.c b/util.c
> index 514bd44..be102e3 100644
> --- a/util.c
> +++ b/util.c
> @@ -125,7 +125,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
>  		fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto);
>  
>  	if (fd < 0) {
> -		perror("L4 socket");
> +		warn("L4 socket: %s", strerror(errno));
>  		return -1;
>  	}
>  
> @@ -193,7 +193,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
>  	}
>  
>  	if (proto == IPPROTO_TCP && listen(fd, 128) < 0) {
> -		perror("TCP socket listen");
> +		warn("TCP socket listen: %s", strerror(errno));
>  		close(fd);
>  		return -1;
>  	}
> @@ -201,7 +201,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,
>  	ev.events = EPOLLIN;
>  	ev.data.u64 = ref.u64;
>  	if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) {
> -		perror("L4 epoll_ctl");
> +		warn("L4 epoll_ctl: %s", strerror(errno));
>  		return -1;
>  	}
>  

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