1
0
mirror of https://passt.top/passt synced 2024-11-05 20:31:11 +00:00
Commit Graph

136 Commits

Author SHA1 Message Date
Stefano Brivio
5ab2e12f98 tcp: False "Out-of-bounds read" positive, CWE-125
Reported by Coverity: it doesn't see that tcp{4,6}_l2_buf_used are
set to zero by tcp_l2_data_buf_flush(), repeat that explicitly here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
2a3b8dad33 tcp, tcp_splice: False "Negative array index read" positives, CWE-129
A flag or event bit is always set by callers. Reported by Coverity.

Signed-by-off: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
71a00f1449 tcp: Dereference null return value, CWE-476
Not an issue with a sane kernel behaviour. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
22ed4467a4 treewide: Unchecked return value from library, CWE-252
All instances were harmless, but it might be useful to have some
debug messages here and there. Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-07 11:44:35 +02:00
Stefano Brivio
6a3f6df865 tcp: False "Untrusted loop bound" positive, CWE-606
Field doff in struct tcp_hdr is 4 bits wide, so optlen in
tcp_tap_handler() is already bound, but make that explicit.
Reported by Coverity.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-05 18:47:07 +02:00
Stefano Brivio
dbd0a7035c treewide: Invalid type in argument to printf format specifier, CWE-686
Harmless except for two bad debugging prints.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-04-05 18:47:04 +02:00
Stefano Brivio
37c228ada8 tap, tcp, udp, icmp: Cut down on some oversized buffers
The existing sizes provide no measurable differences in throughput
and packet rates at this point. They were probably needed as batched
implementations were not complete, but they can be decreased quite a
bit now.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
f643c69806 tcp: Fix warning by gcc 5.4 on ppc64le about comparison in CONN_OR_NULL()
...we don't really need two extra bits, but it's easier to organise
things differently than to silence this.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
48582bf47f treewide: Mark constant references as const
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
bb70811183 treewide: Packet abstraction with mandatory boundary checks
Implement a packet abstraction providing boundary and size checks
based on packet descriptors: packets stored in a buffer can be queued
into a pool (without storage of its own), and data can be retrieved
referring to an index in the pool, specifying offset and length.

Checks ensure data is not read outside the boundaries of buffer and
descriptors, and that packets added to a pool are within the buffer
range with valid offset and indices.

This implies a wider rework: usage of the "queueing" part of the
abstraction mostly affects tap_handler_{passt,pasta}() functions and
their callees, while the "fetching" part affects all the guest or tap
facing implementations: TCP, UDP, ICMP, ARP, NDP, DHCP and DHCPv6
handlers.

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
415ccf6116 tcp, tcp_splice: Use less awkward syntax to swap in/out sockets from pools
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
54d9df3903 tcp: Fit struct tcp_conn into a single 64-byte cacheline
...by:

- storing the chained-hash next connection pointer as numeric
  reference rather than as pointer

- storing the MSS as 14-bit value, and rounding it

- using only the effective amount of bits needed to store the hash
  bucket number

- explicitly limiting window scaling factors to 4-bit values
  (maximum factor is 14, from RFC 7323)

- scaling SO_SNDBUF values, and using a 8-bit representation for
  the duplicate ACK sequence

- keeping window values unscaled, as received and sent

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
92074c16a8 tcp_splice: Close sockets right away on high number of open files
We can't take for granted that the hard limit for open files is
big enough as to allow to delay closing sockets to a timer.

Store the value of RTLIMIT_NOFILE we set at start, and use it to
understand if we're approaching the limit with pending, spliced
TCP connections. If that's the case, close sockets right away as
soon as they're not needed, instead of deferring this task to a
timer.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
be5bbb9b06 tcp: Rework timers to use timerfd instead of periodic bitmap scan
With a lot of concurrent connections, the bitmap scan approach is
not really sustainable.

Switch to per-connection timerfd timers, set based on events and on
two new flags, ACK_FROM_TAP_DUE and ACK_TO_TAP_DUE. Timers are added
to the common epoll list, and implement the existing timeouts.

While at it, drop the CONN_ prefix from flag names, otherwise they
get quite long, and fix the logic to decide if a connection has a
local, possibly unreachable endpoint: we shouldn't go through the
rest of tcp_conn_from_tap() if we reset the connection due to a
successful bind(2), and we'll get EACCES if the port number is low.

Suggested by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
3eb19cfd8a tcp, udp, util: Enforce 24-bit limit on socket numbers
This should never happen, but there are no formal guarantees: ensure
socket numbers are below SOCKET_MAX.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-29 15:35:38 +02:00
Stefano Brivio
5ca555cf78 dhcpv6, tap, tcp: Use IN6_ARE_ADDR_EQUAL instead of open-coded memcmp()
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-28 17:11:40 +02:00
Stefano Brivio
e5eefe7743 tcp: Refactor to use events instead of states, split out spliced implementation
Using events and flags instead of states makes the implementation
much more straightforward: actions are mostly centered on events
that occurred on the connection rather than states.

An example is given by the ESTABLISHED_SOCK_FIN_SENT and
FIN_WAIT_1_SOCK_FIN abominations: we don't actually care about
which side started closing the connection to handle closing of
connection halves.

Split out the spliced implementation, as it has very little in
common with the "regular" TCP path.

Refactor things here and there to improve clarity. Add helpers
to trace where resets and flag settings come from.

No functional changes intended.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-03-28 17:11:40 +02:00
Stefano Brivio
04fd94ab07 seccomp, tcp: Add fcntl64 to pasta syscalls for armv6l, armv7l
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-28 04:23:31 +01:00
Stefano Brivio
6c93111864 tcp, udp: Receive batching doesn't pay off when writing single frames to tap
In pasta mode, when we get data from sockets and write it as single
frames to the tap device, we batch receive operations considerably,
and then (conceptually) split the data in many smaller writes.

It looked like an obvious choice, but performance is actually better
if we receive data in many small frame-sized recvmsg()/recvmmsg().

The syscall overhead with the previous behaviour, observed by perf,
comes predominantly from write operations, but receiving data in
shorter chunks probably improves cache locality by a considerable
amount.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-21 13:41:13 +01:00
Stefano Brivio
0515adceaa passt, pasta: Namespace-based sandboxing, defer seccomp policy application
To reach (at least) a conceptually equivalent security level as
implemented by --enable-sandbox in slirp4netns, we need to create a
new mount namespace and pivot_root() into a new (empty) mountpoint, so
that passt and pasta can't access any filesystem resource after
initialisation.

While at it, also detach IPC, PID (only for passt, to prevent
vulnerabilities based on the knowledge of a target PID), and UTS
namespaces.

With this approach, if we apply the seccomp filters right after the
configuration step, the number of allowed syscalls grows further. To
prevent this, defer the application of seccomp policies after the
initialisation phase, before the main loop, that's where we expect bad
things to happen, potentially. This way, we get back to 22 allowed
syscalls for passt and 34 for pasta, on x86_64.

While at it, move #syscalls notes to specific code paths wherever it
conceptually makes sense.

We have to open all the file handles we'll ever need before
sandboxing:

- the packet capture file can only be opened once, drop instance
  numbers from the default path and use the (pre-sandbox) PID instead

- /proc/net/tcp{,v6} and /proc/net/udp{,v6}, for automatic detection
  of bound ports in pasta mode, are now opened only once, before
  sandboxing, and their handles are stored in the execution context

- the UNIX domain socket for passt is also bound only once, before
  sandboxing: to reject clients after the first one, instead of
  closing the listening socket, keep it open, accept and immediately
  discard new connection if we already have a valid one

Clarify the (unchanged) behaviour for --netns-only in the man page.

To actually make passt and pasta processes run in a separate PID
namespace, we need to unshare(CLONE_NEWPID) before forking to
background (if configured to do so). Introduce a small daemon()
implementation, __daemon(), that additionally saves the PID file
before forking. While running in foreground, the process itself can't
move to a new PID namespace (a process can't change the notion of its
own PID): mention that in the man page.

For some reason, fork() in a detached PID namespace causes SIGTERM
and SIGQUIT to be ignored, even if the handler is still reported as
SIG_DFL: add a signal handler that just exits.

We can now drop most of the pasta_child_handler() implementation,
that took care of terminating all processes running in the same
namespace, if pasta started a shell: the shell itself is now the
init process in that namespace, and all children will terminate
once the init process exits.

Issuing 'echo $$' in a detached PID namespace won't return the
actual namespace PID as seen from the init namespace: adapt
demo and test setup scripts to reflect that.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-02-21 13:41:13 +01:00
Stefano Brivio
292c185553 passt: Address new clang-tidy warnings from LLVM 13.0.1
clang-tidy from LLVM 13.0.1 reports some new warnings from these
checkers:

- altera-unroll-loops, altera-id-dependent-backward-branch: ignore
  for the moment being, add a TODO item

- bugprone-easily-swappable-parameters: ignore, nothing to do about
  those

- readability-function-cognitive-complexity: ignore for the moment
  being, add a TODO item

- altera-struct-pack-align: ignore, alignment is forced in protocol
  headers

- concurrency-mt-unsafe: ignore for the moment being, add a TODO
  item

Fix bugprone-implicit-widening-of-multiplication-result warnings,
though, that's doable and they seem to make sense.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-30 02:59:12 +01:00
Stefano Brivio
34e6429235 passt, tap: Daemonise once socket is ready without waiting for connection
The existing behaviour is not really practical: an automated agent in
charge of starting both qemu and passt would need to fork itself to
start passt, because passt won't fork to background until qemu
connects, and the agent needs to unblock to start qemu.

Instead of waiting for a connection to daemonise, do it right away as
soon as a socket is available: that can be considered an initialised
state already.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-28 18:51:50 +01:00
Stefano Brivio
33b1bdd079 seccomp: Add a number of alternate and per-arch syscalls
Depending on the C library, but not necessarily in all the
functions we use, statx() might be used instead of stat(),
getdents() instead of getdents64(), readlinkat() instead of
readlink(), openat() instead of open().

On aarch64, it's clone() and not fork(), and dup3() instead of
dup2() -- just allow the existing alternative instead of dealing
with per-arch selections.

Since glibc commit 9a7565403758 ("posix: Consolidate fork
implementation"), we need to allow set_robust_list() for
fork()/clone(), even in a single-threaded context.

On some architectures, epoll_pwait() is provided instead of
epoll_wait(), but never both. Same with newfstat() and
fstat(), sigreturn() and rt_sigreturn(), getdents64() and
getdents(), readlink() and readlinkat(), unlink() and
unlinkat(), whereas pipe() might not be available, but
pipe2() always is, exclusively or not.

Seen on Fedora 34: newfstatat() is used on top of fstat().

syslog() is an actual system call on some glibc/arch combinations,
instead of a connect()/send() implementation.

On ppc64 and ppc64le, _llseek(), recv(), send() and getuid()
are used. For ppc64 only: ugetrlimit() for the getrlimit()
implementation, plus sigreturn() and fcntl64().

On s390x, additionally, we need to allow socketcall() (on top
of socket()), and sigreturn() also for passt (not just for
pasta).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
be265eef06 tcp: Don't round down MSS to >= 64KiB page size, but clamp it in any case
On some architectures, the page size is bigger than the maximum size
of an Ethernet frame.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
caa22aa644 tcp, udp, util: Fixes for bitmap handling on big-endian, casts
Bitmap manipulating functions would otherwise refer to inconsistent
sets of bits on big-endian architectures. While at it, fix up a
couple of casts.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
1776de0140 tcp, netlink, HAS{BYTES_ACKED,MIN_RTT,GETRANDOM} and NETLINK_GET_STRICT_CHK
tcpi_bytes_acked and tcpi_min_rtt are only available on recent
kernel versions: provide fall-back paths (incurring some grade of
performance penalty).

Support for getrandom() was introduced in Linux 3.17 and glibc 2.25:
provide an alternate mechanism for that as well, reading from
/dev/random.

Also check if NETLINK_GET_STRICT_CHK is defined before using it:
it's not strictly needed, we'll filter out irrelevant results from
netlink anyway.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 16:30:59 +01:00
Stefano Brivio
b93c2c1713 passt: Drop <linux/ipv6.h> include, carry own ipv6hdr and opt_hdr definitions
This is the only remaining Linux-specific include -- drop it to avoid
clang-tidy warnings and to make code more portable.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
f6d9787d30 tap, tcp: Fix two comparisons with different signedness reported by gcc 7
For some reason, those are not reported by recent versions of gcc.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
6040f16239 tcp: Cover all usages of tcpi_snd_wnd with HAS_SND_WND
...I forgot two of them.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2022-01-26 07:57:09 +01:00
Stefano Brivio
a2b86c5f90 tcp: Restore source address to network endianness before using it for hash table
This was actually fine "on the wire", but it's inconsistent with the
way we hash other addresses/protocols and also ends up with a wrong
endianness in captures in case we replace the address with our
default gateway.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 17:54:03 +02:00
Stefano Brivio
627e18fa8a passt: Add cppcheck target, test, and address resulting warnings
...mostly false positives, but a number of very relevant ones too,
in tcp_get_sndbuf(), tcp_conn_from_tap(), and siphash PREAMBLE().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 09:41:13 +02:00
Stefano Brivio
dd942eaa48 passt: Fix build with gcc 7, use std=c99, enable some more Clang checkers
Unions and structs, you all have names now.

Take the chance to enable bugprone-reserved-identifier,
cert-dcl37-c, and cert-dcl51-cpp checkers in clang-tidy.

Provide a ffsl() weak declaration using gcc built-in.

Start reordering includes, but that's not enough for the
llvm-include-order checker yet.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 04:26:08 +02:00
Stefano Brivio
849308d207 Makefile, tcp: Don't try to use tcpi_snd_wnd from tcp_info on pre-5.3 kernels
Detect missing tcpi_snd_wnd in struct tcp_info at build time,
otherwise build fails with a pre-5.3 linux/tcp.h header.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-21 01:19:27 +02:00
Stefano Brivio
9618d24700 ndp, dhcpv6, tcp, udp: Always use link-local as source if gateway isn't
This shouldn't happen on any sane configuration, but I just met an
example of that: the default IPv6 gateway on the host is configured
with a global unicast address, we use that as source for RA, DHCPv6
replies, and the guest ignores it. Same later on if we talk TCP or
UDP and the guest has no idea where that address comes from.

Use our link-local address in case the gateway address is global.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 11:10:23 +02:00
Stefano Brivio
12cfa6444c passt: Add clang-tidy Makefile target and test, take care of warnings
Most are just about style and form, but a few were actually
serious mistakes (NDP-related).

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:34:22 +02:00
Stefano Brivio
b0b77118fe passt: Address warnings from Clang's scan-build
All false positives so far.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:29:30 +02:00
Stefano Brivio
1a563a0cbd passt: Address gcc 11 warnings
A mix of unchecked return values, a missing permission mask for
open(2) with O_CREAT, and some false positives from
-Wstringop-overflow and -Wmaybe-uninitialized.

Reported-by: Martin Hauke <mardnh@gmx.de>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-20 08:29:30 +02:00
Stefano Brivio
1bddcf3dd7 tcp: Fix for non-blocking splice() on older kernels
For some reason, on 4.19, splice() doesn't honour SOCK_NONBLOCK from
accept4() while reading from a TCP socket. Pass SPLICE_F_NONBLOCK
explicitly in all cases.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-19 09:19:50 +02:00
Stefano Brivio
9e065b1448 tcp: Fix ACK reporting on older kernels (no tcp.kernel_snd_wnd case)
If the window isn't updated on !c->tcp.kernel_snd_wnd, we still
have to send ACKs if the ACK sequence was updated, or if an error
occurred while querying TCP_INFO.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-19 09:13:53 +02:00
Stefano Brivio
1ac0d52820 tcp: Arm tcp_data_noack on insufficient window too, don't reset if ACK doesn't match
...and while at it, reverse the operands in the window equality
comparison to detect the need for fast re-transmit: it's easier
to read this way.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-16 16:58:16 +02:00
Stefano Brivio
6943d41d6c tcp: ...and so I got a socket called zero
I thought I'd get away with it, but no, after some clean-ups, I
finally got a socket with number 0. Fix up all the convenient,
yet botched assumptions.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 20:46:17 +02:00
Stefano Brivio
2f4f29c5a7 tcp: Bump TCP_TAP_FRAMES back to 256
With a batched sendmsg(), this is now beneficial.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:17:57 +02:00
Stefano Brivio
38fbfdbcb9 tcp: Get rid of iov with cached MSS, drop sendmmsg(), add deferred flush
Caching iov_len for messages from socket doesn't actually decrease
overhead by the tiniest bit, and added a lot of complexity. Drop
that.

Also drop the sendmmsg(), we don't need to send multiple messages
with TCP, as long as we make sure no messages with a length
descriptor are sent partially, qemu is fine with it.

Just like it's done for segments without data (flags), also defer
the sendmsg() for sending data segments, to improve batching.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:13:23 +02:00
Stefano Brivio
54e6513d17 tcp: Clamp MSS depending on IP version, properly derive buffer sizes
It makes no sense to include an IPv6 header in the calculation for
clamping MSS on IPv4.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-15 17:09:37 +02:00
Stefano Brivio
c61944a1f8 tcp: Explicitly align IP headers in tcp4_l2_{,flags}buf_t also in non-AVX2 build
Otherwise, tcp4_l2_flags_buf_t is not consistent with tcp4_l2_buf_t and
header fields get all mixed up in tcp_l2_buf_fill_headers().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:20:34 +02:00
Stefano Brivio
f45891cf26 conf, tcp, udp: Add --no-map-gw to disable mapping gateway address to host
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:19:52 +02:00
Stefano Brivio
66d5930ec7 passt, pasta: Add seccomp support
List of allowed syscalls comes from comments in the form:
	#syscalls <list>

for syscalls needed both in passt and pasta mode, and:
	#syscalls:pasta <list>
	#syscalls:passt <list>

for syscalls specifically needed in pasta or passt mode only.

seccomp.sh builds a list of BPF statements from those comments,
prefixed by a binary search tree to keep lookup fast.

While at it, clean up a bit the Makefile using wildcards.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:46 +02:00
Stefano Brivio
c9d57fee7c tcp: Decrease pool size for pipes to 16
This should be a reasonable balance between quick connection
establishment and a fast start-up.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:12 +02:00
Stefano Brivio
675174d4ba conf, tap: Split netlink and pasta functions, allow interface configuration
Move netlink routines to their own file, and use netlink to configure
or fetch all the information we need, except for the TUNSETIFF ioctl.

Move pasta-specific functions to their own file as well, add
parameters and calls to configure the tap interface in the namespace.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-14 13:15:12 +02:00
Giuseppe Scrivano
9a175cc2ce pasta: Allow specifying paths and names of namespaces
Based on a patch from Giuseppe Scrivano, this adds the ability to:

- specify paths and names of target namespaces to join, instead of
  a PID, also for user namespaces, with --userns

- request to join or create a network namespace only, without
  entering or creating a user namespace, with --netns-only

- specify the base directory for netns mountpoints, with --nsrun-dir

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
[sbrivio: reworked logic to actually join the given namespaces when
 they're not created, implemented --netns-only and --nsrun-dir,
 updated pasta demo script and man page]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
2021-10-07 04:05:15 +02:00