- dev - passt

[PATCH] util: In ASSERT(), use "fprintf(stderr, ...)" rather than err(...)
by Laurent Vivier 06 Aug '24

06 Aug '24

We use our own implementation of assert() because the glic implementation uses syscalls that aren't in our seccomp filter, see 7a8ed9459dfe ("Make assertions actually useful"). And we replaced it by an err(), followed by an abort() (that is also catched by seccomp). We don't have a coredump or a backtrace but we have at least the error message... only if logging is enabled. As this kind of information is needed in any case, replace the "err()" function by an "fprintf(stderr, ...)". Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- util.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/util.h b/util.h index b7541ce24e5a..b44b4bfdccd7 100644 --- a/util.h +++ b/util.h @@ -13,6 +13,7 @@ #include <stdint.h> #include <string.h> #include <signal.h> +#include <stdio.h> #include <arpa/inet.h> #include "log.h" @@ -67,8 +68,10 @@ #define ASSERT(expr) \ do { \ if (!(expr)) { \ - err("ASSERTION FAILED in %s (%s:%d): %s", \ - __func__, __FILE__, __LINE__, STRINGIFY(expr)); \ + fprintf(stderr, \ + "ASSERTION FAILED in %s (%s:%d): %s\n", \ + __func__, __FILE__, __LINE__, \ + STRINGIFY(expr)); \ /* This may actually SIGSYS, due to seccomp, \ * but that will still get the job done \ */ \ -- 2.45.2

3 7

[PATCH] log, passt: Keep printing to stderr when passt is running in foreground
by Stefano Brivio 06 Aug '24

06 Aug '24

There are two cases where we want to stop printing to stderr: if it's closed, and if pasta spawned a shell (and --debug wasn't given). But if passt is running in foreground, we currently stop to report any message, even error messages, once we're ready, as reported by Laurent, because we set the log_runtime flag, which we use to indicate we're ready, regardless of whether we're running in foreground or not. Turn that flag (back) to log_stderr, and set it only when we really want to stop printing to stderr. Reported-by: Laurent Vivier <lvivier(a)redhat.com> Fixes: afd9cdc9bb48 ("log, passt: Always print to stderr before initialisation is complete") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 6 +++--- log.h | 2 +- passt.c | 12 +++++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/log.c b/log.c index eb3a780..4ecc2ad 100644 --- a/log.c +++ b/log.c @@ -44,7 +44,7 @@ struct timespec log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ bool log_conf_parsed; /* Logging options already parsed */ -bool log_runtime; /* Daemonised, or ready in foreground */ +bool log_stderr = true; /* Not daemonised, no shell spawned */ /** * logtime_fmt_and_arg() - Build format and arguments to print relative log time @@ -257,7 +257,7 @@ void vlogmsg(bool newline, int pri, const char *format, va_list ap) } if (debug_print || !log_conf_parsed || - (!log_runtime && (log_mask & LOG_MASK(LOG_PRI(pri))))) { + (log_stderr && (log_mask & LOG_MASK(LOG_PRI(pri))))) { (void)vfprintf(stderr, format, ap); if (newline && format[strlen(format)] != '\n') fprintf(stderr, "\n"); @@ -364,7 +364,7 @@ void passt_vsyslog(bool newline, int pri, const char *format, va_list ap) if (newline && format[strlen(format)] != '\n') n += snprintf(buf + n, BUFSIZ - n, "\n"); - if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && !log_runtime) + if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && log_stderr) fprintf(stderr, "Failed to send %i bytes to syslog\n", n); } diff --git a/log.h b/log.h index 5cb16d6..ba17f79 100644 --- a/log.h +++ b/log.h @@ -43,7 +43,7 @@ void logmsg_perror(int pri, const char *format, ...) extern int log_trace; extern bool log_conf_parsed; -extern bool log_runtime; +extern bool log_stderr; extern struct timespec log_start; void trace_init(int enable); diff --git a/passt.c b/passt.c index 6401730..ea5bece 100644 --- a/passt.c +++ b/passt.c @@ -290,15 +290,17 @@ int main(int argc, char **argv) if (isolate_prefork(&c)) die("Failed to sandbox process, exiting"); - if (!c.foreground) + if (!c.foreground) { __daemon(c.pidfile_fd, devnull_fd); - else + log_stderr = false; + } else { pidfile_write(c.pidfile_fd, getpid()); + } - log_runtime = true; - - if (pasta_child_pid) + if (pasta_child_pid) { kill(pasta_child_pid, SIGUSR1); + log_stderr = false; + } isolate_postfork(&c); -- 2.43.0

1 0

[PATCH] util: Use unsigned (size_t) value for iov length
by David Gibson 06 Aug '24

06 Aug '24

The "correct" type for the length of an IOV is unclear: writev() and readv() use an int, but sendmsg() and recvmsg() use a size_t. Using the unsigned size_t has some advantages, though, and it makes more sense for the case of write_remainder. Using size_t throughout here means we don't have a signed vs. unsigned comparison, and we don't have to deal with the case of iov_skip_bytes() returning a value which becomes negative when assigned to an integer. I believe this should fix a Coverity reported integer overflow that's showing up on openscanhub. It's not totally clear from the report what it thinks the path is, but it's on an (iovcnt - i) a few lines later. With both having unsigned type, the loop condition ensures this can't underflow. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- util.c | 5 ++--- util.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/util.c b/util.c index f2e26a7a..54a9f580 100644 --- a/util.c +++ b/util.c @@ -592,10 +592,9 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, * * #syscalls write writev */ -int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip) +int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip) { - int i; - size_t offset; + size_t offset, i; while ((i = iov_skip_bytes(iov, iovcnt, skip, &offset)) < iovcnt) { ssize_t rc; diff --git a/util.h b/util.h index b7541ce2..e8bf9572 100644 --- a/util.h +++ b/util.h @@ -182,7 +182,7 @@ void pidfile_write(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); int fls(unsigned long x); int write_file(const char *path, const char *buf); -int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip); +int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip); /** * af_name() - Return name of an address family -- 2.45.2

1 0

[PATCH 0/2] udp_flow: move all udp_flow functions to udp_flow.c
by Laurent Vivier 05 Aug '24

05 Aug '24

This helps to export them and use them with vhost-user version of udp.c There is only one code change: pass s_in rather than udp_meta_t to udp_flow_from_sock() as udp_meta_t is specific to the socket version of udp.c Otherwise, only code move Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> Laurent Vivier (2): udp_flow: Remove udp_meta_t from the parameters of udp_flow_from_sock() udp_flow: move all udp_flow functions to udp_flow.c Makefile | 2 +- udp.c | 264 +-------------------------------------------------- udp_flow.c | 274 +++++++++++++++++++++++++++++++++++++++++++++++++++++ udp_flow.h | 9 ++ 4 files changed, 286 insertions(+), 263 deletions(-) create mode 100644 udp_flow.c -- 2.45.2

3 7

[PATCH 0/2] Avoid duplicated clock_gettime() in logging
by David Gibson 05 Aug '24

05 Aug '24

Remove a small nit in the logging code. David Gibson (2): log: Make logfile_write() private log: Avoid duplicate calls to clock_gettime() log.c | 344 +++++++++++++++++++++++++++++----------------------------- log.h | 1 - 2 files changed, 172 insertions(+), 173 deletions(-) -- 2.45.2

2 5

[PATCH] pasta: Save errno on signal handler entry, restore on return when needed
by Stefano Brivio 03 Aug '24

03 Aug '24

Ed reported this: # Error: pasta failed with exit code 1: # Couldn't drop cap 3 from bounding set # : No child processes in a Podman CI run with tests being run in parallel. The error message itself, by the way, is fixed by commit 1cd773081f12 ("log: Drop newlines in the middle of the perror()-like messages"), but how can we possibly get ECHILD as failure code for prctl()? Well, we don't, but if we exit early enough, pasta_child_handler() might run before we're even done with isolation steps, and it calls waitid(), which sets errno. We need to restore it before returning from the signal handler (if we return after calling functions that might set it), as signal-safety(7) also implies: Fetching and setting the value of errno is async-signal-safe provided that the signal handler saves errno on entry and restores its value before returning. Eventually, we'll probably need to switch to signalfd(2) the day we want to implement multithreading, but this will do for the moment. Reported-by: Ed Santiago <santiago(a)redhat.com> Link: https://github.com/containers/podman/issues/23478 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- pasta.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pasta.c b/pasta.c index 572048d..b4a3d99 100644 --- a/pasta.c +++ b/pasta.c @@ -61,6 +61,7 @@ int pasta_child_pid; */ void pasta_child_handler(int signal) { + int errno_save = errno; siginfo_t infop; (void)signal; @@ -85,6 +86,8 @@ void pasta_child_handler(int signal) waitid(P_ALL, 0, NULL, WEXITED | WNOHANG); waitid(P_ALL, 0, NULL, WEXITED | WNOHANG); + + errno = errno_save; } /** -- 2.43.0

3 2

[PATCH v2] pasta.c: modify hostname when detaching new namespace
by Danish Prakash 30 Jul '24

30 Jul '24

When invoking pasta without any arguments, it's difficult to tell whether we are in the new namespace or not leaving users a bit confused. This change modifies the host namespace to add a prefix "pasta-" to make it a bit more obvious. Signed-off-by: Danish Prakash <contact(a)danishpraka.sh> --- pasta.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pasta.c b/pasta.c index 31e1e000ee1b..7ce502d3f299 100644 --- a/pasta.c +++ b/pasta.c @@ -50,6 +50,8 @@ #include "netlink.h" #include "log.h" +#define HOSTNAME_PREFIX "pasta-" + /* PID of child, in case we created a namespace */ int pasta_child_pid; @@ -178,6 +180,7 @@ struct pasta_spawn_cmd_arg { */ static int pasta_spawn_cmd(void *arg) { + char hostname[HOST_NAME_MAX + 1] = HOSTNAME_PREFIX; const struct pasta_spawn_cmd_arg *a; sigset_t set; @@ -188,6 +191,13 @@ static int pasta_spawn_cmd(void *arg) if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0")) warn("Cannot set ping_group_range, ICMP requests might fail"); + if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1, HOST_NAME_MAX + 1 - + sizeof(HOSTNAME_PREFIX)) || errno == ENAMETOOLONG) { + hostname[HOST_NAME_MAX] = '\0'; + if (sethostname(hostname, strlen(hostname))) + warn("Unable to set pasta-prefixed hostname"); + } + /* Wait for the parent to be ready: see main() */ sigemptyset(&set); sigaddset(&set, SIGUSR1); -- 2.45.2

2 1

[PATCH v2] Fix typo in README file
by AbdAlRahman Gad 29 Jul '24

29 Jul '24

- remove duplicated 'the' in the 'Services' section Signed-off-by: AbdAlRahman Gad <abdobngad(a)gmail.com> --- v2: Add Signed-off-by: tag README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index aa2afac..bb114ea 100644 --- a/README.md +++ b/README.md @@ -398,7 +398,7 @@ services: and nameserver using SLAAC * [DHCPv6 server](/passt/tree/dhcpv6.c): a simple implementation handing out one single IPv6 address to the guest or namespace, - namely, the the same address as the first one configured for the upstream host + namely, the same address as the first one configured for the upstream host interface, and passing the nameservers configured on the host ## Addresses -- 2.43.0

3 2

[PATCH] pasta.c: modify hostname when detaching new namespace
by Danish Prakash 29 Jul '24

29 Jul '24

When invoking pasta without any arguments, it's difficult to tell whether we are in the new namespace or not leaving users a bit confused. This change modifies the host namespace to add a prefix "pasta-" to make it a bit more obvious. Signed-off-by: Danish Prakash <contact(a)danishpraka.sh> --- pasta.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pasta.c b/pasta.c index 31e1e00..840a2b1 100644 --- a/pasta.c +++ b/pasta.c @@ -180,6 +180,8 @@ static int pasta_spawn_cmd(void *arg) { const struct pasta_spawn_cmd_arg *a; sigset_t set; + char hostname[HOST_NAME_MAX+1], pasta_hostname[HOST_NAME_MAX+1]; + char *hostname_prefix = "pasta-"; /* We run in a detached PID and mount namespace: mount /proc over */ if (mount("", "/proc", "proc", 0, NULL)) @@ -188,6 +190,17 @@ static int pasta_spawn_cmd(void *arg) if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0")) warn("Cannot set ping_group_range, ICMP requests might fail"); + if (gethostname(hostname, HOST_NAME_MAX+1) == 0) { + if ((strlen(hostname) + strlen(hostname_prefix)) > HOST_NAME_MAX) { + hostname[strlen(hostname)-strlen(hostname_prefix)] = '\0'; + } + sprintf(pasta_hostname, "%s%s", hostname_prefix, hostname); + + if (sethostname(pasta_hostname, strlen(pasta_hostname)) != 0) { + warn("Unable to set pasta-prefixed hostname"); + } + } + /* Wait for the parent to be ready: see main() */ sigemptyset(&set); sigaddset(&set, SIGUSR1); -- 2.45.1

2 10

[PATCH 0/5] Fix assorted errors in the Qemu socket tap receive handler
by David Gibson 29 Jul '24

29 Jul '24

A long time ago Matej Hrica pointed out a possible buffer overrun when receiving data from the qemu socket. Stefano recently proposed a fix for this, but I don't think it's quite right. This series is a different approach to fixing that problem and a few adjacent ones. David Gibson (5): tap: Better report errors receiving from QEMU socket tap: Don't attempt to carry on if we get a bad frame length from qemu tap: Don't use EPOLLET on Qemu sockets tap: Correctly handle frames of odd length tap: Improve handling of partially received frames on qemu socket passt.h | 1 - tap.c | 72 ++++++++++++++++++++++++++++++--------------------------- util.h | 16 +++++++++++++ 3 files changed, 54 insertions(+), 35 deletions(-) -- 2.45.2

2 17