- dev - passt

[PATCH v3 0/1] Improve validation of --mtu option
by David Gibson 21 Feb '25

21 Feb '25

Here are some of the more straightforward parts of my patches making various MTU related fixes. These ones improve how we validate the --mtu option. Changes since v2: * Don't disable a protocol because the MTU is too small, just warn. It's only an advertised MTU, not an enforced MTU David Gibson (1): conf: Be more precise about minimum MTUs conf.c | 18 +++++++++++++++--- ip.h | 7 +++++++ util.h | 6 ------ 3 files changed, 22 insertions(+), 9 deletions(-) -- 2.48.1

1 1

[PATCH v3 0/2] Reconstruct ICMP headers for failed UDP connect
by Jon Maloy 21 Feb '25

21 Feb '25

Reconstruct incoming ICMP headers for failed UDP connect and forward back to local peer. v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. Jon Maloy (2): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap.c | 36 ++++++++++++++----- tap.h | 7 ++++ udp.c | 96 +++++++++++++++++++++++++++++++++++++++++++------- udp_internal.h | 3 +- udp_vu.c | 4 +-- 5 files changed, 121 insertions(+), 25 deletions(-) -- 2.48.1

2 7

[PATCH] contrib/selinux: Enable mapping guest memory for libvirt guests
by Stefano Brivio 20 Feb '25

20 Feb '25

This doesn't actually belong to passt's own policy: we should export an interface and libvirt's policy should use it, because passt's policy shouldn't be aware of svirt_image_t at all. However, libvirt doesn't maintain its own policy, which makes policy updates rather involved. Add this workaround to ensure --vhost-user is working in combination with libvirt, as it might take ages before we can get the proper rule in libvirt's policy. Reported-by: Laine Stump <laine(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/selinux/passt.te | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/contrib/selinux/passt.te b/contrib/selinux/passt.te index 8e6120e..f595079 100644 --- a/contrib/selinux/passt.te +++ b/contrib/selinux/passt.te @@ -24,6 +24,12 @@ require { type tmpfs_t; type root_t; + # Workaround: passt --vhost-user needs to map guest memory, but + # libvirt doesn't maintain its own policy, which makes updates + # particularly complicated. To avoid breakage in the short term, + # deal with it in passt's own policy. + type svirt_image_t; + class file { ioctl getattr setattr create read write unlink open relabelto execute execute_no_trans map }; class dir { search write add_name remove_name mounton }; class chr_file { append read write open getattr ioctl }; @@ -131,3 +137,9 @@ allow passt_t user_tmp_t:dir { add_name write }; allow passt_t user_tmp_t:file { create open }; allow passt_t user_tmp_t:sock_file { create read write unlink }; allow passt_t unconfined_t:unix_stream_socket { read write }; + +# Workaround: passt --vhost-user needs to map guest memory, but +# libvirt doesn't maintain its own policy, which makes updates +# particularly complicated. To avoid breakage in the short term, +# deal with it in passt's own policy. +allow passt_t svirt_image_t:file { read write map }; -- 2.43.0

2 3

[PATCH v2] tap: always set the no_frag flag in IPv4 headers
by Jon Maloy 20 Feb '25

20 Feb '25

When studying the Linux source code and Wireshark dumps it seems like the no_frag flag in the IPv4 header is always set. Following discussions in the Internet on this subject indicates that modern routers never fragment packets, and that it isn't even supported in many cases. Adding to this that incoming messages forwarded on the tap interface never even pass through a router it seems safe to always set this flag. This makes the IPv4 headers of forwarded messages identical to those sent by the external sockets, something we must consider desirable. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: Updated checksum algorithm to consider the change --- ip.h | 3 ++- tap.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ip.h b/ip.h index 1544dbf..858cc89 100644 --- a/ip.h +++ b/ip.h @@ -36,13 +36,14 @@ .tos = 0, \ .tot_len = 0, \ .id = 0, \ - .frag_off = 0, \ + .frag_off = htons(IP_DF), \ .ttl = 0xff, \ .protocol = (proto), \ .saddr = 0, \ .daddr = 0, \ } #define L2_BUF_IP4_PSUM(proto) ((uint32_t)htons_constant(0x4500) + \ + (uint32_t)htons_constant(IP_DF) + \ (uint32_t)htons(0xff00 | (proto))) diff --git a/tap.c b/tap.c index d0673e5..44b0fc0 100644 --- a/tap.c +++ b/tap.c @@ -153,7 +153,7 @@ static void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src, ip4h->tos = 0; ip4h->tot_len = htons(l3len); ip4h->id = 0; - ip4h->frag_off = 0; + ip4h->frag_off = htons(IP_DF); ip4h->ttl = 255; ip4h->protocol = proto; ip4h->saddr = src.s_addr; -- 2.48.1

2 1

[PATCH v4 0/2] Reconstruct ICMP headers for failed UDP connect
by Jon Maloy 20 Feb '25

20 Feb '25

Reconstruct incoming ICMP headers for failed UDP connect and forward back to local peer. v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson Jon Maloy (2): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap.c | 36 ++++++++++++++++------ tap.h | 7 +++++ udp.c | 81 ++++++++++++++++++++++++++++++++++++++++++-------- udp_internal.h | 2 +- udp_vu.c | 4 +-- 5 files changed, 106 insertions(+), 24 deletions(-) -- 2.48.1

1 2

[PATCH 0/3] Improve validation of --mtu option
by David Gibson 20 Feb '25

20 Feb '25

Here are some of the more straightforward parts of my patches making various MTU related fixes. These ones improve how we validate the --mtu option. David Gibson (3): conf: More thorough error checking when parsing --mtu option conf: Use 0 instead of -1 as "unassigned" mtu value conf: Be more precise about minimum MTUs conf.c | 40 ++++++++++++++++++++++++++-------------- dhcp.c | 2 +- ip.h | 7 +++++++ ndp.c | 2 +- passt.h | 3 ++- pasta.c | 2 +- tcp.c | 2 +- util.h | 3 --- 8 files changed, 39 insertions(+), 22 deletions(-) -- 2.48.1

2 10

[PATCH] tcp_vu: head_cnt need not be global
by David Gibson 20 Feb '25

20 Feb '25

head_cnt is a global variable which tracks how many entries in head[] are currently used. The fact that it's global obscures the fact that the lifetime over which it has a meaningful value is quite short: a single call to of tcp_vu_data_from_sock(). Make it a local to tcp_vu_data_from_sock() to make that lifetime clearer. We keep the head[] array global for now - although technically it has the same valid lifetime - because it's large enough we might not want to put it on the stack. Cc: Laurent Vivier <lvivier(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp_vu.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tcp_vu.c b/tcp_vu.c index 0622f173..6891ed13 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -38,7 +38,6 @@ static struct iovec iov_vu[VIRTQUEUE_MAX_SIZE + 1]; static struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE]; static int head[VIRTQUEUE_MAX_SIZE + 1]; -static int head_cnt; /** * tcp_vu_hdrlen() - return the size of the header in level 2 frame (TCP) @@ -183,7 +182,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) static ssize_t tcp_vu_sock_recv(const struct ctx *c, const struct tcp_tap_conn *conn, bool v6, uint32_t already_sent, size_t fillsize, - int *iov_cnt) + int *iov_cnt, int *head_cnt) { struct vu_dev *vdev = c->vdev; struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE]; @@ -202,7 +201,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, vu_init_elem(elem, &iov_vu[1], VIRTQUEUE_MAX_SIZE); elem_cnt = 0; - head_cnt = 0; + *head_cnt = 0; while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE) { struct iovec *iov; size_t frame_size, dlen; @@ -221,7 +220,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, ASSERT(iov->iov_len >= hdrlen); iov->iov_base = (char *)iov->iov_base + hdrlen; iov->iov_len -= hdrlen; - head[head_cnt++] = elem_cnt; + head[(*head_cnt)++] = elem_cnt; fillsize -= dlen; elem_cnt += cnt; @@ -261,17 +260,18 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, len -= iov->iov_len; } /* adjust head count */ - while (head_cnt > 0 && head[head_cnt - 1] >= i) - head_cnt--; + while (*head_cnt > 0 && head[*head_cnt - 1] >= i) + (*head_cnt)--; + /* mark end of array */ - head[head_cnt] = i; + head[*head_cnt] = i; *iov_cnt = i; /* release unused buffers */ vu_queue_rewind(vq, elem_cnt - i); /* restore space for headers in iov */ - for (i = 0; i < head_cnt; i++) { + for (i = 0; i < *head_cnt; i++) { struct iovec *iov = &elem[head[i]].in_sg[0]; iov->iov_base = (char *)iov->iov_base - hdrlen; @@ -357,11 +357,11 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) struct vu_dev *vdev = c->vdev; struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE]; ssize_t len, previous_dlen; + int i, iov_cnt, head_cnt; size_t hdrlen, fillsize; int v6 = CONN_V6(conn); uint32_t already_sent; const uint16_t *check; - int i, iov_cnt; if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) { debug("Got packet, but RX virtqueue not usable yet"); @@ -396,7 +396,8 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) /* collect the buffers from vhost-user and fill them with the * data from the socket */ - len = tcp_vu_sock_recv(c, conn, v6, already_sent, fillsize, &iov_cnt); + len = tcp_vu_sock_recv(c, conn, v6, already_sent, fillsize, + &iov_cnt, &head_cnt); if (len < 0) { if (len != -EAGAIN && len != -EWOULDBLOCK) { tcp_rst(c, conn); -- 2.48.1

3 4

[PATCH v2 0/1] Improve validation of --mtu option
by David Gibson 20 Feb '25

20 Feb '25

Here are some of the more straightforward parts of my patches making various MTU related fixes. These ones improve how we validate the --mtu option. David Gibson (1): conf: Be more precise about minimum MTUs conf.c | 26 +++++++++++++++++++------- ip.h | 7 +++++++ util.h | 6 ------ 3 files changed, 26 insertions(+), 13 deletions(-) -- 2.48.1

1 1

[PATCH] conf: Unify several paths in conf_ports()
by David Gibson 20 Feb '25

20 Feb '25

In conf_ports() we have three different paths which actually do the setup of an individual forwarded port: one for the "all" case, one for the exclusions only case and one for the range of ports with possible exclusions case. We can unify those cases using a new helper which handles a single range of ports, with a bitmap of exclusions. Although this is slightly longer (largely due to the new helpers function comment), it reduces duplicated logic. It will also make future improvements to the tracking of port forwards easier. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 175 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 90 insertions(+), 85 deletions(-) diff --git a/conf.c b/conf.c index 18017f51..f862845b 100644 --- a/conf.c +++ b/conf.c @@ -123,6 +123,75 @@ static int parse_port_range(const char *s, char **endptr, return 0; } +/** + * conf_ports_range_except() - Set up forwarding for a range of ports minus a + * bitmap of exclusions + * @c: Execution context + * @optname: Short option name, t, T, u, or U + * @optarg: Option argument (port specification) + * @fwd: Pointer to @fwd_ports to be updated + * @addr: Listening address + * @ifname: Listening interface + * @first: First port to forward + * @last: Last port to forward + * @exclude: Bitmap of ports to exclude + * @to: Port to translate @first to when forwarding + * @weak: Ignore errors, as long as at least one port is mapped + */ +static void conf_ports_range_except(const struct ctx *c, char optname, + const char *optarg, struct fwd_ports *fwd, + const union inany_addr *addr, + const char *ifname, + uint16_t first, uint16_t last, + const uint8_t *exclude, uint16_t to, + bool weak) +{ + bool bound_one = false; + unsigned i; + int ret; + + if (first == 0) { + die("Can't forward port 0 for option '-%c %s'", + optname, optarg); + } + + for (i = first; i <= last; i++) { + if (bitmap_isset(exclude, i)) + continue; + + if (bitmap_isset(fwd->map, i)) { + warn( +"Altering mapping of already mapped port number: %s", optarg); + } + + bitmap_set(fwd->map, i); + fwd->delta[i] = to - first; + + if (optname == 't') + ret = tcp_sock_init(c, addr, ifname, i); + else if (optname == 'u') + ret = udp_sock_init(c, 0, addr, ifname, i); + else + /* No way to check in advance for -T and -U */ + ret = 0; + + if (ret == -ENFILE || ret == -EMFILE) { + die("Can't open enough sockets for port specifier: %s", + optarg); + } + + if (!ret) { + bound_one = true; + } else if (!weak) { + die("Failed to bind port %u (%s) for option '-%c %s'", + i, strerror_(-ret), optname, optarg); + } + } + + if (!bound_one) + die("Failed to bind any port for '-%c %s'", optname, optarg); +} + /** * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets * @c: Execution context @@ -135,10 +204,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, { union inany_addr addr_buf = inany_any6, *addr = &addr_buf; char buf[BUFSIZ], *spec, *ifname = NULL, *p; - bool exclude_only = true, bound_one = false; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; + bool exclude_only = true; unsigned i; - int ret; if (!strcmp(optarg, "none")) { if (fwd->mode) @@ -173,32 +241,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, fwd->mode = FWD_ALL; - /* Skip port 0. It has special meaning for many socket APIs, so - * trying to bind it is not really safe. - */ - for (i = 1; i < NUM_PORTS; i++) { + /* Also exclude ephemeral ports */ + for (i = 0; i < NUM_PORTS; i++) if (fwd_port_is_ephemeral(i)) - continue; - - bitmap_set(fwd->map, i); - if (optname == 't') { - ret = tcp_sock_init(c, NULL, NULL, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } else if (optname == 'u') { - ret = udp_sock_init(c, 0, NULL, NULL, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } - } - - if (!bound_one) - goto bind_all_fail; - + bitmap_set(exclude, i); + conf_ports_range_except(c, optname, optarg, fwd, + NULL, NULL, + 1, NUM_PORTS - 1, exclude, + 1, true); return; } @@ -275,37 +325,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } while ((p = next_chunk(p, ','))); if (exclude_only) { - /* Skip port 0. It has special meaning for many socket APIs, so - * trying to bind it is not really safe. - */ - for (i = 1; i < NUM_PORTS; i++) { - if (fwd_port_is_ephemeral(i) || - bitmap_isset(exclude, i)) - continue; - - bitmap_set(fwd->map, i); - - if (optname == 't') { - ret = tcp_sock_init(c, addr, ifname, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } else if (optname == 'u') { - ret = udp_sock_init(c, 0, addr, ifname, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } else { - /* No way to check in advance for -T and -U */ - bound_one = true; - } - } - - if (!bound_one) - goto bind_all_fail; - + /* Exclude ephemeral ports */ + for (i = 0; i < NUM_PORTS; i++) + if (fwd_port_is_ephemeral(i)) + bitmap_set(exclude, i); + conf_ports_range_except(c, optname, optarg, fwd, + addr, ifname, + 1, NUM_PORTS - 1, exclude, + 1, true); return; } @@ -334,40 +361,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */ goto bad; - for (i = orig_range.first; i <= orig_range.last; i++) { - if (bitmap_isset(fwd->map, i)) - warn( -"Altering mapping of already mapped port number: %s", optarg); - - if (bitmap_isset(exclude, i)) - continue; - - bitmap_set(fwd->map, i); - - fwd->delta[i] = mapped_range.first - orig_range.first; - - ret = 0; - if (optname == 't') - ret = tcp_sock_init(c, addr, ifname, i); - else if (optname == 'u') - ret = udp_sock_init(c, 0, addr, ifname, i); - if (ret) - goto bind_fail; - } + conf_ports_range_except(c, optname, optarg, fwd, + addr, ifname, + orig_range.first, orig_range.last, + exclude, + mapped_range.first, false); } while ((p = next_chunk(p, ','))); return; -enfile: - die("Can't open enough sockets for port specifier: %s", optarg); bad: die("Invalid port specifier %s", optarg); mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); -bind_fail: - die("Failed to bind port %u (%s) for option '-%c %s', exiting", - i, strerror_(-ret), optname, optarg); -bind_all_fail: - die("Failed to bind any port for '-%c %s', exiting", optname, optarg); } /** -- 2.48.1

2 2

[PATCH] dhcp: Add option code byte in calculation for OPT_MAX boundary check
by Stefano Brivio 19 Feb '25

19 Feb '25

Otherwise we'll limit messages to 577 bytes, instead of 576 bytes as intended: $ fqdn="thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.then_make_it_251_with_this" $ hostname="__eighteen_bytes__" $ ./pasta --fqdn ${fqdn} -H ${hostname} -p dhcp.pcap -- /sbin/dhclient -4 Saving packet capture to dhcp.pcap $ tshark -r dhcp.pcap -V -Y 'dhcp.option.value == 5' | grep "Total Length" Total Length: 577 This was hidden by the issue fixed by commit bcc4908c2b4a ("dhcp Remove option 255 length byte") until now. Fixes: 31e8109a86ee ("dhcp, dhcpv6: Add hostname and client fqdn ops") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- dhcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhcp.c b/dhcp.c index 4a209f1..b7d5ea3 100644 --- a/dhcp.c +++ b/dhcp.c @@ -143,7 +143,7 @@ static bool fill_one(struct msg *m, int o, int *offset) size_t slen = opts[o].slen; /* If we don't have space to write the option, then just skip */ - if (*offset + 1 /* length of option */ + slen > OPT_MAX) + if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) return true; m->o[*offset] = o; -- 2.43.0

3 2