aboutgitcodebugslistschat
path: root/util.h
Commit message (Collapse)AuthorAgeFilesLines
* flow: Helper to create sockets based on flowsideDavid Gibson2024-07-191-0/+3
| | | | | | | | | We have upcoming use cases where it's useful to create new bound socket based on information from the flow table. Add flowside_sock_l4() to do this for either PIF_HOST or PIF_SPLICE sockets. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Handle errors on UDP socketsDavid Gibson2024-07-171-0/+3
| | | | | | | | | | | | | | | | | | Currently we ignore all events other than EPOLLIN on UDP sockets. This means that if we ever receive an EPOLLERR event, we'll enter an infinite loop on epoll, because we'll never do anything to clear the error. Luckily that doesn't seem to have happened in practice, but it's certainly fragile. Furthermore changes in how we handle UDP sockets with the flow table mean we will start receiving error events. Add handling of EPOLLERR events. For now we just read the error from the error queue (thereby clearing the error state) and print a debug message. We can add more substantial handling of specific events in future if we want to. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: sock_l4() determine protocol from epoll type rather than the reverseDavid Gibson2024-07-051-1/+2
| | | | | | | | | | | | sock_l4() creates a socket of the given IP protocol number, and adds it to the epoll state. Currently it determines the correct tag for the epoll data based on the protocol. However, we have some future cases where we might want different semantics, and therefore epoll types, for sockets of the same protocol. So, change sock_l4() to take the epoll type as an explicit parameter, and determine the protocol from that. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Use 'long' to represent millisecond durationsDavid Gibson2024-06-071-1/+1
| | | | | | | | | | timespec_diff_ms() returns an int representing a duration in milliseconds. This will overflow in about 25 days when an int is 32 bits. The way we use this function, we're probably not going to get a result that long, but it's not outrageously implausible. Use a long for safety. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Use unsigned indices for bits in bitmapsDavid Gibson2024-06-071-3/+3
| | | | | | | | | A negative bit index in a bitmap doesn't make sense. Avoid this by construction by using unsigned indices. While we're there adjust bitmap_isset() to return a bool instead of an int. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* passt, util: Move opening of PID file to its own functionStefano Brivio2024-05-231-0/+1
| | | | | | | We won't call it from main() any longer: move it. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
* util: Rename write_pidfile() to pidfile_write()Stefano Brivio2024-05-231-1/+1
| | | | | | | | As I'm adding pidfile_open() in the next patch. The function comment didn't match, by the way. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
* util, tcp: Add helper to display socket addressesDavid Gibson2024-05-221-0/+14
| | | | | | | | | | | When reporting errors, we sometimes want to show a relevant socket address. Doing so by extracting the various relevant fields can be pretty awkward, so introduce a sockaddr_ntop() helper to make it simpler. For now we just have one user in tcp.c, but I have further upcoming patches which can make use of it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Replace TCP buffer structure by an iovec arrayLaurent Vivier2024-04-191-0/+3
| | | | | | | | | | | | | To be able to provide pointers to TCP headers and IP headers without worrying about alignment in the structure, split the structure into several arrays and point to each part of the frame using an iovec array. Using iovec also allows us to simply ignore the first entry when the vnet length header is not needed. Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* conf: Don't fail if the template interface doesn't have a MAC addressStefano Brivio2024-04-191-0/+1
| | | | | | | | | | | | | | | | ...simply resort to using locally-administered address (LAA) as host-side source, instead. Pick 02:00:00:00:00:00, to make it clear that we don't actually care about that address, and also to match the 00 (Administratively Assigned Identifier) quadrant of SLAP (RFC 8948). Otherwise, pasta refuses to start if the template is a tun or Wireguard interface. Link: https://bugs.passt.top/show_bug.cgi?id=49 Link: https://github.com/containers/podman/issues/22320 Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* pasta, util: Align stack area for clones to maximum natural alignmentStefano Brivio2024-04-191-1/+3
| | | | | | | | | | | | | | | | Given that we use this stack pointer as a location to store arbitrary data types from the cloned process, we need to guarantee that its alignment matches any of those possible data types. runsisi reports that pasta gets a SIGBUS in pasta_open_ns() on aarch64, where the alignment requirement for stack pointers is a 16 bytes (same as the size of a long double), and similar requirements actually apply to most architectures we run on. Reported-by: runsisi <runsisi@hust.edu.cn> Link: https://bugs.passt.top/show_bug.cgi?id=85 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* util: Add helper to return name of address familyDavid Gibson2024-04-051-0/+18
| | | | | | | | | | | | We have a few places where we want to include the name of the internet protocol version (IPv4 or IPv6) in a message, which we handle with an open-coded ?: expression. This seems like something that might be more widely useful, so make a trivial helper to return the correct string based on the address family. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* pcap: Use clock_gettime() instead of gettimeofday()Stefano Brivio2024-03-141-0/+1
| | | | | | | | | | | | | | | POSIX.1-2008 declared gettimeofday() as obsolete, but I'm a dinosaur. Usually, C libraries translate that to the clock_gettime() system call anyway, but this doesn't happen in Jon's environment, and, there, seccomp happily kills pasta(1) when started with --pcap, because we didn't add gettimeofday() to our seccomp profiles. Use clock_gettime() instead. Reported-by: Jon Maloy <jmaloy@redhat.com> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* util: move IP stuff from util.[ch] to ip.[ch]Laurent Vivier2024-03-061-76/+0
| | | | | | | | | | | | Introduce ip.[ch] file to encapsulate IP protocol handling functions and structures. Modify various files to include the new header ip.h when it's needed. Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Message-ID: <20240303135114.1023026-5-lvivier@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Allow IN4_IS_* macros to operate on untyped addressesDavid Gibson2024-02-291-4/+4
| | | | | | | | | | | The IN4_IS_*() macros expect a pointer to a struct in_addr. That makes sense, but sometimes we have an IPv4 address as a void * pointer or union type which makes these less convenient. Additionally, this doesn't match the behaviour of the standard library's IN6_IS_*() macros on which they're modelled, nor our own IN4_ARE_ADDR_EQUAL(). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Add write_remainder() helperDavid Gibson2024-02-291-0/+1
| | | | | | | | | | | | | | | | | We have several places where we want to write(2) a buffer or buffers and we handle short write()s by retrying until everything is successfully written. Add a helper for this in util.c. This version has some differences from the typical write_all() function. First, take an IO vector rather than a single buffer, because that will be useful for some of our cases. Second, allow it to take an parameter to skip the first n bytes of the given buffers. This will be useful for some of the cases we want, and also falls out quite naturally from the implementation. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> [sbrivio: Minor formatting fixes in write_remainder()] Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* treewide: Use sa_family_t for address family variablesDavid Gibson2024-02-271-1/+1
| | | | | | | | | | Sometimes we use sa_family_t for variables and parameters containing a socket address family, other times we use a plain int. Since sa_family_t is what's actually used in struct sockaddr and friends, standardise on that. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Use htonl_constant() in more placesDavid Gibson2023-12-271-2/+2
| | | | | | | | We might as well when we're passing a known constant value, giving the compiler the best chance to optimise things away. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* treewide: Add IN4ADDR_ANY_INIT macroDavid Gibson2023-12-271-0/+3
| | | | | | | | | | We already define IN4ADDR_LOOPBACK_INIT to initialise a struct in_addr to the loopback address, make a similar one for the unspecified / any address. This avoids messying things with the internal structure of struct in_addr where we don't care about it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp: Switch hash table to linear probing instead of chainingDavid Gibson2023-12-271-0/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently we deal with hash collisions by letting a hash bucket contain multiple entries, forming a linked list using an index in the connection structure. That's a pretty standard and simple approach, but in our case we can use an even simpler one: linear probing. Here if a hash bucket is occupied we just move onto the next one until we find a feww one. This slightly simplifies lookup and more importantly saves some precious bytes in the connection structure by removing the need for a link. It does require some additional complexity for hash removal. This approach can perform poorly with hash table load is high. However, we already size our hash table of pointers larger than the connection table, which puts an upper bound on the load. It's relatively cheap to decrease that bound if we find we need to. I adapted the linear probing operations from Knuth's The Art of Computer Programming, Volume 3, 2nd Edition. Specifically Algorithm L and Algorithm R in Section 6.4. Note that there is an error in Algorithm R as printed, see errata at [0]. [0] https://www-cs-faculty.stanford.edu/~knuth/all3-prepre.ps.gz Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: MAX_FROM_BITS() should be unsignedDavid Gibson2023-12-041-1/+1
| | | | | | | | | | | | | | | MAX_FROM_BITS() computes the maximum value representable in a number of bits. The expression for that is an unsigned value, but we explicitly cast it to a signed int. It looks like this is because one of the main users is for FD_REF_MAX, which is used to bound fd values, typically stored as a signed int. The value MAX_FROM_BITS() is calculating is naturally non-negative, though, so it makes more sense for it to be unsigned, and to move the case to the definition of FD_REF_MAX. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* port_fwd, util: Include additional headers to fix build with muslStefano Brivio2023-12-021-0/+1
| | | | | | | | | | | | | lseek() is declared in unistd.h, and stdio.h provides sscanf(). Include these two headers in port_fwd.c. SIGCHLD, even if used exclusively for clone(), is defined in signal.h: add the include to util.h, as NS_CALL needs it. Reported-by: lemmi <lemmi@nerd2nerd.org> Link: https://github.com/void-linux/void-packages/actions/runs/6999782606/job/19039526604#step:7:57 Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* port_fwd, util: Don't bind UDP ports with opposite-side bound TCP portsStefano Brivio2023-11-221-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When pasta periodically scans bound ports and binds them on the other side in order to forward traffic, we bind UDP ports for corresponding TCP port numbers, too, to support protocols and applications such as iperf3 which use UDP port numbers matching the ones used by the TCP data connection. If we scan UDP ports in order to bind UDP ports, we skip detection of the UDP ports we already bound ourselves, to avoid looping back our own ports. Same with scanning and binding TCP ports. But if we scan for TCP ports in order to bind UDP ports, we need to skip bound TCP ports too, otherwise, as David pointed out: - we find a bound TCP port on side A, and bind the corresponding TCP and UDP ports on side B - at the next periodic scan, we find that UDP port bound on side B, and we bind the corresponding UDP port on side A - at this point, we unbind that UDP port on side B: we would otherwise loop back our own port. To fix this, we need to avoid binding UDP ports that we already bound, on the other side, as a consequence of finding a corresponding bound TCP port. Reproducing this issue is straightforward: ./pasta -- iperf3 -s # Wait one second, then from another terminal: iperf3 -c ::1 -u Reported-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp> Analysed-by: David Gibson <david@gibson.dropbear.id.au> Fixes: 457ff122e33c ("udp,pasta: Periodically scan for ports to automatically forward") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Add open_in_ns() helperDavid Gibson2023-11-071-0/+1
| | | | | | | | | | Most of our helpers which need to enter the pasta network namespace are quite specialised. Add one which is rather general - it just open()s a given file in the namespace context and returns the fd back to the main namespace. This will have some future uses. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* port_fwd: Move automatic port forwarding code to port_fwd.[ch]David Gibson2023-11-071-2/+0
| | | | | | | | | | | | | The implementation of scanning /proc files to do automatic port forwarding is a bit awkwardly split between procfs_scan_listen() in util.c, get_bound_ports() and related functions in conf.c and the initial setup code in conf(). Consolidate all of this into port_fwd.h, which already has some related definitions, and a new port_fwd.c. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* cppcheck: Make many pointers constDavid Gibson2023-10-041-2/+2
| | | | | | | | | Newer versions of cppcheck (as of 2.12.0, at least) added a warning for pointers which could be declared to point at const data, but aren't. Based on that, make many pointers throughout the codebase const. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Consolidate and improve workarounds for clang-tidy issue 58992David Gibson2023-09-271-0/+41
| | | | | | | | | | | | | | | | | | | | | We have several workarounds for a clang-tidy bug where the checker doesn't recognize that a number of system calls write to - and therefore initialise - a socket address. We can't neatly use a suppression, because the bogus warning shows up some time after the actual system call, when we access a field of the socket address which clang-tidy erroneously thinks is uninitialised. Consolidate these workarounds into one place by using macros to implement wrappers around affected system calls which add a memset() of the sockaddr to silence clang-tidy. This removes the need for the individual memset() workarounds at the callers - and the somewhat longwinded explanatory comments. We can then use a #define to not include the hack in "real" builds, but only consider it for clang-tidy. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Avoid shadowing index(3)David Gibson2023-09-271-1/+1
| | | | | | | | | | | | | | | | | A classic gotcha of the standard C library is that its unwise to call any variable 'index' because it will shadow the standard string library function index(3). This can cause warnings from cppcheck amongst others, and it also means that if the variable is removed you tend to get confusing type errors (or sometimes nothing at all) instead of a nice simple "name is not defined" error. Strictly speaking this only occurs if <string.h> is included, but that is so common that as a rule it's best to just avoid it always. We have a number of places which hit this trap, so rename variables and parameters to avoid it. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Fix licensing information display in --version2023_09_08.05627dcStefano Brivio2023-09-081-2/+2
| | | | | | | The regular expression I used when relicensing to GPLv2+ missed this. Fixes: ca2749e1bd52 ("passt: Relicense to GPL 2.0, or any later version") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* tcp, udp: Don't include destination address in partially precomputed csumsDavid Gibson2023-08-221-1/+3
| | | | | | | | | | | | | | | | | | | | | | We partially prepopulate IP and TCP header structures including, amongst other things the destination address, which for IPv4 is always the known address of the guest/namespace. We partially precompute both the IPv4 header checksum and the TCP checksum based on this. In future we're going to want more flexibility with controlling the destination for IPv4 (as we already do for IPv6), so this precomputed value gets in the way. Therefore remove the IPv4 destination from the precomputed checksum and fold it into the checksum update when we actually send a packet. Doing this means we no longer need to recompute those partial sums when the destination address changes ({tcp,udp}_update_l2_buf()) and instead the computation can be moved to compile time. This means while we perform slightly more computations on each packet, we slightly reduce the amount of memory we need to access. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Make ns_enter() a void function and report setns() errorsDavid Gibson2023-08-041-1/+1
| | | | | | | | | | | | | | | | ns_enter() returns an integer... but it's always zero. If we actually fail the function doesn't return. Therefore it makes more sense for this to be a function returning void, and we can remove the cases where we pointlessly checked its return value. In addition ns_enter() is usually called from an ephemeral thread created by NS_CALL(). That means that the exit(EXIT_FAILURE) there usually won't be reported (since NS_CALL() doesn't wait() for the thread). So, use die() instead to print out some information in the unlikely event that our setns() here does fail. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util, conf: Add and use ns_is_init() helperStefano Brivio2023-05-231-0/+2
| | | | | | | | | | We'll need this in isolate_initial(). While at it, don't rely on BUFSIZ: the earlier issue we had with musl reminded me it's not a magic "everything will fit" value. Size the read buffer to what we actually need from uid_map, and check for the final newline too, because uid_map is organised in lines. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* passt: Relicense to GPL 2.0, or any later versionStefano Brivio2023-04-061-1/+1
| | | | | | | | | | | | | | | | | | | In practical terms, passt doesn't benefit from the additional protection offered by the AGPL over the GPL, because it's not suitable to be executed over a computer network. Further, restricting the distribution under the version 3 of the GPL wouldn't provide any practical advantage either, as long as the passt codebase is concerned, and might cause unnecessary compatibility dilemmas. Change licensing terms to the GNU General Public License Version 2, or any later version, with written permission from all current and past contributors, namely: myself, David Gibson, Laine Stump, Andrea Bolognani, Paul Holzinger, Richard W.M. Jones, Chris Kuhn, Florian Weimer, Giuseppe Scrivano, Stefan Hajnoczi, and Vasiliy Ulyanov. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Carry own definition of __bswap_constant{16,32}Stefano Brivio2023-03-091-0/+11
| | | | | | | | | | musl doesn't define those, use our own definition there. This is a trivial implementation, similar to what's shipped by e.g. uClibc, glibc, libiio. Reported-by: Chris Kuhn <kuhnchris+github@kuhnchris.eu> Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Fix definitions of SOCKET_MAX, TCP_MAX_CONNSStefano Brivio2023-02-271-0/+2
| | | | | | | | ...and, given that I keep getting this wrong, add a convenience macro, MAX_FROM_BITS(). Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Make assertions actually usefulDavid Gibson2023-02-121-1/+19
| | | | | | | | | | | | | | | | | | There are some places in passt/pasta which #include <assert.h> and make various assertions. If we hit these something has already gone wrong, but they're there so that we a useful message instead of cryptic misbehaviour if assumptions we thought were correct turn out not to be. Except.. the glibc implementation of assert() uses syscalls that aren't in our seccomp filter, so we'll get a SIGSYS before it actually prints the message. Work around this by adding our own ASSERT() implementation using our existing err() function to log the message, and an abort(). The abort() probably also won't work exactly right with seccomp, but once we've printed the message, dying with a SIGSYS works just as well as dying with a SIGABRT. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Use abstracted tap headerDavid Gibson2023-01-231-7/+0
| | | | | | | | | Update the UDP code to use the tap layer abstractions for initializing and updating the L2 and lower headers. This will make adding other tap backends in future easier. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Parameterize ethernet header initializer macroDavid Gibson2023-01-231-9/+2
| | | | | | | | | We have separate IPv4 and IPv6 versions of a macro to construct an initializer for ethernet headers. However, now that we have htons_constant it's easy to simply paramterize this with the ethernet protocol number. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util: Introduce hton*_constant() in place of #ifdefsDavid Gibson2023-01-231-26/+11
| | | | | | | | | | | We have several places where we have fairly ugly #ifdefs on __BYTE_ORDER where we need network order values in a constant expression (so we can't use htons() or htonl()). We can do this more cleanly by using a single __BYTE_ORDER ifdef to define htons_constant() and htonl_constant() macros, then using those in all the other places. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* udp: Pre-populate msg_names with local addressDavid Gibson2023-01-131-0/+7
| | | | | | | | | | | udp_splice_namebuf is now used only for spliced sending, and so it is only ever populated with the localhost address, either IPv4 or IPv6. So, replace the awkward initialization in udp_sock_handler_splice() with statically initialized versions for IPv4 and IPv6. We then just need to update the port number in udp_sock_handler_splice(). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util, pasta: Add do_clone() wrapper around __clone2() and clone()Stefano Brivio2022-11-161-3/+5
| | | | | | | | | | | | | | | 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@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Use typing to reduce chances of IPv4 endianness errorsDavid Gibson2022-11-041-2/+10
| | | | | | | | | | | | | | | | | | | 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>
* util: Set NS_FN_STACK_SIZE to one eighth of ulimit-reported maximum stack size2022_10_22.b68da10Stefano Brivio2022-10-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ...instead of one fourth. On the main() -> conf() -> nl_sock_init() call path, LTO from gcc 12 on (at least) x86_64 decides to inline... everything: nl_sock_init() is effectively part of main(), after commit 3e2eb4337bc0 ("conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()"). This means we exceed the maximum stack size, and we get SIGSEGV, under any condition, at start time, as reported by Andrea on a recent build for CentOS Stream 9. The calculation of NS_FN_STACK_SIZE, which is the stack size we reserve for clones, was previously obtained by dividing the maximum stack size by two, to avoid an explicit check on architecture (on PA-RISC, also known as hppa, the stack grows up, so we point the clone to the middle of this area), and then further divided by two to allow for any additional usage in the caller. Well, if there are essentially no function calls anymore, this is not enough. Divide it by eight, which is anyway much more than possibly needed by any clone()d callee. I think this is robust, so it's a fix in some sense. Strictly speaking, though, we have no formal guarantees that this isn't either too little or too much. What we should do, eventually: check cloned() callees, there are just thirteen of them at the moment. Note down any stack usage (they are mostly small helpers), bonus points for an automated way at build time, quadruple that or so, to allow for extreme clumsiness, and use as NS_FN_STACK_SIZE. Perhaps introduce a specific condition for hppa. Reported-by: Andrea Bolognani <abologna@redhat.com> Fixes: 3e2eb4337bc0 ("conf: Bind inbound ports with CAP_NET_BIND_SERVICE before isolate_user()") Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Replace FWRITE with a functionDavid Gibson2022-10-151-12/+1
| | | | | | | | | | | | | | In a few places we use the FWRITE() macro to open a file, replace it's contents with a given string and close it again. There's no real reason this needs to be a macro rather than just a function though. Turn it into a function 'write_file()' and make some ancillary cleanups while we're there: - Add a return code so the caller can handle giving a useful error message - Handle the case of short write()s (unlikely, but possible) - Add O_TRUNC, to make sure we replace the existing contents entirely Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* util.h: Add missing gcc pragma push before pragma popStefano Brivio2022-10-151-0/+1
| | | | | | | | | While building with clang: ./util.h:176:24: warning: pragma diagnostic pop could not pop, no matching push [-Wunknown-pragmas] Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
* Don't create 'tap' socket for ports that are bound to loopback onlyStefano Brivio2022-10-151-0/+3
| | | | | | | | | | | | | | | | | | | | | 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>
* conf, tcp, udp: Allow specification of interface to bind toStefano Brivio2022-10-151-1/+2
| | | | | | | | | | | | | | | | 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>
* conf, log, Makefile: Add versioning informationStefano Brivio2022-10-151-0/+8
| | | | | | | Add a --version option displaying that, and also include this information in the log files. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
* Move logging functions to a new file, log.cStefano Brivio2022-10-141-16/+0
| | | | | | | | 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>
* udp: Replace pragma to ignore bogus stringop-overread warning with workaroundStefano Brivio2022-09-291-23/+0
| | | | | | | | | | | | | | | | | | | Commit c318ffcb4c93 ("udp: Ignore bogus -Wstringop-overread for write() from gcc 12.1") uses a gcc pragma to ignore a bogus warning, which started appearing on gcc 12.1 (aarch64 and x86_64) due to: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103483 ...but gcc 12.2 has the same issue. Not just that: if LTO is enabled, the pragma itself is ignored (this wasn't the case with gcc 12.1), because of: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922 Drop the pragma, and assign a frame (in the networking sense) pointer from the offset of the Ethernet header in the buffer, then pass it to write() and pcap(), so that gcc doesn't obsess anymore with the fact that an Ethernet header is 14 bytes and we're sending more than that. Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-09 17:24:40 +0000