November 2022 - dev

[PATCH] tcp: Explicit bound check on options length field
by Stefano Brivio 18 Nov '22

18 Nov '22

For some reason, Coverity only reports this (harmless) warning after David's series unifying IPv4 and IPv6 sockets for TCP: an untrusted loop bound (CWE-606) in tcp_opt_get(), coming from the fact that we use indeed the value of a TCP header field as loop bound. Note, though, that the loop already checks we're not exceeding the length of the option field, and this field is used as 8-bit unsigned value, so we can't really look for options past the end of the header. In any case, make Coverity happy with an explicit check. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index 8044617..8874789 100644 --- a/tcp.c +++ b/tcp.c @@ -1146,7 +1146,11 @@ static int tcp_opt_get(const char *opts, size_t len, uint8_t type_find, break; default: type = *(opts++); - optlen = *(opts++) - 2; + + if (*opts < 2 || *(uint8_t *)opts > len) + return -1; + + optlen = *((uint8_t *)opts++) - 2; len -= 2; if (type != type_find) -- 2.35.1

1 0

[PATCH passt 0/5] Add fuzzing
by Richard W.M. Jones 17 Nov '22

17 Nov '22

(Note this patch series does not work so far and needs some help, read on ...) Patches 1 & 2 are general cleanup. The rest of the patches aim to add fuzzing support for Passt using AFL++, Clang and ASAN. I used the same approach as with libnbd: https://gitlab.com/nbdkit/libnbd/-/tree/master/fuzzing Firstly (patch 3) I added an --fd option. This is useful for fuzzing, but also generally useful. It allows a controller process to open a connected stream socket and pass that down to passt via inheritance. Uses outside fuzzing include: having the controlling process open the socket with elevated privleges, and allowing alternate address families to be used (eg. vsock or IB). Unfortunately I don't think the --fd option is working. stracing the code shows the socket being added to the epoll, but it somehow never gets read. It might be something obvious but I couldn't see what was wrong. (NB: The socket passed in is *connected* already). Patch 4 adds the fuzzing wrapper. The purpose of the wrapper is to allow AFL to submit test cases to passt as local files. It works by creating a socketpair(2), forking and execing passt in the parent: (parent) passt -f -e -1 --fd <sock> | ^ | | (child) | input file V /dev/null The child reads the input file (test case) from the command line and pushes it into the socket, while discarding anything written by passt. IOW the child takes the place of qemu. With all patches applied you can test the wrapper alone using: $ ./fuzz-wrapper testcase_dir/empty_tap You will see that it currently hangs which it should not do, and I suspect the problem is related to the implementation of --fd as the wrapper is old and well-tested code. Rich.

2 16

[PATCH 00/32] Use dual stack sockets to listen for inbound TCP connections
by David Gibson 17 Nov '22

17 Nov '22

When forwarding many ports, passt can consume a lot of kernel memory because of the many listening sockets it opens. There are not a lot of ways we can reduce that, but here's one. Currently we create separate listening sockets for each port for both IPv4 and IPv6. However in Linux (and probably other platforms), it's possible to listen for both IPv4 and IPv6 connections on an IPv6 socket. This series uses such dual stack sockets to halve the number of listening sockets needed for TCP. When forwarding all TCP and UDP ports, this reduces the kernel memory used from around 677 MiB to around 487 MiB (kernel 6.0.8 on an x86_64 Fedora 37 machine). This should also be possible for UDP, but that will require a mostly separate implementation. David Gibson (32): clang-tidy: Suppress warning about assignments in if statements style: Minor corrections to function comments tcp_splice: #include tcp_splice.h in tcp_splice.c tcp: Remove unused TCP_MAX_SOCKS constant tcp: Better helpers for converting between connection pointer and index tcp_splice: Helpers for converting from index to/from tcp_splice_conn tcp: Move connection state structures into a shared header tcp: Add connection union type tcp: Improved helpers to update connections after moving tcp: Unify spliced and non-spliced connection tables tcp: Unify tcp_defer_handler and tcp_splice_defer_handler() tcp: Partially unify tcp_timer() and tcp_splice_timer() tcp: Unify the IN_EPOLL flag tcp: Separate helpers to create ns listening sockets tcp: Unify part of spliced and non-spliced conn_from_sock path tcp: Use the same sockets to listen for spliced and non-spliced connections tcp: Remove splice from tcp_epoll_ref tcp: Don't store hash bucket in connection structures inany: Helper functions for handling addresses which could be IPv4 or IPv6 tcp: Hash IPv4 and IPv4-mapped-IPv6 addresses the same tcp: Take tcp_hash_insert() address from struct tcp_conn tcp: Simplify tcp_hash_match() to take an inany_addr tcp: Unify initial sequence number calculation for IPv4 and IPv6 tcp: Have tcp_seq_init() take its parameters from struct tcp_conn tcp: Fix small errors in tcp_seq_init() time handling tcp: Remove v6 flag from tcp_epoll_ref tcp: NAT IPv4-mapped IPv6 addresses like IPv4 addresses tcp_splice: Allow splicing of connections from IPv4-mapped loopback tcp: Consolidate tcp_sock_init[46] util: Allow sock_l4() to open dual stack sockets util: Always return -1 on error in sock_l4() tcp: Use dual stack sockets for port forwarding when possible Makefile | 11 +- conf.c | 12 +- inany.h | 94 +++++ siphash.c | 2 + tap.c | 6 +- tcp.c | 978 ++++++++++++++++++++++----------------------------- tcp.h | 11 +- tcp_conn.h | 192 ++++++++++ tcp_splice.c | 337 ++++++++---------- tcp_splice.h | 12 +- util.c | 19 +- 11 files changed, 887 insertions(+), 787 deletions(-) create mode 100644 inany.h create mode 100644 tcp_conn.h -- 2.38.1

2 56

[PATCH] test/lib/test: Clean up iperf3 JSON files before starting the server
by Stefano Brivio 17 Nov '22

17 Nov '22

...instead of doing it after the test. Now that we have pre-built guest images, we might also have old JSON files from previous, interrupted test runs. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- test/lib/test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lib/test b/test/lib/test index df32178..29612c2 100755 --- a/test/lib/test +++ b/test/lib/test @@ -31,6 +31,8 @@ test_iperf3() { __procs="$((${1} - 1))"; shift __time="${1}"; shift + pane_or_context_run "${__sctx}" 'rm -f s*.json' + pane_or_context_run_bg "${__sctx}" \ 'for i in $(seq 0 '${__procs}'); do' \ ' (iperf3 -s1J -p'${__port}' -i'${__time} \ @@ -64,8 +66,6 @@ test_iperf3() { __bw=$(pane_or_context_output "${__sctx}" \ 'cat s*.json | jq -rMs "map('${__jval}') | add"') - pane_or_context_run "${__sctx}" \ - 'for i in $(seq 0 '${__procs}'); do rm s${i}.json; done' TEST_ONE_subs="$(list_add_pair "${TEST_ONE_subs}" "__${__var}__" "${__bw}" )" -- 2.35.1

2 2

[PATCH v3] util, pasta: Add do_clone() wrapper around __clone2() and clone()
by Stefano Brivio 17 Nov '22

17 Nov '22

Spotted in Debian's buildd logs: on ia64, clone(2) is not available: the glibc wrapper is named __clone2() and it takes, additionally, the size of the stack area passed by the caller. Add a do_clone() wrapper handling the different cases, and also taking care of pointing the child's stack in the middle of the allocated area: on PA-RISC (hppa), handled by clone(), the stack grows up, and on ia64 the stack grows down, but the register backing store grows up -- and I think it might be actually used here. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v3: Don't call it __clone(), it conflicts with glibc on static builds v2: Add a wrapper instead of two __ia64__ ifdefs pasta.c | 10 +++++----- util.c | 21 +++++++++++++++++++++ util.h | 8 +++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/pasta.c b/pasta.c index db86317..528f02a 100644 --- a/pasta.c +++ b/pasta.c @@ -226,11 +226,11 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, arg.argv = sh_argv; } - pasta_child_pid = clone(pasta_spawn_cmd, - ns_fn_stack + sizeof(ns_fn_stack) / 2, - CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | - CLONE_NEWUTS, - (void *)&arg); + pasta_child_pid = do_clone(pasta_spawn_cmd, ns_fn_stack, + sizeof(ns_fn_stack), + CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | + CLONE_NEWUTS, + (void *)&arg); if (pasta_child_pid == -1) { perror("clone"); diff --git a/util.c b/util.c index be102e3..2125a67 100644 --- a/util.c +++ b/util.c @@ -482,3 +482,24 @@ int write_file(const char *path, const char *buf) close(fd); return len == 0 ? 0 : -1; } + +/** + * do_clone() - Wrapper of __clone2() for ia64, clone() for other architectures + * @fn: Entry point for child + * @stack_area: Stack area for child: we'll point callees to the middle of it + * @stack_size: Total size of stack area, passed to callee divided by two + * @flags: clone() system call flags + * @arg: Argument to @fn + * + * Return: thread ID of child, -1 on failure + */ +int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, + void *arg) +{ +#ifdef __ia64__ + return __clone2(fn, stack_area + stack_size / 2, stack_size / 2, + flags, arg); +#else + return clone(fn, stack_area + stack_size / 2, flags, arg); +#endif +} diff --git a/util.h b/util.h index 2d4e1ff..02b55a9 100644 --- a/util.h +++ b/util.h @@ -81,13 +81,15 @@ (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) #define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 8) +int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, + void *arg); #define NS_CALL(fn, arg) \ do { \ char ns_fn_stack[NS_FN_STACK_SIZE]; \ \ - clone((fn), ns_fn_stack + sizeof(ns_fn_stack) / 2, \ - CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD, \ - (void *)(arg)); \ + do_clone((fn), ns_fn_stack, sizeof(ns_fn_stack), \ + CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\ + (void *)(arg)); \ } while (0) #if __BYTE_ORDER == __BIG_ENDIAN -- 2.35.1

2 2

vsock regression
by David Gibson 17 Nov '22

17 Nov '22

A word of warning, if you try to run the passt tests on a recent qemu git build. A change to default options causes vsock to break. You can work around it by adding a `queue_reset=false` option to the vsock device, or reverting to qemu commit 3c37f8b8d1e5 or earlier. I've written up the details here: https://gitlab.com/qemu-project/qemu/-/issues/1318 Possibly I should also ping some people on qemu-devel. -- 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

1 0

passt: new version 2022_11_16.ace074c available
by Stefano Brivio 17 Nov '22

17 Nov '22

The new version with tag 2022_11_16.ace074c includes the following changes: ace074c README: The upcoming version of Podman adds support for pasta ab6f825 util, pasta: Add do_clone() wrapper around __clone2() and clone() 7d8c2fb test/lib/test: Clean up iperf3 JSON files before starting the server 25dab96 tap: Revert recently added checks in tap_handler_passt() b27d6d1 arp, tap, util: Don't use perror() after seccomp filter is installed 5f74465 Remove contrib/debian, Debian package development now happens on Salsa fb7b71b contrib/apparmor: Merge pasta and passt profiles, update rules b6400db README: Add links to Debian package tracker 85d8ba3 Makefile: Change HPPA into PARISC while building PASST_AUDIT_ARCH fb8376f Makefile: It's AUDIT_ARCH_MIPSEL64, not AUDIT_ARCH_MIPS64EL b501a8f Makefile: Don't filter out -O2 from supplied flags for AVX2 builds b3c9e76 Makefile: Honour passed CPPFLAGS, not just CFLAGS 3a2afde conf, udp: Drop mostly duplicated dns_send arrays, rename related fields https://passt.top/passt/log/?qt=range&q=2022_11_10.4129764..2022_11_16.ace0… Packages: - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5043502/ permanent mirror: https://passt.top/builds/copr/0^20221116.gace074c/ - Debian: https://tracker.debian.org/pkg/passt https://salsa.debian.org/sbrivio/passt - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Package for other RPM-based distributions, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt-gace074c-1.x86_64.rpm - x86_64 static binaries: https://passt.top/builds/latest/x86_64/ -- Stefano

1 0

[PATCH] README: The upcoming version of Podman adds support for pasta
by Stefano Brivio 16 Nov '22

16 Nov '22

Don't update the demo yet: we could just wait until Podman 4.3.2 is out and packaged. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3d0a5b4..8947702 100644 --- a/README.md +++ b/README.md @@ -330,10 +330,9 @@ speeding up local connections, and usually requiring NAT. _pasta_: * ✅ qemu, libvirt support with [`qrap` wrapper](/passt/tree/qrap.c) * ✅ out-of-tree patches for [qemu](/passt/tree/contrib/qemu) and [libvirt](/passt/tree/contrib/libvirt) available +* ✅ Podman [support](https://github.com/containers/podman/pull/16141) * ✅ bug-to-bug compatible - [_slirp4netns_ replacement](/passt/tree/slirp4netns.sh) (rootless Podman, - RootlessKit) -* ✅ out-of-tree patch for [Podman](/passt/tree/contrib/podman) available + [_slirp4netns_ replacement](/passt/tree/slirp4netns.sh) * ✅ out-of-tree patch for [Kata Containers](/passt/tree/contrib/kata-containers) available * 🛠 native [qemu](https://bugs.passt.top/show_bug.cgi?id=11), @@ -610,6 +609,11 @@ See also the [test logs](/builds/latest/test/). ./pasta + * alternatively, use it directly with Podman (since Podman 4.3.2, or with + commit [`aa47e05ae4a0`](https://github.com/containers/podman/commit/aa47e05ae4a0d14a338cbe106b7eb9cdf098a529): + + podman run --net=pasta ... + * you're now inside a new user and network namespace. For IPv6, SLAAC happens right away as _pasta_ sets up the interface, but DHCPv6 support is available as well. For IPv4, configure the interface with a DHCP client: -- 2.35.1

1 0

[PATCH v2] util, pasta: Add __clone() wrapper around __clone2() and clone()
by Stefano Brivio 16 Nov '22

16 Nov '22

Spotted in Debian's buildd logs: on ia64, clone(2) is not available: the glibc wrapper is named __clone2() and it takes, additionally, the size of the stack area passed by the caller. Add a __clone() wrapper handling the different cases, and also taking care of pointing the child's stack in the middle of the allocated area: on PA-RISC (hppa), handled by clone(), the stack grows up, and on ia64 the stack grows down, but the register backing store grows up -- and I think it might be actually used here. Suggested-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Add a wrapper instead of two __ia64__ ifdefs pasta.c | 10 +++++----- util.c | 21 +++++++++++++++++++++ util.h | 8 +++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/pasta.c b/pasta.c index db86317..e00c197 100644 --- a/pasta.c +++ b/pasta.c @@ -226,11 +226,11 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, arg.argv = sh_argv; } - pasta_child_pid = clone(pasta_spawn_cmd, - ns_fn_stack + sizeof(ns_fn_stack) / 2, - CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | - CLONE_NEWUTS, - (void *)&arg); + pasta_child_pid = __clone(pasta_spawn_cmd, ns_fn_stack, + sizeof(ns_fn_stack), + CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | + CLONE_NEWUTS, + (void *)&arg); if (pasta_child_pid == -1) { perror("clone"); diff --git a/util.c b/util.c index be102e3..d068fe3 100644 --- a/util.c +++ b/util.c @@ -482,3 +482,24 @@ int write_file(const char *path, const char *buf) close(fd); return len == 0 ? 0 : -1; } + +/** + * __clone() - Wrapper of __clone2() for ia64, clone() for other architectures + * @fn: Entry point for child + * @stack_area: Stack area for child: we'll point callees to the middle of it + * @stack_size: Total size of stack area, passed to callee divided by two + * @flags: clone() system call flags + * @arg: Argument to @fn + * + * Return: thread ID of child, -1 on failure + */ +int __clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, + void *arg) +{ +#ifdef __ia64__ + return __clone2(fn, stack_area + stack_size / 2, stack_size / 2, + flags, arg); +#else + return clone(fn, stack_area + stack_size / 2, flags, arg); +#endif +} diff --git a/util.h b/util.h index 2d4e1ff..7591817 100644 --- a/util.h +++ b/util.h @@ -81,13 +81,15 @@ (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) #define NS_FN_STACK_SIZE (RLIMIT_STACK_VAL * 1024 / 8) +int __clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, + void *arg); #define NS_CALL(fn, arg) \ do { \ char ns_fn_stack[NS_FN_STACK_SIZE]; \ \ - clone((fn), ns_fn_stack + sizeof(ns_fn_stack) / 2, \ - CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD, \ - (void *)(arg)); \ + __clone((fn), ns_fn_stack, sizeof(ns_fn_stack), \ + CLONE_VM | CLONE_VFORK | CLONE_FILES | SIGCHLD,\ + (void *)(arg)); \ } while (0) #if __BYTE_ORDER == __BIG_ENDIAN -- 2.35.1

1 0

[PATCH] tap: Revert recently added checks in tap_handler_passt()
by Stefano Brivio 16 Nov '22

16 Nov '22

This reverts commit 198f87835dc4 ("tap: Return -EIO from tap_handler_passt() on inconsistent packet stream") and commit 510dace86ccf ("tap: Keep stream consistent if qemu length descriptor spans two recv() calls"). I can hit occasional failures in perf/passt_tcp tests where we seem to be getting excess data at the end of a recv(), and for some reason I couldn't figure out yet, if we just ignore it, subsequent recv() calls from qemu return correct data. If we close the connection, qemu can't talk to us anymore, of course. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tap.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/tap.c b/tap.c index b7ac996..d26af58 100644 --- a/tap.c +++ b/tap.c @@ -747,31 +747,19 @@ redo: return -ECONNRESET; } - while (n > 0) { - ssize_t len; - - /* Force receiving at least a complete length descriptor to - * avoid an inconsistent stream. - */ - if (n < (ssize_t)sizeof(uint32_t)) { - rem = recv(c->fd_tap, p + n, - (ssize_t)sizeof(uint32_t) - n, 0); - if ((n += rem) != (ssize_t)sizeof(uint32_t)) - return -EIO; - } - - len = ntohl(*(uint32_t *)p); + while (n > (ssize_t)sizeof(uint32_t)) { + ssize_t len = ntohl(*(uint32_t *)p); p += sizeof(uint32_t); n -= sizeof(uint32_t); /* At most one packet might not fit in a single read, and this - * also needs to be blocking. + * needs to be blocking. */ if (len > n) { rem = recv(c->fd_tap, p + n, len - n, 0); if ((n += rem) != len) - return -EIO; + return 0; } /* Complete the partial read above before discarding a malformed -- 2.35.1

1 0