aboutgitcodebugslistschat
path: root/tcp.c
Commit message (Collapse)AuthorAgeFilesLines
* tcp: Unify tcp_defer_handler and tcp_splice_defer_handler()David Gibson2022-11-251-9/+11
| | | | | | | | | | | These two functions each step through non-spliced and spliced connections respectively and clean up entries for closed connections. To avoid scanning the connection table twice, we merge these into a single function which scans the unified table and performs the appropriate sort of cleanup action on each one. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Unify spliced and non-spliced connection tablesDavid Gibson2022-11-251-20/+26
| | | | | | | | | | | | | | | | | | | | | | | Currently spliced and non-spliced connections are stored in completely separate tables, so there are completely independent limits on the number of spliced and non-spliced connections. This is a bit counter-intuitive. More importantly, the fact that the tables are separate prevents us from unifying some other logic between the two cases. So, merge these two tables into one, using the 'c.spliced' common field to distinguish between them when necessary. For now we keep a common limit of 128k connections, whether they're spliced or non-spliced, which means we save memory overall. If necessary we could increase this to a 256k or higher total, which would cost memory but give some more flexibility. For now, the code paths which need to step through all extant connections are still separate for the two cases, just skipping over entries which aren't for them. We'll improve that in later patches. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Improved helpers to update connections after movingDavid Gibson2022-11-251-7/+9
| | | | | | | | | | When we compact the connection tables (both spliced and non-spliced) we need to move entries from one slot to another. That requires some updates in the entries themselves. Add helpers to make all the necessary updates for the spliced and non-spliced cases. This will simplify later cleanups. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Add connection union typeDavid Gibson2022-11-251-0/+4
| | | | | | | | | | | | | | | | Currently, the tables for spliced and non-spliced connections are entirely separate, with different types in different arrays. We want to unify them. As a first step, create a union type which can represent either a spliced or non-spliced connection. For them to be distinguishable, the individual types need to have a common header added, with a bit indicating which type this structure is. This comes at the cost of increasing the size of tcp_tap_conn to over one (64 byte) cacheline. This isn't ideal, but it makes things simpler for now and we'll re-optimize this later. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Move connection state structures into a shared headerDavid Gibson2022-11-251-156/+50
| | | | | | | | | | | | | | | Currently spliced and non-spliced connections use completely independent tracking structures. We want to unify these, so as a preliminary step move the definitions for both variants into a new tcp_conn.h header, shared by tcp.c and tcp_splice.c. This requires renaming some #defines with the same name but different meanings between the two cases. In the process we correct some places that are slightly out of sync between the comments and the code for various event bit names. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Better helpers for converting between connection pointer and indexDavid Gibson2022-11-251-38/+45
| | | | | | | | | | | | | | The macro CONN_OR_NULL() is used to look up connections by index with bounds checking. Replace it with an inline function, which means: - Better type checking - No danger of multiple evaluation of an @index with side effects Also add a helper to perform the reverse translation: from connection pointer to index. Introduce a macro for this which will make later cleanups easier and safer. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, udp: Don't initialise IPv6/IPv4 sockets if IPv4/IPv6 are not enabledStefano Brivio2022-11-101-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | If we disable a given IP version automatically (no corresponding default route on host) or administratively (--ipv4-only or --ipv6-only options), we don't initialise related buffers and services (DHCP for IPv4, NDP and DHCPv6 for IPv6). The "tap" handlers will also ignore packets with a disabled IP version. However, in commit 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") I happily changed socket initialisation functions to take AF_UNSPEC meaning "any enabled IP version", but I forgot to add checks back for the "enabled" part. Reported by Paul: on a host without default IPv6 route, but IPv6 enabled, connect, using IPv6, to a port handled by pasta, which tries to send data to a tap device without initialised buffers for that IP version and exits because the resulting write() fails. Simpler way to reproduce: pasta -6 and inbound IPv4 connection, or pasta -4 and inbound IPv6 connection. Reported-by: Paul Holzinger <pholzing@redhat.com> Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Correct function comments for address typesDavid Gibson2022-11-041-6/+6
| | | | | | | | | A number of functions describe themselves as taking a pointer to 'sin_addr or sin6_addr'. Those are field names, not type names. Replace them with the correct type names, in_addr or in6_addr. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use typing to reduce chances of IPv4 endianness errorsDavid Gibson2022-11-041-25/+23
| | | | | | | | | | | | | | | | | | | We recently corrected some errors handling the endianness of IPv4 addresses. These are very easy errors to make since although we mostly store them in network endianness, we sometimes need to manipulate them in host endianness. To reduce the chances of making such mistakes again, change to always using a (struct in_addr) instead of a bare in_addr_t or uint32_t to store network endian addresses. This makes it harder to accidentally do arithmetic or comparisons on such addresses as if they were host endian. We introduce a number of IN4_IS_ADDR_*() helpers to make it easier to directly work with struct in_addr values. This has the additional benefit of making the IPv4 and IPv6 paths more visually similar. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Don't create 'tap' socket for ports that are bound to loopback onlyStefano Brivio2022-10-151-69/+114
| | | | | | | | | | | | | | | | | | | | | If the user specifies an explicit loopback address for a port binding, we're going to use that address for the 'tap' socket, and the same exact address for the 'spliced' socket (because those are, by definition, only bound to loopback addresses). This means that the second binding will fail, and, unexpectedly, the port is forwarded, but via tap device, which means the source address in the namespace won't be a loopback address. Make it explicit under which conditions we're creating which kind of socket, by refactoring tcp_sock_init() into two separate functions for IPv4 and IPv6 and gathering those conditions at the beginning. Also, don't create spliced sockets if the user specifies explicitly a non-loopback address, those are harmless but not desired either. Fixes: 3c6ae625101a ("conf, tcp, udp: Allow address specification for forwarded ports") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, tcp_splice: Fix port remapping for inbound, spliced connectionsStefano Brivio2022-10-151-4/+3
| | | | | | | | | | | | | | | | | | | | | | | In pasta mode, when we receive a new inbound connection, we need to select a socket that was created in the namespace to proceed and connect() it to its final destination. The existing condition might pick a wrong socket, though, if the destination port is remapped, because we'll check the bitmap of inbound ports using the remapped port (stored in the epoll reference) as index, and not the original port. Instead of using the port bitmap for this purpose, store this information in the epoll reference itself, by adding a new 'outbound' bit, that's set if the listening socket was created the namespace, and unset otherwise. Then, use this bit to pick a socket on the right side. Suggested-by: David Gibson <david@gibson.dropbear.id.au> Fixes: 33482d5bf293 ("passt: Add PASTA mode, major rework") Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* tcp, tcp_splice: Adjust comments to current meaning of inbound and outboundStefano Brivio2022-10-151-1/+1
| | | | | | | | | | | | | | | | | | | | For tcp_sock_init_ns(), "inbound" connections used to be the ones being established toward any listening socket we create, as opposed to sockets we connect(). Similarly, tcp_splice_new() used to handle "inbound" connections in the sense that they originated from listening sockets, and they would in turn cause a connect() on an "outbound" socket. Since commit 1128fa03fe73 ("Improve types and names for port forwarding configuration"), though, inbound connections are more broadly defined as the ones directed to guest or namepsace, and outbound the ones originating from there. Update comments for those two functions. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* conf, tcp, udp: Allow specification of interface to bind toStefano Brivio2022-10-151-12/+15
| | | | | | | | | | | | | | | | Since kernel version 5.7, commit c427bfec18f2 ("net: core: enable SO_BINDTODEVICE for non-root users"), we can bind sockets to interfaces, if they haven't been bound yet (as in bind()). Introduce an optional interface specification for forwarded ports, prefixed by %, that can be passed together with an address. Reported use case: running local services that use ports we want to have externally forwarded: https://github.com/containers/podman/issues/14425 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Move logging functions to a new file, log.cStefano Brivio2022-10-141-0/+1
| | | | | | | | Logging to file is going to add some further complexity that we don't want to squeeze into util.c. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* cppcheck: Suppress NULL pointer warning in tcp_sock_consume()David Gibson2022-09-291-0/+1
| | | | | | | | | | | | | Recent versions of cppcheck give a warning due to the NULL buffer passed to recv() in tcp_sock_consume(). Since this apparently works, I assume it's actually valid, but cppcheck doesn't know that recv() can take a NULL buffer. So, use a suppression to get rid of the error. Also add an unmatchedSuppression suppression since only some cppcheck versions complain about this. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* cppcheck: Work around false positive NULL pointer dereference errorDavid Gibson2022-09-291-0/+1
| | | | | | | | | | | | | Some versions of cppcheck could errneously report a NULL pointer deference inside a sizeof(). This is now fixed in cppcheck upstream[0]. For systems using an affected version, add a suppression to work around the bug. Also add an unmatchedSuppression suppression so the suppression itself doesn't cause a warning if you *do* have a fixed cppcheck. [0] https://github.com/danmar/cppcheck/pull/4471 Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Fix widespread off-by-one error dealing with port numbersDavid Gibson2022-09-241-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a 'short'. USHRT_MAX is therefore the maximum port number and this is widely used in the code. Unfortunately, a lot of those places don't actually want the maximum port number (USHRT_MAX == 65535), they want the total number of ports (65536). This leads to a number of potentially nasty consequences: * We have buffer overruns on the port_fwd::delta array if we try to use port 65535 * We have similar potential overruns for the tcp_sock_* arrays * Interestingly udp_act had the correct size, but we can calculate it in a more direct manner * We have a logical overrun of the ports bitmap as well, although it will just use an unused bit in the last byte so isnt harmful * Many loops don't consider port 65535 (which does mitigate some but not all of the buffer overruns above) * In udp_invert_portmap() we incorrectly compute the reverse port translation for return packets Correct all these by using a new NUM_PORTS defined explicitly for this purpose. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Treat port numbers as unsignedDavid Gibson2022-09-241-2/+2
| | | | | | | | | | | Port numbers are unsigned values, but we're storing them in (signed) int variables in some places. This isn't actually harmful, because int is large enough to hold the entire range of ports. However in places we don't want to use an in_port_t (usually to avoid overflow on the last iteration of a loop) it makes more conceptual sense to use an unsigned int. This will also avoid some problems with later cleanups. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Don't use indirect remap functions for conf_ports()David Gibson2022-09-241-22/+0
| | | | | | | | | | Now that we've delayed initialization of the UDP specific "reverse" map until udp_init(), the only difference between the various 'remap' functions used in conf_ports() is which array they target. So, simplify by open coding the logic into conf_ports() with a pointer to the correct mapping array. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Consolidate port forwarding configuration into a common structureDavid Gibson2022-09-241-23/+19
| | | | | | | | | | | | | | | The configuration for how to forward ports in and out of the guest/ns is divided between several different variables. For each connect direction and protocol we have a mode in the udp/tcp context structure, a bitmap of which ports to forward also in the context structure and an array of deltas to apply if the outward facing and inward facing port numbers are different. This last is a separate global variable, rather than being in the context structure, for no particular reason. UDP also requires an additional array which has the reverse mapping used for return packets. Consolidate these into a re-used substructure in the context structure. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Improve types and names for port forwarding configurationDavid Gibson2022-09-241-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | enum conf_port_type is local to conf.c and is used to track the port forwarding mode during configuration. We don't keep it around in the context structure, however the 'init_detect_ports' and 'ns_detect_ports' fields in the context are based solely on this. Rather than changing encoding, just include the forwarding mode into the context structure. Move the type definition to a new port_fwd.h, which is kind of trivial at the moment but will have more stuff later. While we're there, "conf_port_type" doesn't really convey that this enum is describing how port forwarding is configured. Rename it to port_fwd_mode. The variables (now fields) of this type also have mildly confusing names since it's not immediately obvious whether 'ns' and 'init' refer to the source or destination of the packets. Use "in" (host to guest / init to ns) and "out" (guest to host / ns to init) instead. This has the added bonus that we no longer have locals 'udp_init' and 'tcp_init' which shadow global functions. In addition, add a typedef 'port_fwd_map' for a bitmap of each port number, which is used in several places. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Make substructures for IPv4 and IPv6 specific context informationDavid Gibson2022-07-301-17/+17
| | | | | | | | | | | | The context structure contains a batch of fields specific to IPv4 and to IPv6 connectivity. Split those out into a sub-structure. This allows the conf_ip4() and conf_ip6() functions, which take the entire context but touch very little of it, to be given more specific parameters, making it clearer what it affects without stepping through the code. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* Separate IPv4 and IPv6 configurationDavid Gibson2022-07-301-10/+10
| | | | | | | | | | | | | | | | | | | After recent changes, conf_ip() now has essentially entirely disjoint paths for IPv4 and IPv6 configuration. So, it's cleaner to split them out into different functions conf_ip4() and conf_ip6(). Splitting these out also lets us make the interface a bit nicer, having them return success or failure directly, rather than manipulating c->v4 and c->v6 to indicate success/failure of the two versions. Since these functions may also initialize the interface index for each protocol, it turns out we can then drop c->v4 and c->v6 entirely, replacing tests on those with tests on whether c->ifi4 or c->ifi6 is non-zero (since a 0 interface index is never valid). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> [sbrivio: Whitespace fixes] Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Allow different external interfaces for IPv4 and IPv6 connectivityDavid Gibson2022-07-301-1/+1
| | | | | | | | | | | | | | | | It's quite plausible for a host to have both IPv4 and IPv6 connectivity, but only via different interfaces. For example, this will happen in the case that IPv6 connectivity is via a tunnel (e.g. 6in4 or 6rd). It would also happen in the case that IPv4 access is via a tunnel on an otherwise IPv6 only local network, which is a setup that might become more common in the post IPv4 address exhaustion world. In turns out there's no real need for passt/pasta to get its IPv4 and IPv6 connectivity via the same interface, so we can handle this situation fairly easily. Change the core to allow eparate external interfaces for IPv4 and IPv6. We don't actually set these separately for now. Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
* tcp: Silence warning from gcc 11.3 with -OfastStefano Brivio2022-06-081-2/+8
| | | | | | | If the first packet_get() call doesn't assign len, the second one will also return NULL, but gcc doesn't see this. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Work around gcc 12 bogus warning in tcp_rtt_dst_check()Stefano Brivio2022-05-201-0/+6
| | | | | | | | | | | | | | | | | | | gcc 12.1.x (e.g. current OpenSUSE Tumbleweed, x86_64 only, gcc-12-1.4.x86_64) reports: tcp.c: In function ‘tcp_send_flag’: tcp.c:1014:9: warning: writing 16 bytes into a region of size 0 [-Wstringop-overflow=] 1014 | memcpy(low_rtt_dst + hole++, &conn->a.a6, sizeof(conn->a.a6)); | ^ tcp.c:559:24: note: at offset -16 into destination object ‘low_rtt_dst’ of size 128 559 | static struct in6_addr low_rtt_dst[LOW_RTT_TABLE_SIZE]; | but 'hole' can't be -1, because the low_rtt_dst table is guaranteed to have a hole: if we happened to write to the last entry, we'll go back to index 0 and clear that one. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf, tcp, udp: Allow address specification for forwarded portsStefano Brivio2022-05-011-27/+36
| | | | | | | | | | | | | This feature is available in slirp4netns but was missing in passt and pasta. Given that we don't do dynamic memory allocation, we need to bind sockets while parsing port configuration. This means we need to process all other options first, as they might affect addressing and IP version support. It also implies a minor rework of how TCP and UDP implementations bind sockets. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: False "Out-of-bounds read" positive, CWE-125Stefano Brivio2022-04-071-1/+5
| | | | | | | 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>
* tcp, tcp_splice: False "Negative array index read" positives, CWE-129Stefano Brivio2022-04-071-4/+8
| | | | | | A flag or event bit is always set by callers. Reported by Coverity. Signed-by-off: Stefano Brivio <sbrivio@redhat.com>
* tcp: Dereference null return value, CWE-476Stefano Brivio2022-04-071-1/+1
| | | | | | Not an issue with a sane kernel behaviour. Reported by Coverity. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* treewide: Unchecked return value from library, CWE-252Stefano Brivio2022-04-071-7/+12
| | | | | | | 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>
* tcp: False "Untrusted loop bound" positive, CWE-606Stefano Brivio2022-04-051-0/+2
| | | | | | | | 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>
* treewide: Invalid type in argument to printf format specifier, CWE-686Stefano Brivio2022-04-051-19/+19
| | | | | | Harmless except for two bad debugging prints. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tap, tcp, udp, icmp: Cut down on some oversized buffersStefano Brivio2022-03-291-10/+41
| | | | | | | | | 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>
* tcp: Fix warning by gcc 5.4 on ppc64le about comparison in CONN_OR_NULL()Stefano Brivio2022-03-291-13/+13
| | | | | | | ...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>
* treewide: Mark constant references as constStefano Brivio2022-03-291-44/+51
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* treewide: Packet abstraction with mandatory boundary checksStefano Brivio2022-03-291-195/+247
| | | | | | | | | | | | | | | | | | | | 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>
* tcp, tcp_splice: Use less awkward syntax to swap in/out sockets from poolsStefano Brivio2022-03-291-5/+4
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Fit struct tcp_conn into a single 64-byte cachelineStefano Brivio2022-03-291-135/+163
| | | | | | | | | | | | | | | | | | | | | | ...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>
* tcp_splice: Close sockets right away on high number of open filesStefano Brivio2022-03-291-0/+1
| | | | | | | | | | | | | 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>
* tcp: Rework timers to use timerfd instead of periodic bitmap scanStefano Brivio2022-03-291-231/+272
| | | | | | | | | | | | | | | | | | 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>
* tcp, udp, util: Enforce 24-bit limit on socket numbersStefano Brivio2022-03-291-0/+17
| | | | | | | 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>
* dhcpv6, tap, tcp: Use IN6_ARE_ADDR_EQUAL instead of open-coded memcmp()Stefano Brivio2022-03-281-6/+6
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Refactor to use events instead of states, split out spliced implementationStefano Brivio2022-03-281-1495/+851
| | | | | | | | | | | | | | | | | | | | | 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>
* seccomp, tcp: Add fcntl64 to pasta syscalls for armv6l, armv7lStefano Brivio2022-02-281-1/+1
| | | | Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, udp: Receive batching doesn't pay off when writing single frames to tapStefano Brivio2022-02-211-16/+20
| | | | | | | | | | | | | | | | 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>
* passt, pasta: Namespace-based sandboxing, defer seccomp policy applicationStefano Brivio2022-02-211-8/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* passt: Address new clang-tidy warnings from LLVM 13.0.1Stefano Brivio2022-01-301-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* passt, tap: Daemonise once socket is ready without waiting for connectionStefano Brivio2022-01-281-1/+1
| | | | | | | | | | | | | 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>
* seccomp: Add a number of alternate and per-arch syscallsStefano Brivio2022-01-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-05 05:54:58 +0000