- dev - passt

[PATCH] Makefile: Fix SuperH 4 builds: it's AUDIT_ARCH_SH, not AUDIT_ARCH_SH4
by Stefano Brivio 28 Feb '23

28 Feb '23

sh4 builds currently fail because of this: https://buildd.debian.org/status/fetch.php?pkg=passt&arch=sh4&ver=0.0%7Egit… Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index e5723a7..6f8bd81 100644 --- a/Makefile +++ b/Makefile @@ -31,6 +31,7 @@ AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/PPC64/PPC/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/PPCLE/PPC64LE/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/MIPS64EL/MIPSEL64/') AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/HPPA/PARISC/') +AUDIT_ARCH := $(shell echo $(AUDIT_ARCH) | sed 's/SH4/SH/') FLAGS := -Wall -Wextra -pedantic -std=c99 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE FLAGS += -D_FORTIFY_SOURCE=2 -O2 -pie -fPIE -- 2.39.1

1 0

passt: new version 2023_02_27.c538ee8 available
by Stefano Brivio 27 Feb '23

27 Feb '23

The new version with tag 2023_02_27.c538ee8 includes the following changes: c538ee8 util: Add own prototype for __clone2() on ia64 b1b75bd contrib/apparmor: Split profile into abstractions, use them 0d547a5 qrap: Generate -netdev as JSON 4f2341f qrap: Introduce machine-specific PCI address base 8828a63 qrap: Drop args in JSON format 16f19c8 qrap: Fix support for pc machines 9cec430 qrap: Fix limits for PCI addresses d2df763 log, conf, tap: Define die() as err() plus exit(), drop cppcheck workarounds fb05a71 doc/demo: Fix and suppress ShellCheck warnings 26a0e4d Fix definitions of SOCKET_MAX, TCP_MAX_CONNS 4f523c3 tcp: Avoid (theoretical) resource leak (CWE-772) Coverity warning a1d5537 tcp: Avoid false (but convoluted) positive Coverity CWE-476 warning 5474bc5 tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls() a48c5c2 treewide: Disable gcc strict aliasing rules as needed, drop workarounds da46fda tcp: Suppress knownConditionTrueFalse cppcheck false positive 834b9a3 log: Send identifier string in log messages, openlog() won't work for us bad2526 conf, udp: Allow any loopback address to be used as resolver 8ca907a conf: Split add_dns{4,6}() out of get_dns() ddf7097 udp: Actually use host resolver to forward DNS queries https://passt.top/passt/log/?qt=range&q=2023_02_22.4ddbcb9..2023_02_27.c538… Packages: - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5574807/ permanent mirror: https://passt.top/builds/copr/0^20230227.gc538ee8/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Ubuntu tracker: https://packages.ubuntu.com/lunar/passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-gc538ee8-1.x86_64.rpm - x86_64 static binaries: https://passt.top/builds/latest/x86_64/ - Debian package, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt_0c538ee8-1_all.deb -- Stefano

1 0

[PATCH] util: Add own prototype for __clone2() on ia64
by Stefano Brivio 27 Feb '23

27 Feb '23

ia64 needs to use __clone2() as clone() is not available, but glibc doesn't export the prototype. Take it from clone(2) to avoid an implicit declaration: util.c: In function ‘do_clone’: util.c:512:16: warning: implicit declaration of function ‘__clone2’ [-Wimplicit-function-declaration] 512 | return __clone2(fn, stack_area + stack_size / 2, stack_size / 2, | ^~~~~~~~ Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/util.c b/util.c index c5ee1c0..799173f 100644 --- a/util.c +++ b/util.c @@ -495,6 +495,15 @@ int write_file(const char *path, const char *buf) return len == 0 ? 0 : -1; } +#ifdef __ia64__ +/* Needed by do_clone() below: glibc doesn't export the prototype of __clone2(), + * use the description from clone(2). + */ +int __clone2(int (*fn)(void *), void *stack_base, size_t stack_size, int flags, + void *arg, ... /* pid_t *parent_tid, struct user_desc *tls, + pid_t *child_tid */ ); +#endif + /** * do_clone() - Wrapper of __clone2() for ia64, clone() for other architectures * @fn: Entry point for child -- 2.39.1

2 1

[PATCH 0/5] qrap: Fix a number of issues
by Andrea Bolognani 27 Feb '23

27 Feb '23

Andrea Bolognani (5): qrap: Fix limits for PCI addresses qrap: Fix support for pc machines qrap: Drop args in JSON format qrap: Introduce machine-specific PCI address base qrap: Generate -netdev as JSON qrap.c | 47 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) -- 2.39.2

3 14

[PATCH] contrib/apparmor: Split profile into abstractions, use them
by Stefano Brivio 27 Feb '23

27 Feb '23

One day, libvirt might actually support running passt to provide guest connectivity. Should libvirtd (or virtqemud) start passt, it will need to access socket and PID files in specific locations, and passt needs to accept SIGTERM in case QEMU fails to start after passt is already started. To make this more convenient, split the current profile into two abstractions, for passt and for pasta, so that external programmes can include the bits they need (and especially not include the pasta abstraction if they only need to start passt), plus whatever specific adaptation is needed. For stand-alone usage of passt and pasta, the 'passt' profile simply includes both abstractions, plus rules to create and access PID and capture files in default or reasonable ($HOME) locations. Tested on Debian with libvirt 9.0.0 together with a local fix to start passt as intended, namely libvirt commit c0efdbdb9f66 ("qemu_passt: Avoid double daemonizing passt"). This is an example of how the libvirtd profile (or virtqemud abstraction, or virtqemud profile) can use this: # support for passt network back-end /usr/bin/passt Cx -> passt, profile passt { /usr/bin/passt r, owner @{run}/user/[0-9]*/libvirt/qemu/run/passt/* rw, signal (receive) set=("term") peer=/usr/sbin/libvirtd, signal (receive) set=("term") peer=libvirtd, include if exists <abstractions/passt> } translated: - when executing /usr/bin/passt, switch to the subprofile "passt" (not the "discrete", i.e. stand-alone profile), described below. Scrub the environment (e.g. LD_PRELOAD is dropped) - in the "passt" subprofile: - allow reading the binary - allow read and write access to PID and socket files - make passt accept SIGTERM from /usr/sbin/libvirtd, and libvirtd peer names - include anything else that's needed by passt itself Suggested-by: Andrea Bolognani <abologna(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/apparmor/abstractions/passt | 47 ++++++++++++++++++++ contrib/apparmor/abstractions/pasta | 35 +++++++++++++++ contrib/apparmor/usr.bin.passt | 67 +++-------------------------- 3 files changed, 89 insertions(+), 60 deletions(-) create mode 100644 contrib/apparmor/abstractions/passt create mode 100644 contrib/apparmor/abstractions/pasta diff --git a/contrib/apparmor/abstractions/passt b/contrib/apparmor/abstractions/passt new file mode 100644 index 0000000..000e3db --- /dev/null +++ b/contrib/apparmor/abstractions/passt @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# contrib/apparmor/abstractions/passt - Abstraction for passt(1) +# +# Copyright (c) 2022 Red Hat GmbH +# Author: Stefano Brivio <sbrivio(a)redhat.com> + + abi <abi/3.0>, + + include <abstractions/base> + + # Alternatively: include <abstractions/nameservice> + @{etc_ro}/resolv.conf r, # get_dns(), conf.c + + capability net_bind_service, # isolation.c, conf.c + capability setuid, + capability setgid, + capability sys_admin, + capability setpcap, + capability net_admin, + capability sys_ptrace, + + / r, # isolate_prefork(), isolation.c + mount "" -> "/", + mount "" -> "/tmp/", + pivot_root "/tmp/" -> "/tmp/", + umount "/", + + network netlink raw, # nl_sock_init_do(), netlink.c + + network inet stream, # tcp.c + network inet6 stream, + + network inet dgram, # udp.c + network inet6 dgram, + + network unix stream, # tap.c + + network unix dgram, # __openlog(), log.c + + /usr/bin/passt.avx2 ix, # arch_avx2_exec(), arch.c diff --git a/contrib/apparmor/abstractions/pasta b/contrib/apparmor/abstractions/pasta new file mode 100644 index 0000000..7d802ac --- /dev/null +++ b/contrib/apparmor/abstractions/pasta @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# contrib/apparmor/abstractions/pasta - Abstraction for pasta(1) +# +# Copyright (c) 2022 Red Hat GmbH +# Author: Stefano Brivio <sbrivio(a)redhat.com> + + abi <abi/3.0>, + + include <abstractions/passt> + + @{PROC}/net/tcp r, # procfs_scan_listen(), util.c + @{PROC}/net/tcp6 r, + @{PROC}/net/udp r, + @{PROC}/net/udp6 r, + + @{run}/user/@{uid}/netns/* r, # pasta_open_ns(), pasta.c + + @{PROC}/[0-9]*/ns/net r, # pasta_wait_for_ns(), + @{PROC}/[0-9]*/ns/user r, # conf_pasta_ns() + + /dev/net/tun rw, # tap_ns_tun(), tap.c + + owner @{PROC}/@{pid}/gid_map w, # pasta_start_ns(), conf_ugid() + owner @{PROC}/@{pid}/setgroups w, + owner @{PROC}/@{pid}/uid_map rw, + + owner @{PROC}/sys/net/ipv4/ping_group_range w, # pasta_spawn_cmd(), pasta.c + /{usr/,}bin/** Ux, diff --git a/contrib/apparmor/usr.bin.passt b/contrib/apparmor/usr.bin.passt index 96b61ef..7433d01 100644 --- a/contrib/apparmor/usr.bin.passt +++ b/contrib/apparmor/usr.bin.passt @@ -16,73 +16,20 @@ abi <abi/3.0>, include <tunables/global> profile passt /usr/bin/passt{,.avx2} flags=(attach_disconnected) { - ### Common rules for passt and pasta - - include <abstractions/base> - - # Alternatively: include <abstractions/nameservice> - @{etc_ro}/resolv.conf r, # get_dns(), conf.c - - capability net_bind_service, # isolation.c, conf.c - capability setuid, - capability setgid, - capability sys_admin, - capability setpcap, - capability net_admin, - capability sys_ptrace, - - / r, # isolate_prefork(), isolation.c - mount "" -> "/", - mount "" -> "/tmp/", - pivot_root "/tmp/" -> "/tmp/", - umount "/", - - network netlink raw, # nl_sock_init_do(), netlink.c - - network inet stream, # tcp.c - network inet6 stream, - - network inet dgram, # udp.c - network inet6 dgram, - - network unix stream, # tap.c - - network unix dgram, # __openlog(), log.c - - # Alternatively: include <abstractions/user-tmp> - owner /tmp/** w, # tap_sock_unix_init(), pcap(), - # write_pidfile(), - # logfile_init() - - owner @{HOME}/** w, # pcap(), write_pidfile() - - /usr/bin/passt.avx2 ix, # arch_avx2_exec(), arch.c - - - ### Rules for pasta - ### ### TODO: AppArmor doesn't give us the chance to attach a separate profile ### depending on the executable symlink. That's possible with SELinux. Two ### alternatives: implement that in AppArmor, or consider aa_change_hat(2). ### With this, rules for passt(1) could be restricted significantly. Note that ### the attach_disconnected flag is not needed for passt(1). - @{PROC}/net/tcp r, # procfs_scan_listen(), util.c - @{PROC}/net/tcp6 r, - @{PROC}/net/udp r, - @{PROC}/net/udp6 r, - - @{run}/user/@{uid}/netns/* r, # pasta_open_ns(), pasta.c + include <abstractions/passt> - @{PROC}/[0-9]*/ns/net r, # pasta_wait_for_ns(), - @{PROC}/[0-9]*/ns/user r, # conf_pasta_ns() - - /dev/net/tun rw, # tap_ns_tun(), tap.c + # Alternatively: include <abstractions/user-tmp> + owner /tmp/** w, # tap_sock_unix_init(), pcap(), + # write_pidfile(), + # logfile_init() - owner @{PROC}/@{pid}/gid_map w, # pasta_start_ns(), conf_ugid() - owner @{PROC}/@{pid}/setgroups w, - owner @{PROC}/@{pid}/uid_map rw, + owner @{HOME}/** w, # pcap(), write_pidfile() - owner @{PROC}/sys/net/ipv4/ping_group_range w, # pasta_spawn_cmd(), pasta.c - /{usr/,}bin/** Ux, + include <abstractions/pasta> } -- 2.39.1

1 0

[PATCH 0/3] Allow non-127.0.0.1 loopback address as host resolver
by Stefano Brivio 27 Feb '23

27 Feb '23

This series fixes an issue with DNS resolution reported by Andrea: if both guest and host use systemd-resolved, reachable at 127.0.0.53, we need to advertise a reachable address as DNS to the guest, which we already do, but also to map UDP packets for that address and port 53 to the original host resolver address. Otherwise, DNS resolution doesn't work at all in the guest, with default options. Stefano Brivio (3): udp: Actually use host resolver to forward DNS queries conf: Split add_dns{4,6}() out of get_dns() conf, udp: Allow any loopback address to be used as resolver conf.c | 92 ++++++++++++++++++++++++++++++++++++---------------------- udp.c | 20 ++++++------- 2 files changed, 67 insertions(+), 45 deletions(-) -- 2.39.1

3 7

[PATCH] log, conf, tap: Define die() as err() plus exit(), drop cppcheck workarounds
by Stefano Brivio 27 Feb '23

27 Feb '23

If we define die() as a variadic macro, passing __VA_ARGS__ to err(), and calling exit() outside err() itself, we can drop the workarounds introduced in commit 36f0199f6ef4 ("conf, tap: Silence two false positive invalidFunctionArg from cppcheck"). Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 3 --- log.c | 14 +++++--------- log.h | 7 ++++++- tap.c | 6 +----- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/conf.c b/conf.c index 37f25d6..0e512f4 100644 --- a/conf.c +++ b/conf.c @@ -1064,9 +1064,6 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) { die("Can't determine if we're in init namespace: %s", strerror(errno)); - - /* Silence cppcheck's invalidFunctionArg for 'fd' in read() */ - return; } if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || diff --git a/log.c b/log.c index bfde6ce..d2f08fa 100644 --- a/log.c +++ b/log.c @@ -46,7 +46,7 @@ int log_trace; /* --trace mode enabled */ #define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) -#define logfn(name, level, doexit) \ +#define logfn(name, level) \ void name(const char *format, ...) { \ struct timespec tp; \ va_list args; \ @@ -75,9 +75,6 @@ void name(const char *format, ...) { \ if (format[strlen(format)] != '\n') \ fprintf(stderr, "\n"); \ } \ - \ - if (doexit) \ - exit(EXIT_FAILURE); \ } /* Prefixes for log file messages, indexed by priority */ @@ -90,11 +87,10 @@ const char *logfile_prefix[] = { " ", /* LOG_DEBUG */ }; -logfn(die, LOG_ERR, 1) -logfn(err, LOG_ERR, 0) -logfn(warn, LOG_WARNING, 0) -logfn(info, LOG_INFO, 0) -logfn(debug,LOG_DEBUG, 0) +logfn(err, LOG_ERR) +logfn(warn, LOG_WARNING) +logfn(info, LOG_INFO) +logfn(debug,LOG_DEBUG) /** * trace_init() - Set log_trace depending on trace (debug) mode diff --git a/log.h b/log.h index d4e9d85..d4ea141 100644 --- a/log.h +++ b/log.h @@ -10,12 +10,17 @@ #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ #define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE)) -void die(const char *format, ...); void err(const char *format, ...); void warn(const char *format, ...); void info(const char *format, ...); void debug(const char *format, ...); +#define die(...) \ + do { \ + err(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + extern int log_trace; void trace_init(int enable); #define trace(...) \ diff --git a/tap.c b/tap.c index d6f962e..88eed88 100644 --- a/tap.c +++ b/tap.c @@ -1037,13 +1037,9 @@ static void tap_sock_unix_init(struct ctx *c) snprintf(path, UNIX_PATH_MAX - 1, UNIX_SOCK_PATH, i); ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); - if (ex < 0) { + if (ex < 0) die("UNIX domain socket check: %s", strerror(errno)); - /* Silence cppcheck's invalidFunctionArg for 'ex' */ - return; - } - ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); if (!ret || (errno != ENOENT && errno != ECONNREFUSED && errno != EACCES)) { -- 2.39.1

2 1

[PATCH] doc/demo: Fix and suppress ShellCheck warnings
by Stefano Brivio 27 Feb '23

27 Feb '23

ShellCheck reports (SC2034) that __qemu_arch is not used. Use it, and silence the resulting SC2086 warning as we want word splitting on options we pass with it. While at it, silence SC2317 warnings for commands in cleanup() that appear to be unreachable: cleanup() is only called as trap. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- doc/demo.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/demo.sh b/doc/demo.sh index ed71ffb..5b05ddf 100755 --- a/doc/demo.sh +++ b/doc/demo.sh @@ -110,6 +110,7 @@ next() { } # cleanup() - Terminate pasta and passt, clean up, restore TTY settings +# shellcheck disable=SC2317 cleanup() { [ -f "${DEMO_DIR}/pasta.pid" ] && kill "$(cat "${DEMO_DIR}/pasta.pid")" [ -f "${DEMO_DIR}/passt.pid" ] && kill "$(cat "${DEMO_DIR}/passt.pid")" @@ -223,7 +224,8 @@ into_ns() { echo "Use ^C to terminate it." next - cmd qrap 5 qemu-system-x86_64 -M pc,accel=kvm:tcg \ + # shellcheck disable=SC2086 + cmd qrap 5 ${__qemu_arch} \ -smp "$(nproc)" -m 1024 \ -nographic -serial stdio -nodefaults -no-reboot -vga none \ -initrd "${DEMO_DIR}/demo.img" \ -- 2.39.1

2 1

[PATCH] Fix definitions of SOCKET_MAX, TCP_MAX_CONNS
by Stefano Brivio 27 Feb '23

27 Feb '23

...and, given that I keep getting this wrong, add a convenience macro, MAX_FROM_BITS(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.h | 4 ++-- tcp.h | 4 ++-- tcp_conn.h | 2 +- util.h | 2 ++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/passt.h b/passt.h index 3d7e567..e0383eb 100644 --- a/passt.h +++ b/passt.h @@ -42,7 +42,7 @@ union epoll_ref; /** * union epoll_ref - Breakdown of reference for epoll socket bookkeeping * @proto: IP protocol number - * @s: Socket number (implies 2^24 limit on number of descriptors) + * @s: Socket number (implies 2^24-1 limit on number of descriptors) * @tcp: TCP-specific reference part * @udp: UDP-specific reference part * @icmp: ICMP-specific reference part @@ -53,7 +53,7 @@ union epoll_ref { struct { int32_t proto:8, #define SOCKET_REF_BITS 24 -#define SOCKET_MAX (1 << SOCKET_REF_BITS) +#define SOCKET_MAX MAX_FROM_BITS(SOCKET_REF_BITS) s:SOCKET_REF_BITS; union { union tcp_epoll_ref tcp; diff --git a/tcp.h b/tcp.h index 5527c5b..36e5391 100644 --- a/tcp.h +++ b/tcp.h @@ -8,8 +8,8 @@ #define TCP_TIMER_INTERVAL 1000 /* ms */ -#define TCP_CONN_INDEX_BITS 17 /* 128k */ -#define TCP_MAX_CONNS (1 << TCP_CONN_INDEX_BITS) +#define TCP_CONN_INDEX_BITS 17 /* 128k - 1 */ +#define TCP_MAX_CONNS MAX_FROM_BITS(TCP_CONN_INDEX_BITS) struct ctx; diff --git a/tcp_conn.h b/tcp_conn.h index a499f34..c22632b 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -54,7 +54,7 @@ struct tcp_tap_conn { #define TCP_RETRANS_BITS 3 unsigned int retrans :TCP_RETRANS_BITS; -#define TCP_MAX_RETRANS ((1U << TCP_RETRANS_BITS) - 1) +#define TCP_MAX_RETRANS MAX_FROM_BITS(TCP_RETRANS_BITS) #define TCP_WS_BITS 4 /* RFC 7323 */ #define TCP_WS_MAX 14 diff --git a/util.h b/util.h index 6303c17..570094c 100644 --- a/util.h +++ b/util.h @@ -40,6 +40,8 @@ #define ROUND_DOWN(x, y) ((x) & ~((y) - 1)) #define ROUND_UP(x, y) (((x) + (y) - 1) & ~((y) - 1)) +#define MAX_FROM_BITS(n) ((int)((1U << (n)) - 1)) + #define BIT(n) (1UL << (n)) #define BITMAP_BIT(n) (BIT((n) % (sizeof(long) * 8))) #define BITMAP_WORD(n) (n / (sizeof(long) * 8)) -- 2.39.1

2 1

[PATCH 0/3] Avoid some warnings reported by Coverity
by Stefano Brivio 27 Feb '23

27 Feb '23

Stefano Brivio (3): tcp, tcp_splice: Get rid of false positive CWE-394 Coverity warning from fls() tcp: Avoid false (but convoluted) positive Coverity CWE-476 warning tcp: Avoid (theoretical) resource leak (CWE-772) Coverity warning tcp.c | 17 ++++++++++++----- tcp_splice.c | 24 ++++++++++++++++-------- 2 files changed, 28 insertions(+), 13 deletions(-) -- 2.39.1

2 6

2024

2023

2022

2021

dev