From: David Gibson Although fwd_rule_add() performs some sanity checks on the rule it is given, there are invalid rules we don't check for, assuming that its callers will do that. That won't be enough when we can get rules inserted by a dynamic update client without going through the existing parsing code. So, add stricter checks to fwd_rule_add(), which is now possible thanks to the capabilities bits in the struct fwd_table. Where those duplicate existing checks in the callers, remove the old copies. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- conf.c | 21 --------------------- fwd.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/conf.c b/conf.c index 6e884e5..b470b0d 100644 --- a/conf.c +++ b/conf.c @@ -176,8 +176,6 @@ static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto, die("Invalid interface name: %s", ifname); } - assert(first != 0); - for (base = first; base <= last; base++) { if (exclude && bitmap_isset(exclude, base)) continue; @@ -310,10 +308,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, if (p != ep) /* Garbage after the ranges */ goto bad; - if (orig_range.first == 0) { - die("Can't forward port 0 included in '%s'", spec); - } - conf_ports_range_except(fwd, proto, addr, ifname, orig_range.first, orig_range.last, exclude, @@ -356,11 +350,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) return; } - if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) - die("TCP port forwarding requested but TCP is disabled"); - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) - die("UDP port forwarding requested but UDP is disabled"); - strncpy(buf, optarg, sizeof(buf) - 1); if ((spec = strchr(buf, '/'))) { @@ -405,16 +394,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) addr = NULL; } - if (addr) { - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { - die("IPv4 is disabled, can't use -%c %s", - optname, optarg); - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { - die("IPv6 is disabled, can't use -%c %s", - optname, optarg); - } - } - if (optname == 'T' || optname == 'U') { assert(!addr && !ifname); diff --git a/fwd.c b/fwd.c index c7fd1a9..979c149 100644 --- a/fwd.c +++ b/fwd.c @@ -367,17 +367,59 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) new->first, new->last); return -EINVAL; } + if (!new->first) { + warn("Forwarding rule attempts to map from port 0"); + return -EINVAL; + } + if (!new->to || + (in_port_t)(new->to + new->last - new->first) < new->to) { + warn("Forwarding rule attempts to map to port 0"); + return -EINVAL; + } if (new->flags & ~allowed_flags) { warn("Rule has invalid flags 0x%hhx", new->flags & ~allowed_flags); return -EINVAL; } - if (new->flags & FWD_DUAL_STACK_ANY && - !inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN]; - warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); return -EINVAL; } -- 2.43.0

From: David Gibson The code parsing command line options into forwarding rules has now been decoupled from most of passt/pasta's internals. This is good, because we'll soon want to share it with a configuration update client. Make the next step by moving this code into fwd_rule.[ch]. Signed-off-by: David Gibson Reviewed-by: Laurent Vivier Signed-off-by: Stefano Brivio --- conf.c | 376 +------------------------------------------ fwd.c | 94 ----------- fwd.h | 33 ---- fwd_rule.c | 464 ++++++++++++++++++++++++++++++++++++++++++++++++++++- fwd_rule.h | 36 ++++- 5 files changed, 502 insertions(+), 501 deletions(-) diff --git a/conf.c b/conf.c index b470b0d..5aacfe0 100644 --- a/conf.c +++ b/conf.c @@ -13,7 +13,6 @@ */ #include -#include #include #include #include @@ -66,365 +65,6 @@ const char *pasta_default_ifn = "tap0"; -/** - * port_range() - Represents a non-empty range of ports - * @first: First port number in the range - * @last: Last port number in the range (inclusive) - * - * Invariant: @last >= @first - */ -struct port_range { - in_port_t first, last; -}; - -/** - * parse_port_range() - Parse a range of port numbers '<first>[-<last>]' - * @s: String to parse - * @endptr: Update to the character after the parsed range (similar to - * strtol() etc.) - * @range: Update with the parsed values on success - * - * Return: -EINVAL on parsing error, -ERANGE on out of range port - * numbers, 0 on success - */ -static int parse_port_range(const char *s, const char **endptr, - struct port_range *range) -{ - unsigned long first, last; - char *ep; - - last = first = strtoul(s, &ep, 10); - if (ep == s) /* Parsed nothing */ - return -EINVAL; - if (*ep == '-') { /* we have a last value too */ - const char *lasts = ep + 1; - last = strtoul(lasts, &ep, 10); - if (ep == lasts) /* Parsed nothing */ - return -EINVAL; - } - - if ((last < first) || (last >= NUM_PORTS)) - return -ERANGE; - - range->first = first; - range->last = last; - *endptr = ep; - - return 0; -} - -/** - * parse_keyword() - Parse a literal keyword - * @s: String to parse - * @endptr: Update to the character after the keyword - * @kw: Keyword to accept - * - * Return: 0, if @s starts with @kw, -EINVAL if it does not - */ -static int parse_keyword(const char *s, const char **endptr, const char *kw) -{ - size_t len = strlen(kw); - - if (strlen(s) < len) - return -EINVAL; - - if (memcmp(s, kw, len)) - return -EINVAL; - - *endptr = s + len; - return 0; -} - -/** - * conf_ports_range_except() - Set up forwarding for a range of ports minus a - * bitmap of exclusions - * @fwd: Forwarding table to be updated - * @proto: Protocol to forward - * @addr: Listening address - * @ifname: Listening interface - * @first: First port to forward - * @last: Last port to forward - * @exclude: Bitmap of ports to exclude (may be NULL) - * @to: Port to translate @first to when forwarding - * @flags: Flags for forwarding entries - */ -static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto, - const union inany_addr *addr, - const char *ifname, - uint16_t first, uint16_t last, - const uint8_t *exclude, uint16_t to, - uint8_t flags) -{ - struct fwd_rule rule = { - .addr = addr ? *addr : inany_any6, - .ifname = { 0 }, - .proto = proto, - .flags = flags, - }; - char rulestr[FWD_RULE_STRLEN]; - unsigned delta = to - first; - unsigned base, i; - - if (!addr) - rule.flags |= FWD_DUAL_STACK_ANY; - if (ifname) { - int ret; - - ret = snprintf(rule.ifname, sizeof(rule.ifname), - "%s", ifname); - if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname)) - die("Invalid interface name: %s", ifname); - } - - for (base = first; base <= last; base++) { - if (exclude && bitmap_isset(exclude, base)) - continue; - - for (i = base; i <= last; i++) { - if (exclude && bitmap_isset(exclude, i)) - break; - } - - rule.first = base; - rule.last = i - 1; - rule.to = base + delta; - - fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); - if (fwd_rule_add(fwd, &rule) < 0) - goto fail; - - base = i - 1; - } - return; - -fail: - die("Unable to add rule %s", - fwd_rule_fmt(&rule, rulestr, sizeof(rulestr))); -} - -/* - * for_each_chunk - Step through delimited chunks of a string - * @p_: Pointer to start of each chunk (updated) - * @ep_: Pointer to end of each chunk (updated) - * @s_: String to step through - * @sep_: String of all allowed delimiters - */ -#define for_each_chunk(p_, ep_, s_, sep_) \ - for ((p_) = (s_); \ - (ep_) = (p_) + strcspn((p_), (sep_)), *(p_); \ - (p_) = *(ep_) ? (ep_) + 1 : (ep_)) - -/** - * conf_ports_spec() - Parse port range(s) specifier - * @fwd: Forwarding table to be updated - * @proto: Protocol to forward - * @addr: Listening address for forwarding - * @ifname: Interface name for listening - * @spec: Port range(s) specifier - */ -static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, - const union inany_addr *addr, const char *ifname, - const char *spec) -{ - uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - bool exclude_only = true; - const char *p, *ep; - uint8_t flags = 0; - unsigned i; - - if (!strcmp(spec, "all")) { - /* Treat "all" as equivalent to "": all non-ephemeral ports */ - spec = ""; - } - - /* Parse excluded ranges and "auto" in the first pass */ - for_each_chunk(p, ep, spec, ",") { - struct port_range xrange; - - if (isdigit(*p)) { - /* Include range, parse later */ - exclude_only = false; - continue; - } - - if (parse_keyword(p, &p, "auto") == 0) { - if (p != ep) /* Garbage after the keyword */ - goto bad; - - if (!(fwd->caps & FWD_CAP_SCAN)) { - die( -"'auto' port forwarding is only allowed for pasta"); - } - - flags |= FWD_SCAN; - continue; - } - - /* Should be an exclude range */ - if (*p != '~') - goto bad; - p++; - - if (parse_port_range(p, &p, &xrange)) - goto bad; - if (p != ep) /* Garbage after the range */ - goto bad; - - for (i = xrange.first; i <= xrange.last; i++) - bitmap_set(exclude, i); - } - - if (exclude_only) { - /* Exclude ephemeral ports */ - fwd_port_map_ephemeral(exclude); - - conf_ports_range_except(fwd, proto, addr, ifname, - 1, NUM_PORTS - 1, exclude, - 1, flags | FWD_WEAK); - return; - } - - /* Now process base ranges, skipping exclusions */ - for_each_chunk(p, ep, spec, ",") { - struct port_range orig_range, mapped_range; - - if (!isdigit(*p)) - /* Already parsed */ - continue; - - if (parse_port_range(p, &p, &orig_range)) - goto bad; - - if (*p == ':') { /* There's a range to map to as well */ - if (parse_port_range(p + 1, &p, &mapped_range)) - goto bad; - if ((mapped_range.last - mapped_range.first) != - (orig_range.last - orig_range.first)) - goto bad; - } else { - mapped_range = orig_range; - } - - if (p != ep) /* Garbage after the ranges */ - goto bad; - - conf_ports_range_except(fwd, proto, addr, ifname, - orig_range.first, orig_range.last, - exclude, - mapped_range.first, flags); - } - - return; -bad: - die("Invalid port specifier '%s'", spec); -} - -/** - * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets - * @optname: Short option name, t, T, u, or U - * @optarg: Option argument (port specification) - * @fwd: Forwarding table to be updated - */ -static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) -{ - union inany_addr addr_buf = inany_any6, *addr = &addr_buf; - char buf[BUFSIZ], *spec, *ifname = NULL; - uint8_t proto; - - if (optname == 't' || optname == 'T') - proto = IPPROTO_TCP; - else if (optname == 'u' || optname == 'U') - proto = IPPROTO_UDP; - else - assert(0); - - if (!strcmp(optarg, "none")) { - unsigned i; - - for (i = 0; i < fwd->count; i++) { - if (fwd->rules[i].proto == proto) { - die("-%c none conflicts with previous options", - optname); - } - } - return; - } - - strncpy(buf, optarg, sizeof(buf) - 1); - - if ((spec = strchr(buf, '/'))) { - *spec = 0; - spec++; - - if (optname != 't' && optname != 'u') - die("Listening address not allowed for -%c %s", - optname, optarg); - - if ((ifname = strchr(buf, '%'))) { - *ifname = 0; - ifname++; - - /* spec is already advanced one past the '/', - * so the length of the given ifname is: - * (spec - ifname - 1) - */ - if (spec - ifname - 1 >= IFNAMSIZ) { - die("Interface name '%s' is too long (max %u)", - ifname, IFNAMSIZ - 1); - } - } - - if (ifname == buf + 1) { /* Interface without address */ - addr = NULL; - } else { - char *p = buf; - - /* Allow square brackets for IPv4 too for convenience */ - if (*p == '[' && p[strlen(p) - 1] == ']') { - p[strlen(p) - 1] = '\0'; - p++; - } - - if (!inany_pton(p, addr)) - die("Bad forwarding address '%s'", p); - } - } else { - spec = buf; - - addr = NULL; - } - - if (optname == 'T' || optname == 'U') { - assert(!addr && !ifname); - - if (!(fwd->caps & FWD_CAP_IFNAME)) { - warn( -"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'", - optname, optarg); - - if (fwd->caps & FWD_CAP_IPV4) { - conf_ports_spec(fwd, proto, - &inany_loopback4, NULL, spec); - } - if (fwd->caps & FWD_CAP_IPV6) { - conf_ports_spec(fwd, proto, - &inany_loopback6, NULL, spec); - } - return; - } - - ifname = "lo"; - } - - if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) { - die( -"Device binding for '-%c %s' unsupported (requires kernel 5.7+)", - optname, optarg); - } - - conf_ports_spec(fwd, proto, addr, ifname, spec); -} - /** * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration * @c: Execution context @@ -2160,16 +1800,16 @@ void conf(struct ctx *c, int argc, char **argv) if (name == 't') { opt_t = true; - conf_ports(name, optarg, c->fwd[PIF_HOST]); + fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]); } else if (name == 'u') { opt_u = true; - conf_ports(name, optarg, c->fwd[PIF_HOST]); + fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]); } else if (name == 'T') { opt_T = true; - conf_ports(name, optarg, c->fwd[PIF_SPLICE]); + fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]); } else if (name == 'U') { opt_U = true; - conf_ports(name, optarg, c->fwd[PIF_SPLICE]); + fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]); } } while (name != -1); @@ -2221,13 +1861,13 @@ void conf(struct ctx *c, int argc, char **argv) if (c->mode == MODE_PASTA) { if (!opt_t) - conf_ports('t', "auto", c->fwd[PIF_HOST]); + fwd_rule_parse('t', "auto", c->fwd[PIF_HOST]); if (!opt_T) - conf_ports('T', "auto", c->fwd[PIF_SPLICE]); + fwd_rule_parse('T', "auto", c->fwd[PIF_SPLICE]); if (!opt_u) - conf_ports('u', "auto", c->fwd[PIF_HOST]); + fwd_rule_parse('u', "auto", c->fwd[PIF_HOST]); if (!opt_U) - conf_ports('U', "auto", c->fwd[PIF_SPLICE]); + fwd_rule_parse('U', "auto", c->fwd[PIF_SPLICE]); } if (!c->quiet) diff --git a/fwd.c b/fwd.c index a6d75b7..728a783 100644 --- a/fwd.c +++ b/fwd.c @@ -275,100 +275,6 @@ void fwd_rule_init(struct ctx *c) c->fwd[PIF_SPLICE] = &fwd_out; } -/** - * fwd_rule_add() - Validate and add a rule to a forwarding table - * @fwd: Table to add to - * @new: Rule to add - * - * Return: 0 on success, negative error code on failure - */ -int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) -{ - /* Flags which can be set from the caller */ - const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; - unsigned num = (unsigned)new->last - new->first + 1; - unsigned port; - - if (new->first > new->last) { - warn("Rule has invalid port range %u-%u", - new->first, new->last); - return -EINVAL; - } - if (!new->first) { - warn("Forwarding rule attempts to map from port 0"); - return -EINVAL; - } - if (!new->to || - (in_port_t)(new->to + new->last - new->first) < new->to) { - warn("Forwarding rule attempts to map to port 0"); - return -EINVAL; - } - if (new->flags & ~allowed_flags) { - warn("Rule has invalid flags 0x%hhx", - new->flags & ~allowed_flags); - return -EINVAL; - } - if (new->flags & FWD_DUAL_STACK_ANY) { - if (!inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; - - warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); - return -EINVAL; - } - if (!(fwd->caps & FWD_CAP_IPV4)) { - warn("Dual stack forward, but IPv4 not enabled"); - return -EINVAL; - } - if (!(fwd->caps & FWD_CAP_IPV6)) { - warn("Dual stack forward, but IPv6 not enabled"); - return -EINVAL; - } - } else { - if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { - warn("IPv4 forward, but IPv4 not enabled"); - return -EINVAL; - } - if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { - warn("IPv6 forward, but IPv6 not enabled"); - return -EINVAL; - } - } - if (new->proto == IPPROTO_TCP) { - if (!(fwd->caps & FWD_CAP_TCP)) { - warn("Can't add TCP forwarding rule, TCP not enabled"); - return -EINVAL; - } - } else if (new->proto == IPPROTO_UDP) { - if (!(fwd->caps & FWD_CAP_UDP)) { - warn("Can't add UDP forwarding rule, UDP not enabled"); - return -EINVAL; - } - } else { - warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", - new->proto, ipproto_name(new->proto)); - return -EINVAL; - } - - if (fwd->count >= ARRAY_SIZE(fwd->rules)) { - warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); - return -ENOSPC; - } - if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { - warn("Rules require too many listening sockets (maximum %u)", - ARRAY_SIZE(fwd->socks)); - return -ENOSPC; - } - - fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; - for (port = new->first; port <= new->last; port++) - fwd->rulesocks[fwd->count][port - new->first] = -1; - - fwd->rules[fwd->count++] = *new; - fwd->sock_count += num; - return 0; -} - /** * fwd_rule_match() - Does a prospective flow match a given forwarding rule? * @rule: Forwarding rule diff --git a/fwd.h b/fwd.h index e664d1d..8f845d0 100644 --- a/fwd.h +++ b/fwd.h @@ -20,8 +20,6 @@ struct flowside; -#define FWD_RULE_BITS 8 -#define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) #define FWD_NO_HINT (-1) /** @@ -36,36 +34,6 @@ struct fwd_listen_ref { unsigned rule :FWD_RULE_BITS; }; -/* Maximum number of listening sockets (per pif) - * - * Rationale: This lets us listen on every port for two addresses and two - * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a - * comfortable number of extras. - */ -#define MAX_LISTEN_SOCKS (NUM_PORTS * 5) - -/** - * struct fwd_table - Forwarding state (per initiating pif) - * @caps: Forwarding capabilities for this initiating pif - * @count: Number of forwarding rules - * @rules: Array of forwarding rules - * @rulesocks: Parallel array of @rules (@count valid entries) of pointers to - * @socks entries giving the start of the corresponding rule's - * sockets within the larger array - * @sock_count: Number of entries used in @socks (for all rules combined) - * @socks: Listening sockets for forwarding - */ -struct fwd_table { - uint32_t caps; - unsigned count; - struct fwd_rule rules[MAX_FWD_RULES]; - int *rulesocks[MAX_FWD_RULES]; - unsigned sock_count; - int socks[MAX_LISTEN_SOCKS]; -}; - -#define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8) - /** * struct fwd_scan - Port scanning state for a protocol+direction * @scan4: /proc/net fd to scan for IPv4 ports when in AUTO mode @@ -81,7 +49,6 @@ struct fwd_scan { #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ void fwd_rule_init(struct ctx *c); -int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new); const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, const struct flowside *ini, uint8_t proto, int hint); diff --git a/fwd_rule.c b/fwd_rule.c index 9d48982..cd3dec0 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -15,6 +15,7 @@ * Author: David Gibson */ +#include #include #include #include @@ -89,7 +90,7 @@ parse_err: * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap * @map: Bitmap to update */ -void fwd_port_map_ephemeral(uint8_t *map) +static void fwd_port_map_ephemeral(uint8_t *map) { unsigned port; @@ -123,6 +124,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) */ __attribute__((noinline)) #endif +/* cppcheck-suppress staticFunction */ const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size) { const char *percent = *rule->ifname ? "%" : ""; @@ -199,8 +201,8 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule * * @rules: Existing rules against which to test * @count: Number of rules in @rules */ -void fwd_rule_conflict_check(const struct fwd_rule *new, - const struct fwd_rule *rules, size_t count) +static void fwd_rule_conflict_check(const struct fwd_rule *new, + const struct fwd_rule *rules, size_t count) { unsigned i; @@ -215,3 +217,459 @@ void fwd_rule_conflict_check(const struct fwd_rule *new, fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr))); } } + +/** + * fwd_rule_add() - Validate and add a rule to a forwarding table + * @fwd: Table to add to + * @new: Rule to add + * + * Return: 0 on success, negative error code on failure + */ +static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) +{ + /* Flags which can be set from the caller */ + const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; + unsigned num = (unsigned)new->last - new->first + 1; + unsigned port; + + if (new->first > new->last) { + warn("Rule has invalid port range %u-%u", + new->first, new->last); + return -EINVAL; + } + if (!new->first) { + warn("Forwarding rule attempts to map from port 0"); + return -EINVAL; + } + if (!new->to || + (in_port_t)(new->to + new->last - new->first) < new->to) { + warn("Forwarding rule attempts to map to port 0"); + return -EINVAL; + } + if (new->flags & ~allowed_flags) { + warn("Rule has invalid flags 0x%hhx", + new->flags & ~allowed_flags); + return -EINVAL; + } + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN]; + + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); + return -EINVAL; + } + + if (fwd->count >= ARRAY_SIZE(fwd->rules)) { + warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); + return -ENOSPC; + } + if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { + warn("Rules require too many listening sockets (maximum %u)", + ARRAY_SIZE(fwd->socks)); + return -ENOSPC; + } + + fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; + for (port = new->first; port <= new->last; port++) + fwd->rulesocks[fwd->count][port - new->first] = -1; + + fwd->rules[fwd->count++] = *new; + fwd->sock_count += num; + return 0; +} + +/** + * port_range() - Represents a non-empty range of ports + * @first: First port number in the range + * @last: Last port number in the range (inclusive) + * + * Invariant: @last >= @first + */ +struct port_range { + in_port_t first, last; +}; + +/** + * parse_port_range() - Parse a range of port numbers '<first>[-<last>]' + * @s: String to parse + * @endptr: Update to the character after the parsed range (similar to + * strtol() etc.) + * @range: Update with the parsed values on success + * + * Return: -EINVAL on parsing error, -ERANGE on out of range port + * numbers, 0 on success + */ +static int parse_port_range(const char *s, const char **endptr, + struct port_range *range) +{ + unsigned long first, last; + char *ep; + + last = first = strtoul(s, &ep, 10); + if (ep == s) /* Parsed nothing */ + return -EINVAL; + if (*ep == '-') { /* we have a last value too */ + const char *lasts = ep + 1; + last = strtoul(lasts, &ep, 10); + if (ep == lasts) /* Parsed nothing */ + return -EINVAL; + } + + if ((last < first) || (last >= NUM_PORTS)) + return -ERANGE; + + range->first = first; + range->last = last; + *endptr = ep; + + return 0; +} + +/** + * parse_keyword() - Parse a literal keyword + * @s: String to parse + * @endptr: Update to the character after the keyword + * @kw: Keyword to accept + * + * Return: 0, if @s starts with @kw, -EINVAL if it does not + */ +static int parse_keyword(const char *s, const char **endptr, const char *kw) +{ + size_t len = strlen(kw); + + if (strlen(s) < len) + return -EINVAL; + + if (memcmp(s, kw, len)) + return -EINVAL; + + *endptr = s + len; + return 0; +} + +/** + * fwd_rule_range_except() - Set up forwarding for a range of ports minus a + * bitmap of exclusions + * @fwd: Forwarding table to be updated + * @proto: Protocol to forward + * @addr: Listening address + * @ifname: Listening interface + * @first: First port to forward + * @last: Last port to forward + * @exclude: Bitmap of ports to exclude (may be NULL) + * @to: Port to translate @first to when forwarding + * @flags: Flags for forwarding entries + */ +static void fwd_rule_range_except(struct fwd_table *fwd, uint8_t proto, + const union inany_addr *addr, + const char *ifname, + uint16_t first, uint16_t last, + const uint8_t *exclude, uint16_t to, + uint8_t flags) +{ + struct fwd_rule rule = { + .addr = addr ? *addr : inany_any6, + .ifname = { 0 }, + .proto = proto, + .flags = flags, + }; + char rulestr[FWD_RULE_STRLEN]; + unsigned delta = to - first; + unsigned base, i; + + if (!addr) + rule.flags |= FWD_DUAL_STACK_ANY; + if (ifname) { + int ret; + + ret = snprintf(rule.ifname, sizeof(rule.ifname), + "%s", ifname); + if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname)) + die("Invalid interface name: %s", ifname); + } + + for (base = first; base <= last; base++) { + if (exclude && bitmap_isset(exclude, base)) + continue; + + for (i = base; i <= last; i++) { + if (exclude && bitmap_isset(exclude, i)) + break; + } + + rule.first = base; + rule.last = i - 1; + rule.to = base + delta; + + fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); + if (fwd_rule_add(fwd, &rule) < 0) + goto fail; + + base = i - 1; + } + return; + +fail: + die("Unable to add rule %s", + fwd_rule_fmt(&rule, rulestr, sizeof(rulestr))); +} + +/* + * for_each_chunk - Step through delimited chunks of a string + * @p_: Pointer to start of each chunk (updated) + * @ep_: Pointer to end of each chunk (updated) + * @s_: String to step through + * @sep_: String of all allowed delimiters + */ +#define for_each_chunk(p_, ep_, s_, sep_) \ + for ((p_) = (s_); \ + (ep_) = (p_) + strcspn((p_), (sep_)), *(p_); \ + (p_) = *(ep_) ? (ep_) + 1 : (ep_)) + +/** + * fwd_rule_parse_ports() - Parse port range(s) specifier + * @fwd: Forwarding table to be updated + * @proto: Protocol to forward + * @addr: Listening address for forwarding + * @ifname: Interface name for listening + * @spec: Port range(s) specifier + */ +static void fwd_rule_parse_ports(struct fwd_table *fwd, uint8_t proto, + const union inany_addr *addr, + const char *ifname, + const char *spec) +{ + uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; + bool exclude_only = true; + const char *p, *ep; + uint8_t flags = 0; + unsigned i; + + if (!strcmp(spec, "all")) { + /* Treat "all" as equivalent to "": all non-ephemeral ports */ + spec = ""; + } + + /* Parse excluded ranges and "auto" in the first pass */ + for_each_chunk(p, ep, spec, ",") { + struct port_range xrange; + + if (isdigit(*p)) { + /* Include range, parse later */ + exclude_only = false; + continue; + } + + if (parse_keyword(p, &p, "auto") == 0) { + if (p != ep) /* Garbage after the keyword */ + goto bad; + + if (!(fwd->caps & FWD_CAP_SCAN)) { + die( +"'auto' port forwarding is only allowed for pasta"); + } + + flags |= FWD_SCAN; + continue; + } + + /* Should be an exclude range */ + if (*p != '~') + goto bad; + p++; + + if (parse_port_range(p, &p, &xrange)) + goto bad; + if (p != ep) /* Garbage after the range */ + goto bad; + + for (i = xrange.first; i <= xrange.last; i++) + bitmap_set(exclude, i); + } + + if (exclude_only) { + /* Exclude ephemeral ports */ + fwd_port_map_ephemeral(exclude); + + fwd_rule_range_except(fwd, proto, addr, ifname, + 1, NUM_PORTS - 1, exclude, + 1, flags | FWD_WEAK); + return; + } + + /* Now process base ranges, skipping exclusions */ + for_each_chunk(p, ep, spec, ",") { + struct port_range orig_range, mapped_range; + + if (!isdigit(*p)) + /* Already parsed */ + continue; + + if (parse_port_range(p, &p, &orig_range)) + goto bad; + + if (*p == ':') { /* There's a range to map to as well */ + if (parse_port_range(p + 1, &p, &mapped_range)) + goto bad; + if ((mapped_range.last - mapped_range.first) != + (orig_range.last - orig_range.first)) + goto bad; + } else { + mapped_range = orig_range; + } + + if (p != ep) /* Garbage after the ranges */ + goto bad; + + fwd_rule_range_except(fwd, proto, addr, ifname, + orig_range.first, orig_range.last, + exclude, + mapped_range.first, flags); + } + + return; +bad: + die("Invalid port specifier '%s'", spec); +} + +/** + * fwd_rule_parse() - Parse port configuration option + * @optname: Short option name, t, T, u, or U + * @optarg: Option argument (port specification) + * @fwd: Forwarding table to be updated + */ +void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd) +{ + union inany_addr addr_buf = inany_any6, *addr = &addr_buf; + char buf[BUFSIZ], *spec, *ifname = NULL; + uint8_t proto; + + if (optname == 't' || optname == 'T') + proto = IPPROTO_TCP; + else if (optname == 'u' || optname == 'U') + proto = IPPROTO_UDP; + else + assert(0); + + if (!strcmp(optarg, "none")) { + unsigned i; + + for (i = 0; i < fwd->count; i++) { + if (fwd->rules[i].proto == proto) { + die("-%c none conflicts with previous options", + optname); + } + } + return; + } + + strncpy(buf, optarg, sizeof(buf) - 1); + + if ((spec = strchr(buf, '/'))) { + *spec = 0; + spec++; + + if (optname != 't' && optname != 'u') + die("Listening address not allowed for -%c %s", + optname, optarg); + + if ((ifname = strchr(buf, '%'))) { + *ifname = 0; + ifname++; + + /* spec is already advanced one past the '/', + * so the length of the given ifname is: + * (spec - ifname - 1) + */ + if (spec - ifname - 1 >= IFNAMSIZ) { + die("Interface name '%s' is too long (max %u)", + ifname, IFNAMSIZ - 1); + } + } + + if (ifname == buf + 1) { /* Interface without address */ + addr = NULL; + } else { + char *p = buf; + + /* Allow square brackets for IPv4 too for convenience */ + if (*p == '[' && p[strlen(p) - 1] == ']') { + p[strlen(p) - 1] = '\0'; + p++; + } + + if (!inany_pton(p, addr)) + die("Bad forwarding address '%s'", p); + } + } else { + spec = buf; + + addr = NULL; + } + + if (optname == 'T' || optname == 'U') { + assert(!addr && !ifname); + + if (!(fwd->caps & FWD_CAP_IFNAME)) { + warn( +"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'", + optname, optarg); + + if (fwd->caps & FWD_CAP_IPV4) { + fwd_rule_parse_ports(fwd, proto, + &inany_loopback4, NULL, + spec); + } + if (fwd->caps & FWD_CAP_IPV6) { + fwd_rule_parse_ports(fwd, proto, + &inany_loopback6, NULL, + spec); + } + return; + } + + ifname = "lo"; + } + + if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) { + die( +"Device binding for '-%c %s' unsupported (requires kernel 5.7+)", + optname, optarg); + } + + fwd_rule_parse_ports(fwd, proto, addr, ifname, spec); +} diff --git a/fwd_rule.h b/fwd_rule.h index 5c7b67a..f0f4efd 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -19,6 +19,7 @@ /* Number of ports for both TCP and UDP */ #define NUM_PORTS (1U << 16) +#define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8) /* Forwarding capability bits */ #define FWD_CAP_IPV4 BIT(0) @@ -54,8 +55,38 @@ struct fwd_rule { uint8_t flags; }; +#define FWD_RULE_BITS 8 +#define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) + +/* Maximum number of listening sockets (per pif) + * + * Rationale: This lets us listen on every port for two addresses and two + * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a + * comfortable number of extras. + */ +#define MAX_LISTEN_SOCKS (NUM_PORTS * 5) + +/** + * struct fwd_table - Forwarding state (per initiating pif) + * @caps: Forwarding capabilities for this initiating pif + * @count: Number of forwarding rules + * @rules: Array of forwarding rules + * @rulesocks: Parallel array of @rules (@count valid entries) of pointers to + * @socks entries giving the start of the corresponding rule's + * sockets within the larger array + * @sock_count: Number of entries used in @socks (for all rules combined) + * @socks: Listening sockets for forwarding + */ +struct fwd_table { + uint32_t caps; + unsigned count; + struct fwd_rule rules[MAX_FWD_RULES]; + int *rulesocks[MAX_FWD_RULES]; + unsigned sock_count; + int socks[MAX_LISTEN_SOCKS]; +}; + void fwd_probe_ephemeral(void); -void fwd_port_map_ephemeral(uint8_t *map); #define FWD_RULE_STRLEN \ (IPPROTO_STRLEN - 1 \ @@ -67,7 +98,6 @@ void fwd_port_map_ephemeral(uint8_t *map); const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size); void fwd_rules_info(const struct fwd_rule *rules, size_t count); -void fwd_rule_conflict_check(const struct fwd_rule *new, - const struct fwd_rule *rules, size_t count); +void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd); #endif /* FWD_RULE_H */ -- 2.43.0

From: David Gibson In pesto we're going to want several levels of error/warning messages, much like passt itself. Particularly as we start to share mode code between passt and pesto, we want to use a similar interface to emit those. However we don't want to use the same implementation - logging to a file or syslog doesn't make sense for the command line tool. To accomplish this loosely share log.h, but not log.c between pesto and passt. In fact, an #ifdef means even most of log.h isn't actually shared, but we do provide similar warn(), die() etc. macros. This includes the *_perror() variants, which need strerror(). However, we want to avoid allocations for pesto as we do for passt, and strerror() allocates in some libc versions. Therefore, also move our workaround for this to be shared with pesto. Signed-off-by: Stefano Brivio [dwg: Based on changes part of a larger patch by Stefano] Signed-off-by: David Gibson --- Makefile | 6 +++++- common.h | 32 ++++++++++++++++++++++++++++++ log.h | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- pesto.c | 14 ++++---------- util.h | 32 ------------------------------ 5 files changed, 99 insertions(+), 44 deletions(-) diff --git a/Makefile b/Makefile index 030681b..f6cec8a 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ vhost_user.h virtio.h vu_common.h QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h -PESTO_HEADERS = common.h pesto.h +PESTO_HEADERS = common.h pesto.h log.h C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -121,6 +121,7 @@ qrap: $(QRAP_SRCS) $(QRAP_HEADERS) passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h +pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ @@ -221,9 +222,12 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck %.cppcheck: $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^ +passt.cppcheck: BASE_CPPFLAGS += -UPESTO passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h +pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unmatchedSuppression pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h diff --git a/common.h b/common.h index a9c115a..2f2e6f1 100644 --- a/common.h +++ b/common.h @@ -21,4 +21,36 @@ /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) +/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * strerror_() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *strerror_(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" + #endif /* _COMMON_H */ diff --git a/log.h b/log.h index dbab006..1058ca5 100644 --- a/log.h +++ b/log.h @@ -6,8 +6,63 @@ #ifndef LOG_H #define LOG_H -#include #include +#include +#include + +#ifdef PESTO + +#include + +#include "common.h" + +extern bool debug_flag; + +#define msg(...) \ + do { \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, "\n"); \ + } while (0) + +#define msg_perror(...) \ + do { \ + int errno_ = errno; \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, ": %s\n", strerror_(errno_)); \ + } while (0) + +#define die(...) \ + do { \ + msg(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define die_perror(...) \ + do { \ + msg_perror(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define warn(...) msg(__VA_ARGS__) +#define warn_perror(...) msg_perror(__VA_ARGS__) +#define info(...) msg(__VA_ARGS__) +#define info_perror(...) msg_perror(__VA_ARGS__) + +#define debug(...) \ + do { \ + if (debug_flag) \ + msg(__VA_ARGS__); \ + } while (0) + +#define debug_perror_(...) \ + do { \ + if (debug_flag) \ + msg_perror(__VA_ARGS__); \ + } while (0) + +#else /* !PESTO */ + +#include #include #include @@ -109,4 +164,6 @@ void __openlog(const char *ident, int option, int facility); void logfile_init(const char *name, const char *path, size_t size); void __setlogmask(int mask); +#endif /* !PESTO */ + #endif /* LOG_H */ diff --git a/pesto.c b/pesto.c index 9f2fa5d..f0916e8 100644 --- a/pesto.c +++ b/pesto.c @@ -34,18 +34,12 @@ #include "common.h" #include "seccomp_pesto.h" #include "pesto.h" +#include "log.h" -static bool debug_flag = false; +bool debug_flag = false; static char stdout_buf[BUFSIZ]; -#define die(...) \ - do { \ - FPRINTF(stderr, __VA_ARGS__); \ - FPRINTF(stderr, "\n"); \ - exit(EXIT_FAILURE); \ - } while (0) - /** * usage() - Print usage, exit with given status code * @name: Executable name @@ -99,7 +93,7 @@ int main(int argc, char **argv) * breaking our seccomp profile. */ if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf))) - die("Failed to set stdout buffer"); + die_perror("Failed to set stdout buffer"); do { optname = getopt_long(argc, argv, optstring, options, NULL); @@ -126,7 +120,7 @@ int main(int argc, char **argv) if (argc - optind != 1) usage(argv[0], stderr, EXIT_FAILURE); - printf("debug_flag=%d, path=\"%s\"\n", debug_flag, argv[optind]); + debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]); die("pesto is not implemented yet"); } diff --git a/util.h b/util.h index 770ff93..e90be47 100644 --- a/util.h +++ b/util.h @@ -302,38 +302,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) void raw_random(void *buf, size_t buflen); -/* - * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, - * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs - * getrandom(2) and brk(2) as it allocates memory for the locale-translated - * error description, but our seccomp profiles forbid both. - * - * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get - * a static untranslated string. It's a GNU implementation, but also defined by - * bionic. - * - * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries - * not defining strerrordesc_np() are expected to provide strerror() - * implementations that are simple enough for us to call. - */ -__attribute__ ((weak)) const char *strerrordesc_np(int errnum); - -/** - * strerror_() - strerror() wrapper calling strerrordesc_np() if available - * @errnum: Error code - * - * Return: error description string - */ -static inline const char *strerror_(int errnum) -{ - if (strerrordesc_np) - return strerrordesc_np(errnum); - - return strerror(errnum); -} - -#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" - /* * Workarounds for https://github.com/llvm/llvm-project/issues/58992 * -- 2.43.0

From: David Gibson All current pif names are quite short, and we expect them to remain short when/if we allow arbitrary pifs. However, because of the structure of the current code we don't enforce any limit on the length. This will become more important with dynamic configuration updates, so start enforcing a length limit. Specifically we allow pif names to be up to 128 bytes (PIF_NAME_SIZE), including the terminating \0. This is more or less arbitrary, but seems like it should be comfortably enough for all the cases we have in mind. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- pif.c | 2 +- pif.h | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pif.c b/pif.c index 1e80724..d5e3161 100644 --- a/pif.c +++ b/pif.c @@ -17,7 +17,7 @@ #include "inany.h" #include "epoll_ctl.h" -const char *pif_type_str[] = { +const char pif_type_str[][PIF_NAME_SIZE] = { [PIF_NONE] = "<none>", [PIF_HOST] = "HOST", [PIF_TAP] = "TAP", diff --git a/pif.h b/pif.h index 7bb58e5..90dd3a3 100644 --- a/pif.h +++ b/pif.h @@ -35,7 +35,9 @@ enum pif_type { PIF_NUM_TYPES, }; -extern const char *pif_type_str[]; +/* Maxmimum size of a pif name, including \0 */ +#define PIF_NAME_SIZE (128) +extern const char pif_type_str[][PIF_NAME_SIZE]; static inline const char *pif_type(enum pif_type pt) { @@ -43,6 +45,7 @@ static inline const char *pif_type(enum pif_type pt) return pif_type_str[pt]; else return "?"; + static_assert(sizeof("?") <= PIF_NAME_SIZE); } static inline const char *pif_name(uint8_t pif) -- 2.43.0

From: David Gibson The new warnings in fwd_rule_add() have some not quite technically correct format specifiers. For some weird reason these don't trip warnings on passt itself, but do when we re-use this code in the upcoming configuration client. Fix them. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- fwd_rule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fwd_rule.c b/fwd_rule.c index 777282d..7fd20dd 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -208,7 +208,7 @@ static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) return -EINVAL; } if (new->flags & ~allowed_flags) { - warn("Rule has invalid flags 0x%hhx", + warn("Rule has invalid flags 0x%x", new->flags & ~allowed_flags); return -EINVAL; } @@ -267,11 +267,11 @@ static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) } if (fwd->count >= ARRAY_SIZE(fwd->rules)) { - warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); + warn("Too many rules (maximum %d)", ARRAY_SIZE(fwd->rules)); return -ENOSPC; } if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { - warn("Rules require too many listening sockets (maximum %u)", + warn("Rules require too many listening sockets (maximum %d)", ARRAY_SIZE(fwd->socks)); return -ENOSPC; } -- 2.43.0

From: David Gibson Extend the dynamic update protocol to expose the pif indices and names from a running passt/pasta to the pesto tool. pesto records that data and, if requested with a new --show flag, prints it out. Signed-off-by: David Gibson [sbrivio: In read_pif_conf(), force a redundant termination of the interface name, the existing check isn't obvious enough for static checkers] [sbrivio: Drop @resv_ left-over in description of struct pesto_pif_info, reported by Jon Maloy] Signed-off-by: Stefano Brivio --- Makefile | 1 + common.h | 2 + conf.c | 41 ++++++++++++++++ pesto.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++ pesto.h | 18 ++++++- pif.h | 4 +- serialise.c | 4 ++ serialise.h | 1 + util.h | 2 - 9 files changed, 201 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 1718ddb..6da76b4 100644 --- a/Makefile +++ b/Makefile @@ -223,6 +223,7 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^ passt.cppcheck: BASE_CPPFLAGS += -UPESTO +passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h diff --git a/common.h b/common.h index 2f2e6f1..45f66ea 100644 --- a/common.h +++ b/common.h @@ -53,4 +53,6 @@ static inline const char *strerror_(int errnum) #define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" +#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) + #endif /* _COMMON_H */ diff --git a/conf.c b/conf.c index 823e08d..3b2fe42 100644 --- a/conf.c +++ b/conf.c @@ -1925,6 +1925,43 @@ void conf(struct ctx *c, int argc, char **argv) static void conf_accept(struct ctx *c); +/** + * conf_send_rules() - Send current forwarding rules to config client (pesto) + * @c: Execution context + * @fd: Socket to the client + * + * Return: 0 on success, -1 on failure + * + * FIXME: So far only sends pif ids and names + */ +static int conf_send_rules(const struct ctx *c, int fd) +{ + unsigned pif; + + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { + struct pesto_pif_info info; + int rc; + + if (!c->fwd[pif]) + continue; + + assert(pif != PIF_NONE); + + rc = snprintf(info.name, sizeof(info.name), "%s", pif_name(pif)); + assert(rc >= 0 && (size_t)rc < sizeof(info.name)); + + if (write_u8(fd, pif) < 0) + return -1; + if (write_all_buf(fd, &info, sizeof(info)) < 0) + return -1; + } + + if (write_u8(fd, PIF_NONE) < 0) + return -1; + + return 0; +} + /** * conf_close() - Close configuration / control socket and clean up * @c: Execution context @@ -1968,6 +2005,7 @@ static void conf_accept(struct ctx *c) struct pesto_hello hello = { .magic = PESTO_SERVER_MAGIC, .version = htonl(PESTO_PROTOCOL_VERSION), + .pif_name_size = htonl(PIF_NAME_SIZE), }; union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; struct ucred uc = { 0 }; @@ -2009,6 +2047,9 @@ retry: "Warning: Using experimental unsupported configuration protocol"); } + if (conf_send_rules(c, fd) < 0) + goto fail; + return; fail: diff --git a/pesto.c b/pesto.c index 762cfe9..77244b3 100644 --- a/pesto.c +++ b/pesto.c @@ -60,6 +60,127 @@ static void usage(const char *name, FILE *f, int status) exit(status); } +/* Maximum number of pifs with rule tables */ +#define MAX_PIFS 3 + +struct pif_configuration { + uint8_t pif; + char name[PIF_NAME_SIZE]; +}; + +struct configuration { + uint32_t npifs; + struct pif_configuration pif[MAX_PIFS]; +}; + +/** + * pif_conf_by_num() - Find a pif's configuration by pif id + * @conf: Configuration description + * @pif: pif id + * + * Return: pointer to the pif_configuration for @pif, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_num(struct configuration *conf, + uint8_t pif) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (conf->pif[i].pif == pif) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pif_conf_by_name() - Find a pif's configuration by name + * @conf: Configuration description + * @name: Interface name + * + * Return: pif_configuration for pif named @name, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_name(struct configuration *conf, + const char *name) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (strcmp(conf->pif[i].name, name) == 0) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pesto_read_rules() - Read rulestate from passt/pasta + * @fd: Control socket + * @conf: Configuration description to update + */ +static bool read_pif_conf(int fd, struct configuration *conf) +{ + struct pif_configuration *pc; + struct pesto_pif_info info; + uint8_t pif; + + if (read_u8(fd, &pif) < 0) + die("Error reading from control socket"); + + if (pif == PIF_NONE) + return false; + + debug("Receiving config for PIF %"PRIu8, pif); + + if (conf->npifs >= ARRAY_SIZE(conf->pif)) { + die("passt has more pifs than pesto can manage (max %d)", + ARRAY_SIZE(conf->pif)); + } + + pc = &conf->pif[conf->npifs]; + pc->pif = pif; + + if (read_all_buf(fd, &info, sizeof(info)) < 0) + die("Error reading from control socket"); + + if (info.name[sizeof(info.name)-1]) + die("Interface name was not NULL terminated"); + /* Redundant, to make static checkers happy */ + info.name[sizeof(info.name) - 1] = '\0'; + + static_assert(sizeof(info.name) == sizeof(pc->name), + "Mismatching pif name lengths"); + memcpy(pc->name, info.name, sizeof(pc->name)); + + debug("PIF %"PRIu8": %s", pc->pif, pc->name); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_num(conf, pc->pif)) + die("Received duplicate interface identifier"); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_name(conf, pc->name)) + die("Received duplicate interface name"); + + conf->npifs++; + return true; +} + +/** + * show_conf() - Show current configuration obtained from passt/pasta + * @conf: Configuration description + */ +static void show_conf(const struct configuration *conf) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + const struct pif_configuration *pc = &conf->pif[i]; + printf(" %s\n", pc->name); + printf(" TBD\n"); + } +} + /** * main() - Dynamic reconfiguration client main program * @argc: Argument count @@ -80,6 +201,7 @@ int main(int argc, char **argv) { 0 }, }; struct sockaddr_un a = { AF_UNIX, "" }; + struct configuration conf = { 0 }; const char *optstring = "dh"; struct pesto_hello hello; struct sock_fprog prog; @@ -162,6 +284,18 @@ int main(int argc, char **argv) "Warning: Using experimental protocol version, client and server must match\n"); } + if (ntohl(hello.pif_name_size) != PIF_NAME_SIZE) { + die("Server has unexpected pif name size (%" + PRIu32" not %"PRIu32"\n", + ntohl(hello.pif_name_size), PIF_NAME_SIZE); + } + + while (read_pif_conf(s, &conf)) + ; + + printf("passt/pasta configuration (%s)\n", a.sun_path); + show_conf(&conf); + if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) die_perror("Error shutting down control socket"); diff --git a/pesto.h b/pesto.h index 92d4df3..1879759 100644 --- a/pesto.h +++ b/pesto.h @@ -17,18 +17,32 @@ /* Version 0 is reserved for unreleased / unsupported experimental versions */ #define PESTO_PROTOCOL_VERSION 0 +/* Maxmimum size of a pif name, including \0 */ +#define PIF_NAME_SIZE (128) +#define PIF_NONE 0 + /** * struct pesto_hello - Server introduction message - * @magic: PESTO_SERVER_MAGIC - * @version: Version number + * @magic: PESTO_SERVER_MAGIC + * @version: Version number + * @pif_name_size: Server's value for PIF_NAME_SIZE */ struct pesto_hello { char magic[8]; uint32_t version; + uint32_t pif_name_size; } __attribute__ ((__packed__)); static_assert(sizeof(PESTO_SERVER_MAGIC) == sizeof(((struct pesto_hello *)0)->magic), "PESTO_SERVER_MAGIC has wrong size"); +/** + * struct pesto_pif_info - Message with basic metadata about a pif + * @name: Name (\0 terminated) + */ +struct pesto_pif_info { + char name[PIF_NAME_SIZE]; +} __attribute__ ((__packed__)); + #endif /* PESTO_H */ diff --git a/pif.h b/pif.h index 90dd3a3..d770860 100644 --- a/pif.h +++ b/pif.h @@ -11,6 +11,7 @@ #include +#include "pesto.h" #include "epoll_type.h" union inany_addr; @@ -24,7 +25,7 @@ union sockaddr_inany; */ enum pif_type { /* Invalid or not present pif */ - PIF_NONE = 0, + PIF_NONE_ = PIF_NONE, /* Host socket interface */ PIF_HOST, /* Qemu socket or namespace tuntap interface */ @@ -36,7 +37,6 @@ enum pif_type { }; /* Maxmimum size of a pif name, including \0 */ -#define PIF_NAME_SIZE (128) extern const char pif_type_str[][PIF_NAME_SIZE]; static inline const char *pif_type(enum pif_type pt) diff --git a/serialise.c b/serialise.c index 346df99..e083112 100644 --- a/serialise.c +++ b/serialise.c @@ -121,6 +121,10 @@ int write_all_buf(int fd, const void *buf, size_t len) return write_all_buf(fd, &beval, sizeof(beval)); \ } +#define be8toh(x) (x) +#define htobe8(x) (x) + +SERIALISE_UINT(8) SERIALISE_UINT(32) #undef SERIALISE_UINT diff --git a/serialise.h b/serialise.h index a88f3de..4714f4c 100644 --- a/serialise.h +++ b/serialise.h @@ -16,6 +16,7 @@ int write_all_buf(int fd, const void *buf, size_t len); int read_u##bits(int fd, uint##bits##_t *val); \ int write_u##bits(int fd, uint##bits##_t val); +SERIALISE_UINT_DECL(8) SERIALISE_UINT_DECL(32) #endif /* SERIALISE_H */ diff --git a/util.h b/util.h index e90be47..c788382 100644 --- a/util.h +++ b/util.h @@ -87,8 +87,6 @@ void abort_with_msg(const char *fmt, ...) #define V6 1 #define IP_VERSIONS 2 -#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) - #define foreach(item, array) \ for ((item) = (array); (item) - (array) < ARRAY_SIZE(array); (item)++) -- 2.43.0

From: David Gibson Most things in ip.[ch] related purely to IP addresses and headers with no dependency on other passt/pasta internals. A number of these will be useful to re-use in pesto. The exception is ipv6_l4hdr() which uses iov_tail. The only caller of this is in tap.c, so move the function there. Along with moving the constant byteswapping functions to common.h, that lets ip.[ch] to be linked into pesto as well as passt/pasta. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- common.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ inany.h | 2 ++ ip.c | 56 +++----------------------------------------------------- ip.h | 4 +--- tap.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ util.h | 48 ------------------------------------------------ 6 files changed, 106 insertions(+), 104 deletions(-) diff --git a/common.h b/common.h index 45f66ea..4a167ae 100644 --- a/common.h +++ b/common.h @@ -55,4 +55,52 @@ static inline const char *strerror_(int errnum) #define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) +#ifndef __bswap_constant_16 +#define __bswap_constant_16(x) \ + ((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8))) +#endif + +#ifndef __bswap_constant_32 +#define __bswap_constant_32(x) \ + ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ + (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) +#endif + +#ifndef __bswap_constant_32 +#define __bswap_constant_32(x) \ + ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ + (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) +#endif + +#ifndef __bswap_constant_64 +#define __bswap_constant_64(x) \ + ((((x) & 0xff00000000000000ULL) >> 56) | \ + (((x) & 0x00ff000000000000ULL) >> 40) | \ + (((x) & 0x0000ff0000000000ULL) >> 24) | \ + (((x) & 0x000000ff00000000ULL) >> 8) | \ + (((x) & 0x00000000ff000000ULL) << 8) | \ + (((x) & 0x0000000000ff0000ULL) << 24) | \ + (((x) & 0x000000000000ff00ULL) << 40) | \ + (((x) & 0x00000000000000ffULL) << 56)) +#endif + +#if __BYTE_ORDER == __BIG_ENDIAN +#define htons_constant(x) (x) +#define htonl_constant(x) (x) +#define htonll_constant(x) (x) +#define ntohs_constant(x) (x) +#define ntohl_constant(x) (x) +#define ntohll_constant(x) (x) +#else +#define htons_constant(x) (__bswap_constant_16(x)) +#define htonl_constant(x) (__bswap_constant_32(x)) +#define htonll_constant(x) (__bswap_constant_64(x)) +#define ntohs_constant(x) (__bswap_constant_16(x)) +#define ntohl_constant(x) (__bswap_constant_32(x)) +#define ntohll_constant(x) (__bswap_constant_64(x)) +#endif + +#define ntohll(x) (be64toh((x))) +#define htonll(x) (htobe64((x))) + #endif /* _COMMON_H */ diff --git a/inany.h b/inany.h index 30e2416..1f7741d 100644 --- a/inany.h +++ b/inany.h @@ -10,8 +10,10 @@ #define INANY_H #include +#include #include +#include "util.h" #include "ip.h" #include "siphash.h" diff --git a/ip.c b/ip.c index 25fa407..f2506bb 100644 --- a/ip.c +++ b/ip.c @@ -6,6 +6,9 @@ * PASTA - Pack A Subtle Tap Abstraction * for network namespace/tap device mode * + * PESTO - Programmable Extensible Socket Translation Orchestrator + * front-end for passt(1) and pasta(1) forwarding configuration + * * ip.c - IP related functions * * Copyright (c) 2020-2021 Red Hat GmbH @@ -16,61 +19,8 @@ #include #include -#include "util.h" #include "ip.h" -#define IPV6_NH_OPT(nh) \ - ((nh) == 0 || (nh) == 43 || (nh) == 44 || (nh) == 50 || \ - (nh) == 51 || (nh) == 60 || (nh) == 135 || (nh) == 139 || \ - (nh) == 140 || (nh) == 253 || (nh) == 254) - -/** - * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol - * @data: IPv6 packet - * @proto: Filled with L4 protocol number - * @dlen: Data length (payload excluding header extensions), set on return - * - * Return: true if the L4 header is found and @data, @proto, @dlen are set, - * false on error. Outputs are indeterminate on failure. - */ -bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) -{ - struct ipv6_opt_hdr o_storage; - const struct ipv6_opt_hdr *o; - struct ipv6hdr ip6h_storage; - const struct ipv6hdr *ip6h; - int hdrlen; - uint8_t nh; - - ip6h = IOV_REMOVE_HEADER(data, ip6h_storage); - if (!ip6h) - return false; - - nh = ip6h->nexthdr; - if (!IPV6_NH_OPT(nh)) - goto found; - - while ((o = IOV_PEEK_HEADER(data, o_storage))) { - nh = o->nexthdr; - hdrlen = (o->hdrlen + 1) * 8; - - if (IPV6_NH_OPT(nh)) - iov_drop_header(data, hdrlen); - else - goto found; - } - - return false; - -found: - if (nh == IPPROTO_NONE) - return false; - - *dlen = iov_tail_size(data); - *proto = nh; - return true; -} - /** * ipproto_name() - Get IP protocol name from number * @proto: IP protocol number diff --git a/ip.h b/ip.h index fb4119a..aab9b86 100644 --- a/ip.h +++ b/ip.h @@ -9,7 +9,7 @@ #include #include -#include "util.h" +#include "common.h" #define IN4_IS_ADDR_UNSPECIFIED(a) \ (((struct in_addr *)(a))->s_addr == htonl_constant(INADDR_ANY)) @@ -117,8 +117,6 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h) ip6h->flow_lbl[2]; } -bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen); - #define IPPROTO_STRLEN (sizeof("<unknown protocol>")) const char *ipproto_name(uint8_t proto); diff --git a/tap.c b/tap.c index 7d06189..cfd7b64 100644 --- a/tap.c +++ b/tap.c @@ -874,6 +874,58 @@ append: return in->count; } +#define IPV6_NH_OPT(nh) \ + ((nh) == 0 || (nh) == 43 || (nh) == 44 || (nh) == 50 || \ + (nh) == 51 || (nh) == 60 || (nh) == 135 || (nh) == 139 || \ + (nh) == 140 || (nh) == 253 || (nh) == 254) + +/** + * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol + * @data: IPv6 packet + * @proto: Filled with L4 protocol number + * @dlen: Data length (payload excluding header extensions), set on return + * + * Return: true if the L4 header is found and @data, @proto, @dlen are set, + * false on error. Outputs are indeterminate on failure. + */ +static bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) +{ + struct ipv6_opt_hdr o_storage; + const struct ipv6_opt_hdr *o; + struct ipv6hdr ip6h_storage; + const struct ipv6hdr *ip6h; + int hdrlen; + uint8_t nh; + + ip6h = IOV_REMOVE_HEADER(data, ip6h_storage); + if (!ip6h) + return false; + + nh = ip6h->nexthdr; + if (!IPV6_NH_OPT(nh)) + goto found; + + while ((o = IOV_PEEK_HEADER(data, o_storage))) { + nh = o->nexthdr; + hdrlen = (o->hdrlen + 1) * 8; + + if (IPV6_NH_OPT(nh)) + iov_drop_header(data, hdrlen); + else + goto found; + } + + return false; + +found: + if (nh == IPPROTO_NONE) + return false; + + *dlen = iov_tail_size(data); + *proto = nh; + return true; +} + /** * tap6_handler() - IPv6 packet handler for tap file descriptor * @c: Execution context diff --git a/util.h b/util.h index c788382..dc14c78 100644 --- a/util.h +++ b/util.h @@ -101,54 +101,6 @@ void abort_with_msg(const char *fmt, ...) #define MAC_UNDEF MAC_BROADCAST #define MAC_IS_UNDEF(addr) (!memcmp((addr), MAC_UNDEF, ETH_ALEN)) -#ifndef __bswap_constant_16 -#define __bswap_constant_16(x) \ - ((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8))) -#endif - -#ifndef __bswap_constant_32 -#define __bswap_constant_32(x) \ - ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ - (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) -#endif - -#ifndef __bswap_constant_32 -#define __bswap_constant_32(x) \ - ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ - (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) -#endif - -#ifndef __bswap_constant_64 -#define __bswap_constant_64(x) \ - ((((x) & 0xff00000000000000ULL) >> 56) | \ - (((x) & 0x00ff000000000000ULL) >> 40) | \ - (((x) & 0x0000ff0000000000ULL) >> 24) | \ - (((x) & 0x000000ff00000000ULL) >> 8) | \ - (((x) & 0x00000000ff000000ULL) << 8) | \ - (((x) & 0x0000000000ff0000ULL) << 24) | \ - (((x) & 0x000000000000ff00ULL) << 40) | \ - (((x) & 0x00000000000000ffULL) << 56)) -#endif - -#if __BYTE_ORDER == __BIG_ENDIAN -#define htons_constant(x) (x) -#define htonl_constant(x) (x) -#define htonll_constant(x) (x) -#define ntohs_constant(x) (x) -#define ntohl_constant(x) (x) -#define ntohll_constant(x) (x) -#else -#define htons_constant(x) (__bswap_constant_16(x)) -#define htonl_constant(x) (__bswap_constant_32(x)) -#define htonll_constant(x) (__bswap_constant_64(x)) -#define ntohs_constant(x) (__bswap_constant_16(x)) -#define ntohl_constant(x) (__bswap_constant_32(x)) -#define ntohll_constant(x) (__bswap_constant_64(x)) -#endif - -#define ntohll(x) (be64toh((x))) -#define htonll(x) (htobe64((x))) - extern uint8_t eth_pad[ETH_ZLEN]; /** -- 2.43.0

From: David Gibson This adds parsing of options using fwd_rule_parse(), validates them and adds them to the existing rules. It doesn't yet send those rules back to passt or pasta. Signed-off-by: Stefano Brivio Message-ID: <20260322141843.4095972-3-sbrivio@redhat.com> [dwg: Based on an early draft by Stefano] Signed-off-by: David Gibson --- Makefile | 1 + fwd_rule.c | 2 +- fwd_rule.h | 1 + pesto.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 111 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 057e4eb..125ec01 100644 --- a/Makefile +++ b/Makefile @@ -227,6 +227,7 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck passt.cppcheck: BASE_CPPFLAGS += -UPESTO passt.cppcheck: CPPCHECK_FLAGS += \ --suppress=unusedFunction:fwd_rule.c \ + --suppress=staticFunction:fwd_rule.c \ --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h diff --git a/fwd_rule.c b/fwd_rule.c index da9d893..3c1eaa4 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -187,7 +187,7 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule * * * Return: 0 on success, negative error code on failure */ -static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) +int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) { /* Flags which can be set from the caller */ const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; diff --git a/fwd_rule.h b/fwd_rule.h index 330d49e..f43b37d 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -103,6 +103,7 @@ const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size); void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd); int fwd_rule_read(int fd, struct fwd_rule *rule); int fwd_rule_write(int fd, const struct fwd_rule *rule); +int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new); /** * fwd_rules_dump() - Dump forwarding rules diff --git a/pesto.c b/pesto.c index 4bf9bd8..95aecad 100644 --- a/pesto.c +++ b/pesto.c @@ -55,6 +55,43 @@ static void usage(const char *name, FILE *f, int status) FPRINTF(f, "Usage: %s [OPTION]... PATH\n", name); FPRINTF(f, "\n" + " -t, --tcp-ports SPEC TCP inbound port forwarding\n" + " can be specified multiple times\n" + " SPEC can be:\n" + " 'none': don't forward any ports\n" + " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" + " PORTS is either 'all' (forward all unbound, non-ephemeral\n" + " ports), or a comma-separated list of ports, optionally\n" + " ranged with '-' and optional target ports after ':'.\n" + " Ranges can be reduced by excluding ports or ranges\n" + " prefixed by '~'.\n" + " The 'auto' keyword may be given to only forward\n" + " ports which are bound in the target namespace\n" + " Examples:\n" + " -t all Forward all ports\n" + " -t 127.0.0.1/all Forward all ports from local address\n" + " 127.0.0.1\n" + " -t 22 Forward local port 22 to 22\n" + " -t 22:23 Forward local port 22 to 23\n" + " -t 22,25 Forward ports 22, 25 to ports 22, 25\n" + " -t 22-80 Forward ports 22 to 80\n" + " -t 22-80:32-90 Forward ports 22 to 80 to\n" + " corresponding port numbers plus 10\n" + " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1\n" + " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n" + " -t ~25 Forward all ports except for 25\n" + " -t auto Forward all ports bound in namespace\n" + " -t 192.0.2.2/auto Forward ports from 192.0.2.2 if\n" + " they are bound in the namespace\n" + " -t 8000-8010,auto Forward ports 8000-8010 if they\n" + " are bound in the namespace\n" + " -u, --udp-ports SPEC UDP inbound port forwarding\n" + " SPEC is as described for TCP above\n" + " -T, --tcp-ns SPEC TCP outbound port forwarding\n" + " SPEC is as described above\n" + " -U, --udp-ns SPEC UDP outbound port forwarding\n" + " SPEC is as described above\n" + " -s, --show Show configuration before and after\n" " -d, --debug Print debugging messages\n" " -h, --help Display this help message and exit\n" " --version Show version and exit\n"); @@ -204,6 +241,8 @@ static void show_conf(const struct configuration *conf) fwd_rules_dump(printf, pc->fwd.rules, pc->fwd.count, " ", "\n"); } + /* Flush stdout, so this doesn't get misordered with later debug()s */ + (void)fflush(stdout); } /** @@ -215,7 +254,7 @@ static void show_conf(const struct configuration *conf) * * #syscalls:pesto socket s390x:socketcall i686:socketcall * #syscalls:pesto connect shutdown close - * #syscalls:pesto exit_group fstat read write + * #syscalls:pesto exit_group fstat read write openat */ int main(int argc, char **argv) { @@ -223,11 +262,18 @@ int main(int argc, char **argv) {"debug", no_argument, NULL, 'd' }, {"help", no_argument, NULL, 'h' }, {"version", no_argument, NULL, 1 }, + {"tcp-ports", required_argument, NULL, 't' }, + {"udp-ports", required_argument, NULL, 'u' }, + {"tcp-ns", required_argument, NULL, 'T' }, + {"udp-ns", required_argument, NULL, 'U' }, + {"show", no_argument, NULL, 's' }, { 0 }, }; + struct pif_configuration *inbound, *outbound; struct sockaddr_un a = { AF_UNIX, "" }; + const char *optstring = "dht:u:T:U:s"; struct configuration conf = { 0 }; - const char *optstring = "dh"; + bool update = false, show = false; struct pesto_hello hello; struct sock_fprog prog; int optname, ret, s; @@ -248,6 +294,8 @@ int main(int argc, char **argv) if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf))) die_perror("Failed to set stdout buffer"); + fwd_probe_ephemeral(); + do { optname = getopt_long(argc, argv, optstring, options, NULL); @@ -255,6 +303,16 @@ int main(int argc, char **argv) case -1: case 0: break; + case 't': + case 'u': + case 'T': + case 'U': + /* Parse these options after we've read state from passt/pasta */ + update = true; + break; + case 's': + show = true; + break; case 'h': usage(argv[0], stdout, EXIT_SUCCESS); break; @@ -287,6 +345,8 @@ int main(int argc, char **argv) die_perror("Failed to connect to %s", a.sun_path); } + debug("Connected to passt/pasta control socket"); + ret = read_all_buf(s, &hello, sizeof(hello)); if (ret < 0) die_perror("Couldn't read server greeting"); @@ -324,11 +384,54 @@ int main(int argc, char **argv) while (read_pif_conf(s, &conf)) ; - printf("passt/pasta configuration (%s)\n", a.sun_path); - show_conf(&conf); + if (!update) { + printf("passt/pasta configuration (%s)\n", a.sun_path); + show_conf(&conf); + goto noupdate; + } + + if (show) { + printf("Previous configuration (%s)\n", a.sun_path); + show_conf(&conf); + } + + inbound = pif_conf_by_name(&conf, "HOST"); + outbound = pif_conf_by_name(&conf, "SPLICE"); + + optind = 0; + do { + optname = getopt_long(argc, argv, optstring, options, NULL); + switch (optname) { + case 't': + case 'u': + if (!inbound) { + die("Can't use -%c, no inbound interface", + optname); + } + fwd_rule_parse(optname, optarg, &inbound->fwd); + break; + case 'T': + case 'U': + if (!outbound) { + die("Can't use -%c, no outbound interface", + optname); + } + fwd_rule_parse(optname, optarg, &outbound->fwd); + break; + default: + continue; + } + } while (optname != -1); + + if (show) { + printf("Updated configuration (%s)\n", a.sun_path); + show_conf(&conf); + } + +noupdate: if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) die_perror("Error shutting down control socket"); - + exit(0); } -- 2.43.0

From: David Gibson We can now receive updates to the forwarding rules from the pesto client and store them in a "pending" copy of the forwarding tables. Implement switching to using the new rules. The logic is in a new fwd_listen_switch(). For now this closes all listening sockets related to the old tables, swaps the active and pending tables, then listens based on the new tables. In future we look to improve this so that we don't temporarily stop listening on ports that both the old and new tables specify. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- conf.c | 5 ++--- fwd.c | 34 ++++++++++++++++++++++++++++++++++ fwd.h | 1 + 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/conf.c b/conf.c index f035fd3..75b8291 100644 --- a/conf.c +++ b/conf.c @@ -2159,15 +2159,14 @@ void conf_handler(struct ctx *c, uint32_t events) fwd_rules_dump(info, fwd->rules, fwd->count, " ", ""); } + + fwd_listen_switch(c); } if (events & EPOLLHUP) { debug("Configuration client hangup"); - goto close; } - return; - close: conf_close(c); diff --git a/fwd.c b/fwd.c index d93d2e5..35b9e2b 100644 --- a/fwd.c +++ b/fwd.c @@ -534,6 +534,40 @@ int fwd_listen_init(const struct ctx *c) return 0; } +/** + * fwd_listen_switch() - Switch from current to pending rules table + * @c: Execution context + */ +void fwd_listen_switch(struct ctx *c) +{ + struct fwd_table *tmp[PIF_NUM_TYPES]; + unsigned i; + + /* Stop listening on the old tables */ + for (i = 0; i < PIF_NUM_TYPES; i++) { + struct fwd_table *fwd = c->fwd[i]; + + if (!fwd) + continue; + + debug("Flushing %u old %s rules", fwd->count, pif_name(i)); + fwd_listen_close(fwd); + fwd->count = fwd->sock_count = 0; + } + + /* Swap active and pending tables */ + static_assert(sizeof(tmp) == sizeof(c->fwd) && + sizeof(tmp) == sizeof(c->fwd_pending), + "Temporary has wrong size"); + memcpy(&tmp, (void *)c->fwd, sizeof(tmp)); + memcpy((void *)c->fwd, (void *)c->fwd_pending, sizeof(tmp)); + memcpy((void *)c->fwd_pending, &tmp, sizeof(tmp)); + + /* Start listening on the new tables */ + if (fwd_listen_init(c) < 0) + err("Error switching to new forwarding rules"); +} + /* See enum in kernel's include/net/tcp_states.h */ #define UDP_LISTEN 0x07 #define TCP_LISTEN 0x0a diff --git a/fwd.h b/fwd.h index ac24782..b60697d 100644 --- a/fwd.h +++ b/fwd.h @@ -61,6 +61,7 @@ int fwd_listen_sync(const struct ctx *c, uint8_t pif, const struct fwd_scan *tcp, const struct fwd_scan *udp); void fwd_listen_close(const struct fwd_table *fwd); int fwd_listen_init(const struct ctx *c); +void fwd_listen_switch(struct ctx *c); bool nat_inbound(const struct ctx *c, const union inany_addr *addr, union inany_addr *translated); -- 2.43.0

On 5/3/26 23:55, Stefano Brivio wrote:

From: David Gibson

Although fwd_rule_add() performs some sanity checks on the rule it is given, there are invalid rules we don't check for, assuming that its callers will do that.

That won't be enough when we can get rules inserted by a dynamic update client without going through the existing parsing code. So, add stricter checks to fwd_rule_add(), which is now possible thanks to the capabilities bits in the struct fwd_table. Where those duplicate existing checks in the callers, remove the old copies.

Signed-off-by: David Gibson Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier

--- conf.c | 21 --------------------- fwd.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/conf.c b/conf.c index 6e884e5..b470b0d 100644 --- a/conf.c +++ b/conf.c @@ -176,8 +176,6 @@ static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto, die("Invalid interface name: %s", ifname); }

- assert(first != 0); - for (base = first; base <= last; base++) { if (exclude && bitmap_isset(exclude, base)) continue; @@ -310,10 +308,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, if (p != ep) /* Garbage after the ranges */ goto bad;

- if (orig_range.first == 0) { - die("Can't forward port 0 included in '%s'", spec); - } - conf_ports_range_except(fwd, proto, addr, ifname, orig_range.first, orig_range.last, exclude, @@ -356,11 +350,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) return; }

- if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) - die("TCP port forwarding requested but TCP is disabled"); - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) - die("UDP port forwarding requested but UDP is disabled"); - strncpy(buf, optarg, sizeof(buf) - 1);

if ((spec = strchr(buf, '/'))) { @@ -405,16 +394,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) addr = NULL; }

- if (addr) { - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { - die("IPv4 is disabled, can't use -%c %s", - optname, optarg); - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { - die("IPv6 is disabled, can't use -%c %s", - optname, optarg); - } - } - if (optname == 'T' || optname == 'U') { assert(!addr && !ifname);

diff --git a/fwd.c b/fwd.c index c7fd1a9..979c149 100644 --- a/fwd.c +++ b/fwd.c @@ -367,17 +367,59 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) new->first, new->last); return -EINVAL; } + if (!new->first) { + warn("Forwarding rule attempts to map from port 0"); + return -EINVAL; + } + if (!new->to || + (in_port_t)(new->to + new->last - new->first) < new->to) { + warn("Forwarding rule attempts to map to port 0"); + return -EINVAL; + } if (new->flags & ~allowed_flags) { warn("Rule has invalid flags 0x%hhx", new->flags & ~allowed_flags); return -EINVAL; } - if (new->flags & FWD_DUAL_STACK_ANY && - !inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN];

- warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); return -EINVAL; }

On 5/3/26 23:55, Stefano Brivio wrote:

From: David Gibson

In pesto we're going to want several levels of error/warning messages, much like passt itself. Particularly as we start to share mode code between passt and pesto, we want to use a similar interface to emit those. However we don't want to use the same implementation - logging to a file or syslog doesn't make sense for the command line tool.

To accomplish this loosely share log.h, but not log.c between pesto and passt. In fact, an #ifdef means even most of log.h isn't actually shared, but we do provide similar warn(), die() etc. macros.

This includes the *_perror() variants, which need strerror(). However, we want to avoid allocations for pesto as we do for passt, and strerror() allocates in some libc versions. Therefore, also move our workaround for this to be shared with pesto.

Signed-off-by: Stefano Brivio [dwg: Based on changes part of a larger patch by Stefano] Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier One little nit below

--- Makefile | 6 +++++- common.h | 32 ++++++++++++++++++++++++++++++ log.h | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- pesto.c | 14 ++++---------- util.h | 32 ------------------------------ 5 files changed, 99 insertions(+), 44 deletions(-)

diff --git a/Makefile b/Makefile index 030681b..f6cec8a 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ vhost_user.h virtio.h vu_common.h QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h -PESTO_HEADERS = common.h pesto.h +PESTO_HEADERS = common.h pesto.h log.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -121,6 +121,7 @@ qrap: $(QRAP_SRCS) $(QRAP_HEADERS)

passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ @@ -221,9 +222,12 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck %.cppcheck: $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

+passt.cppcheck: BASE_CPPFLAGS += -UPESTO passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unmatchedSuppression pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

diff --git a/common.h b/common.h index a9c115a..2f2e6f1 100644 --- a/common.h +++ b/common.h @@ -21,4 +21,36 @@ /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)

+/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * strerror_() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *strerror_(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" + #endif /* _COMMON_H */ diff --git a/log.h b/log.h index dbab006..1058ca5 100644 --- a/log.h +++ b/log.h @@ -6,8 +6,63 @@ #ifndef LOG_H #define LOG_H

-#include #include +#include +#include + +#ifdef PESTO + +#include + +#include "common.h" + +extern bool debug_flag; + +#define msg(...) \ + do { \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, "\n"); \ + } while (0) + +#define msg_perror(...) \ + do { \ + int errno_ = errno; \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, ": %s\n", strerror_(errno_)); \ + } while (0) + +#define die(...) \ + do { \ + msg(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define die_perror(...) \ + do { \ + msg_perror(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define warn(...) msg(__VA_ARGS__) +#define warn_perror(...) msg_perror(__VA_ARGS__) +#define info(...) msg(__VA_ARGS__) +#define info_perror(...) msg_perror(__VA_ARGS__) + +#define debug(...) \ + do { \ + if (debug_flag) \ + msg(__VA_ARGS__); \ + } while (0) + +#define debug_perror_(...) \

Why is this "debug_perror_()" and not "debug_perror()"?

+ do { \ + if (debug_flag) \ + msg_perror(__VA_ARGS__); \ + } while (0) + +#else /* !PESTO */ + +#include #include #include

@@ -109,4 +164,6 @@ void __openlog(const char *ident, int option, int facility); void logfile_init(const char *name, const char *path, size_t size); void __setlogmask(int mask);

+#endif /* !PESTO */ + #endif /* LOG_H */ diff --git a/pesto.c b/pesto.c index 9f2fa5d..f0916e8 100644 --- a/pesto.c +++ b/pesto.c @@ -34,18 +34,12 @@ #include "common.h" #include "seccomp_pesto.h" #include "pesto.h" +#include "log.h"

-static bool debug_flag = false; +bool debug_flag = false;

static char stdout_buf[BUFSIZ];

-#define die(...) \ - do { \ - FPRINTF(stderr, __VA_ARGS__); \ - FPRINTF(stderr, "\n"); \ - exit(EXIT_FAILURE); \ - } while (0) - /** * usage() - Print usage, exit with given status code * @name: Executable name @@ -99,7 +93,7 @@ int main(int argc, char **argv) * breaking our seccomp profile. */ if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf))) - die("Failed to set stdout buffer"); + die_perror("Failed to set stdout buffer");

do { optname = getopt_long(argc, argv, optstring, options, NULL); @@ -126,7 +120,7 @@ int main(int argc, char **argv) if (argc - optind != 1) usage(argv[0], stderr, EXIT_FAILURE);

- printf("debug_flag=%d, path=\"%s\"\n", debug_flag, argv[optind]); + debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]);

die("pesto is not implemented yet"); } diff --git a/util.h b/util.h index 770ff93..e90be47 100644 --- a/util.h +++ b/util.h @@ -302,38 +302,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)

void raw_random(void *buf, size_t buflen);

-/* - * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, - * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs - * getrandom(2) and brk(2) as it allocates memory for the locale-translated - * error description, but our seccomp profiles forbid both. - * - * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get - * a static untranslated string. It's a GNU implementation, but also defined by - * bionic. - * - * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries - * not defining strerrordesc_np() are expected to provide strerror() - * implementations that are simple enough for us to call. - */ -__attribute__ ((weak)) const char *strerrordesc_np(int errnum); - -/** - * strerror_() - strerror() wrapper calling strerrordesc_np() if available - * @errnum: Error code - * - * Return: error description string - */ -static inline const char *strerror_(int errnum) -{ - if (strerrordesc_np) - return strerrordesc_np(errnum); - - return strerror(errnum); -} - -#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" - /* * Workarounds for https://github.com/llvm/llvm-project/issues/58992 *

On 5/3/26 23:55, Stefano Brivio wrote:

From: David Gibson

Build a new "pesto" binary, which will become the tool to update a running passt/pasta's configuration. For now, we just build a stub binary which sets up a basic environment, parses trivial command line options but does nothing else.

Signed-off-by: David Gibson Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier One little nit below

--- .gitignore | 2 + Makefile | 42 +++++++++++------ common.h | 24 ++++++++++ pesto.1 | 46 +++++++++++++++++++ pesto.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++ pesto.h | 12 +++++ util.h | 12 +---- 7 files changed, 244 insertions(+), 26 deletions(-) create mode 100644 common.h create mode 100644 pesto.1 create mode 100644 pesto.c create mode 100644 pesto.h

diff --git a/.gitignore b/.gitignore index 3c16adc..3e40d9f 100644 --- a/.gitignore +++ b/.gitignore @@ -4,9 +4,11 @@ /pasta /pasta.avx2 /passt-repair +/pesto /qrap /pasta.1 /seccomp.h +/seccomp_pesto.h /seccomp_repair.h /c*.json README.plain.md diff --git a/Makefile b/Makefile index 7875d23..030681b 100644 --- a/Makefile +++ b/Makefile @@ -47,19 +47,21 @@ PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c -SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) - -MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1 - -PASST_HEADERS = arch.h arp.h bitmap.h checksum.h conf.h dhcp.h dhcpv6.h \ - epoll_ctl.h flow.h fwd.h fwd_rule.h flow_table.h icmp.h icmp_flow.h \ - inany.h iov.h ip.h isolation.h lineread.h log.h migrate.h ndp.h \ - netlink.h packet.h passt.h pasta.h pcap.h pif.h repair.h serialise.h \ - siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.h \ - tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h vhost_user.h \ - virtio.h vu_common.h +PESTO_SRCS = pesto.c +SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS) + +MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1 + +PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ + dhcpv6.h epoll_ctl.h flow.h fwd.h fwd_rule.h flow_table.h icmp.h \ + icmp_flow.h inany.h iov.h ip.h isolation.h lineread.h log.h migrate.h \ + ndp.h netlink.h packet.h passt.h pasta.h pcap.h pesto.h pif.h repair.h \ + serialise.h siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h \ + tcp_splice.h tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h \ + vhost_user.h virtio.h vu_common.h QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h +PESTO_HEADERS = common.h pesto.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -78,7 +80,7 @@ docdir ?= $(datarootdir)/doc/passt mandir ?= $(datarootdir)/man man1dir ?= $(mandir)/man1

-BASEBIN = passt qrap passt-repair +BASEBIN = passt qrap passt-repair pesto ifeq ($(TARGET_ARCH),x86_64) BASEBIN += passt.avx2 endif @@ -100,6 +102,9 @@ seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)

+seccomp_pesto.h: seccomp.sh $(PESTO_SRCS) + @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_pesto.h $(PESTO_SRCS) + $(BASEBIN): %: $(CC) $(BASE_CPPFLAGS) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) -o $@

@@ -116,6 +121,8 @@ qrap: $(QRAP_SRCS) $(QRAP_HEADERS)

passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h + valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ @@ -126,7 +133,7 @@ valgrind: all

.PHONY: clean clean: - $(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h pasta.1 \ + $(RM) $(BIN) *~ *.o seccomp.h seccomp_repair.h seccomp_pesto.h pasta.1 \ passt.tar passt.tar.gz *.deb *.rpm \ passt.pid README.plain.md

@@ -183,7 +190,8 @@ docs: README.md CLANG_TIDY = clang-tidy CLANG_TIDY_FLAGS = -DCLANG_TIDY_58992

-clang-tidy: passt.clang-tidy passt-repair.clang-tidy qrap.clang-tidy +clang-tidy: passt.clang-tidy passt-repair.clang-tidy pesto.clang-tidy \ + qrap.clang-tidy

.PHONY: %.clang-tidy %.clang-tidy: @@ -191,6 +199,7 @@ clang-tidy: passt.clang-tidy passt-repair.clang-tidy qrap.clang-tidy

passt.clang-tidy: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h passt-repair.clang-tidy: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h +pesto.clang-tidy: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h qrap.clang-tidy: $(QRAP_SRCS) $(QRAP_HEADERS)

CPPCHECK = cppcheck @@ -206,7 +215,7 @@ CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \ --suppress=unusedStructMember \ -D CPPCHECK_6936

-cppcheck: passt.cppcheck passt-repair.cppcheck qrap.cppcheck +cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck

.PHONY: %.cppcheck %.cppcheck: @@ -215,6 +224,9 @@ cppcheck: passt.cppcheck passt-repair.cppcheck qrap.cppcheck passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unmatchedSuppression +pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h + qrap.cppcheck: BASE_CPPFLAGS += -DARCH=\"$(TARGET_ARCH)\" qrap.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction qrap.cppcheck: $(QRAP_SRCS) $(QRAP_HEADERS) diff --git a/common.h b/common.h new file mode 100644 index 0000000..a9c115a --- /dev/null +++ b/common.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: David Gibson + * + * Definitions used by both passt/pasta and other tools + */ + +#ifndef COMMON_H +#define COMMON_H + +#include + +#define VERSION_BLOB \ + VERSION "\n" \ + "Copyright Red Hat\n" \ + "GNU General Public License, version 2 or later\n" \ + " https://www.gnu.org/licenses/old-licenses/gpl-2.0.html\n" \ + "This is free software: you are free to change and redistribute it.\n" \ + "There is NO WARRANTY, to the extent permitted by law.\n\n" + +/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ +#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) + +#endif /* _COMMON_H */

Why "_COMMON_H" and not "COMMON_H"?

diff --git a/pesto.1 b/pesto.1 new file mode 100644 index 0000000..338fb8a --- /dev/null +++ b/pesto.1 @@ -0,0 +1,46 @@ +.\" SPDX-License-Identifier: GPL-2.0-or-later +.\" Copyright Red Hat +.\" Author: David Gibson +.TH pesto 1 + +.SH NAME +.B pesto +\- Configure a running \fBpasst\fR(1) or \fBpasta\fR(1) instance. + +.SH SYNOPSIS +.B pesto +\fIPATH\fR + +.SH DESCRIPTION + +.B pesto +is an experimental client to view and update the port forwarding +configuration of a running \fBpasst\fR(1) or \fBpasta\fR(1) instance. + +\fIPATH\fR gives the path to the UNIX domain socket created by +\fBpasst\fR or \fBpasta\fR. It should match the \fB-c\fR command line +option given to that instance. + +.SH AUTHORS + +Stefano Brivio , +David Gibson . + +.SH REPORTING BUGS + +Please report issues on the bug tracker at https://bugs.passt.top/, or +send a message to the passt-user@passt.top mailing list, see +https://lists.passt.top/. + +.SH COPYRIGHT + +Copyright Red Hat + +\fBpesto\fR is free software: you can redistribute them and/or modify +them under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 2 of the License, or (at +your option) any later version. + +.SH SEE ALSO + +\fBpasst\fR(1), \fBpasta\fR(1), \fBunix\fR(7). diff --git a/pesto.c b/pesto.c new file mode 100644 index 0000000..9f2fa5d --- /dev/null +++ b/pesto.c @@ -0,0 +1,132 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* PESTO - Programmable Extensible Socket Translation Orchestrator + * front-end for passt(1) and pasta(1) forwarding configuration + * + * pesto.c - Main program (it's not actually extensible) + * + * Copyright (c) 2026 Red Hat GmbH + * Author: Stefano Brivio + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "common.h" +#include "seccomp_pesto.h" +#include "pesto.h" + +static bool debug_flag = false; + +static char stdout_buf[BUFSIZ]; + +#define die(...) \ + do { \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, "\n"); \ + exit(EXIT_FAILURE); \ + } while (0) + +/** + * usage() - Print usage, exit with given status code + * @name: Executable name + * @f: Stream to print usage info to + * @status: Status code for exit(2) + * + * #syscalls:pesto exit_group fstat write + */ +static void usage(const char *name, FILE *f, int status) +{ + FPRINTF(f, "Usage: %s [OPTION]... PATH\n", name); + FPRINTF(f, + "\n" + " -d, --debug Print debugging messages\n" + " -h, --help Display this help message and exit\n" + " --version Show version and exit\n"); + exit(status); +} + +/** + * main() - Dynamic reconfiguration client main program + * @argc: Argument count + * @argv: Arguments: socket path, operation, port specifiers + * + * Return: 0 on success, won't return on failure + * + * #syscalls:pesto exit_group fstat read write + */ +int main(int argc, char **argv) +{ + const struct option options[] = { + {"debug", no_argument, NULL, 'd' }, + {"help", no_argument, NULL, 'h' }, + {"version", no_argument, NULL, 1 }, + { 0 }, + }; + const char *optstring = "dh"; + struct sock_fprog prog; + int optname; + + prctl(PR_SET_DUMPABLE, 0); + + prog.len = (unsigned short)sizeof(filter_pesto) / + sizeof(filter_pesto[0]); + prog.filter = filter_pesto; + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) || + prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) + die("Failed to apply seccomp filter"); + + /* Explicitly set stdout buffer, otherwise printf() might allocate, + * breaking our seccomp profile. + */ + if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf))) + die("Failed to set stdout buffer"); + + do { + optname = getopt_long(argc, argv, optstring, options, NULL); + + switch (optname) { + case -1: + case 0: + break; + case 'h': + usage(argv[0], stdout, EXIT_SUCCESS); + break; + case 'd': + debug_flag = true; + break; + case 1: + FPRINTF(stdout, "pesto "); + FPRINTF(stdout, VERSION_BLOB); + exit(EXIT_SUCCESS); + default: + usage(argv[0], stderr, EXIT_FAILURE); + } + } while (optname != -1); + + if (argc - optind != 1) + usage(argv[0], stderr, EXIT_FAILURE); + + printf("debug_flag=%d, path=\"%s\"\n", debug_flag, argv[optind]); + + die("pesto is not implemented yet"); +} diff --git a/pesto.h b/pesto.h new file mode 100644 index 0000000..e9b329f --- /dev/null +++ b/pesto.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: David Gibson + * + * Definitions and functions used by both client and server of the configuration + * update protocol (pesto). + */ + +#ifndef PESTO_H +#define PESTO_H + +#endif /* PESTO_H */ diff --git a/util.h b/util.h index 92aeabc..770ff93 100644 --- a/util.h +++ b/util.h @@ -19,16 +19,9 @@ #include #include

+#include "common.h" #include "log.h"

-#define VERSION_BLOB \ - VERSION "\n" \ - "Copyright Red Hat\n" \ - "GNU General Public License, version 2 or later\n" \ - " https://www.gnu.org/licenses/old-licenses/gpl-2.0.html\n" \ - "This is free software: you are free to change and redistribute it.\n" \ - "There is NO WARRANTY, to the extent permitted by law.\n\n" - #ifndef SECCOMP_RET_KILL_PROCESS #define SECCOMP_RET_KILL_PROCESS SECCOMP_RET_KILL #endif @@ -307,9 +300,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m) return mod_sub(x, i, m) < mod_sub(j, i, m); }

-/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ -#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) - void raw_random(void *buf, size_t buflen);

/*

On 5/4/26 14:01, Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

Start implementing pesto in earnest.  Create a control/configuration socket in passt.  Have pesto connect to it and retrieve a server greeting Perform some basic version checking.

Signed-off-by: David Gibson [sbrivio: Avoid potential recursive calling between conf_accept() and   conf_close(), reported by clang-tidy] [sbrivio: In conf(), check we're not exceeding sizeof(c->control_path)   instead of sizeof(c->socket_path), and in pesto's main, print   argv[optind] instead of argv[1] to indicate an invalid socket path,   both reported by Jon Maloy] Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier

but...

...

---   Makefile     |   8 ++-   conf.c       | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++-   conf.h       |   2 +   epoll_type.h |   4 ++   passt.1      |   5 ++   passt.c      |   8 +++   passt.h      |   6 ++   pesto.c      |  47 ++++++++++++-   pesto.h      |  22 +++++++   serialise.c  |   3 +   10 files changed, 282 insertions(+), 6 deletions(-)

...

...

diff --git a/pesto.c b/pesto.c index f0916e8..762cfe9 100644 --- a/pesto.c +++ b/pesto.c @@ -33,6 +33,7 @@   #include "common.h"   #include "seccomp_pesto.h" +#include "serialise.h"   #include "pesto.h"   #include "log.h" @@ -66,6 +67,8 @@ static void usage(const char *name, FILE *f, int status)    *    * Return: 0 on success, won't return on failure    * + * #syscalls:pesto socket s390x:socketcall i686:socketcall + * #syscalls:pesto connect shutdown close    * #syscalls:pesto exit_group fstat read write    */   int main(int argc, char **argv) @@ -76,9 +79,12 @@ int main(int argc, char **argv)           {"version",    no_argument,        NULL,        1 },           { 0 },       }; +    struct sockaddr_un a = { AF_UNIX, "" };       const char *optstring = "dh"; +    struct pesto_hello hello;       struct sock_fprog prog; -    int optname; +    int optname, ret, s; +    uint32_t s_version;       prctl(PR_SET_DUMPABLE, 0); @@ -122,5 +128,42 @@ int main(int argc, char **argv)       debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]); -    die("pesto is not implemented yet"); +    if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) +        die_perror("Failed to create AF_UNIX socket"); + +    ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[optind]); +    if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) +        die("Invalid socket path \"%s\"", argv[optind]); + +    ret = connect(s, (struct sockaddr *)&a, sizeof(a)); +    if (ret < 0) { +        die_perror("Failed to connect to %s", a.sun_path); +    } + +    ret = read_all_buf(s, &hello, sizeof(hello)); +    if (ret < 0) +        die_perror("Couldn't read server greeting"); + +    if (memcmp(hello.magic, PESTO_SERVER_MAGIC, sizeof(hello.magic))) +        die("Bad magic number from server"); + +    s_version = ntohl(hello.version); + +    if (s_version > PESTO_PROTOCOL_VERSION) { +        die("Unknown server protocol version %"PRIu32" > %"PRIu32"\n",

The trailing '\n' is not needed.

...

+            s_version, PESTO_PROTOCOL_VERSION); +    } + ...

On 5/3/26 23:55, Stefano Brivio wrote:

From: David Gibson

Most things in ip.[ch] related purely to IP addresses and headers with no dependency on other passt/pasta internals. A number of these will be useful to re-use in pesto. The exception is ipv6_l4hdr() which uses iov_tail.

The only caller of this is in tap.c, so move the function there. Along with moving the constant byteswapping functions to common.h, that lets ip.[ch] to be linked into pesto as well as passt/pasta.

Signed-off-by: David Gibson Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier But we can fix a pre-existing error while doing the move, see below:

--- common.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ inany.h | 2 ++ ip.c | 56 +++----------------------------------------------------- ip.h | 4 +--- tap.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ util.h | 48 ------------------------------------------------ 6 files changed, 106 insertions(+), 104 deletions(-)

diff --git a/common.h b/common.h index 45f66ea..4a167ae 100644 --- a/common.h +++ b/common.h @@ -55,4 +55,52 @@ static inline const char *strerror_(int errnum)

#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0])))

+#ifndef __bswap_constant_16 +#define __bswap_constant_16(x) \ + ((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8))) +#endif + +#ifndef __bswap_constant_32 +#define __bswap_constant_32(x) \ + ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ + (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) +#endif + +#ifndef __bswap_constant_32 +#define __bswap_constant_32(x) \ + ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ + (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) +#endif

__bswap_constant_32 block is written twice.

+ +#ifndef __bswap_constant_64 +#define __bswap_constant_64(x) \ + ((((x) & 0xff00000000000000ULL) >> 56) | \ + (((x) & 0x00ff000000000000ULL) >> 40) | \ + (((x) & 0x0000ff0000000000ULL) >> 24) | \ + (((x) & 0x000000ff00000000ULL) >> 8) | \ + (((x) & 0x00000000ff000000ULL) << 8) | \ + (((x) & 0x0000000000ff0000ULL) << 24) | \ + (((x) & 0x000000000000ff00ULL) << 40) | \ + (((x) & 0x00000000000000ffULL) << 56)) +#endif + +#if __BYTE_ORDER == __BIG_ENDIAN +#define htons_constant(x) (x) +#define htonl_constant(x) (x) +#define htonll_constant(x) (x) +#define ntohs_constant(x) (x) +#define ntohl_constant(x) (x) +#define ntohll_constant(x) (x) +#else +#define htons_constant(x) (__bswap_constant_16(x)) +#define htonl_constant(x) (__bswap_constant_32(x)) +#define htonll_constant(x) (__bswap_constant_64(x)) +#define ntohs_constant(x) (__bswap_constant_16(x)) +#define ntohl_constant(x) (__bswap_constant_32(x)) +#define ntohll_constant(x) (__bswap_constant_64(x)) +#endif + +#define ntohll(x) (be64toh((x))) +#define htonll(x) (htobe64((x))) + #endif /* _COMMON_H */ diff --git a/inany.h b/inany.h index 30e2416..1f7741d 100644 --- a/inany.h +++ b/inany.h @@ -10,8 +10,10 @@ #define INANY_H

#include +#include #include

+#include "util.h" #include "ip.h" #include "siphash.h"

diff --git a/ip.c b/ip.c index 25fa407..f2506bb 100644 --- a/ip.c +++ b/ip.c @@ -6,6 +6,9 @@ * PASTA - Pack A Subtle Tap Abstraction * for network namespace/tap device mode * + * PESTO - Programmable Extensible Socket Translation Orchestrator + * front-end for passt(1) and pasta(1) forwarding configuration + * * ip.c - IP related functions * * Copyright (c) 2020-2021 Red Hat GmbH @@ -16,61 +19,8 @@ #include #include

-#include "util.h" #include "ip.h"

-#define IPV6_NH_OPT(nh) \ - ((nh) == 0 || (nh) == 43 || (nh) == 44 || (nh) == 50 || \ - (nh) == 51 || (nh) == 60 || (nh) == 135 || (nh) == 139 || \ - (nh) == 140 || (nh) == 253 || (nh) == 254) - -/** - * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol - * @data: IPv6 packet - * @proto: Filled with L4 protocol number - * @dlen: Data length (payload excluding header extensions), set on return - * - * Return: true if the L4 header is found and @data, @proto, @dlen are set, - * false on error. Outputs are indeterminate on failure. - */ -bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) -{ - struct ipv6_opt_hdr o_storage; - const struct ipv6_opt_hdr *o; - struct ipv6hdr ip6h_storage; - const struct ipv6hdr *ip6h; - int hdrlen; - uint8_t nh; - - ip6h = IOV_REMOVE_HEADER(data, ip6h_storage); - if (!ip6h) - return false; - - nh = ip6h->nexthdr; - if (!IPV6_NH_OPT(nh)) - goto found; - - while ((o = IOV_PEEK_HEADER(data, o_storage))) { - nh = o->nexthdr; - hdrlen = (o->hdrlen + 1) * 8; - - if (IPV6_NH_OPT(nh)) - iov_drop_header(data, hdrlen); - else - goto found; - } - - return false; - -found: - if (nh == IPPROTO_NONE) - return false; - - *dlen = iov_tail_size(data); - *proto = nh; - return true; -} - /** * ipproto_name() - Get IP protocol name from number * @proto: IP protocol number diff --git a/ip.h b/ip.h index fb4119a..aab9b86 100644 --- a/ip.h +++ b/ip.h @@ -9,7 +9,7 @@ #include #include

-#include "util.h" +#include "common.h"

#define IN4_IS_ADDR_UNSPECIFIED(a) \ (((struct in_addr *)(a))->s_addr == htonl_constant(INADDR_ANY)) @@ -117,8 +117,6 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h) ip6h->flow_lbl[2]; }

-bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen); - #define IPPROTO_STRLEN (sizeof("<unknown protocol>")) const char *ipproto_name(uint8_t proto);

diff --git a/tap.c b/tap.c index 7d06189..cfd7b64 100644 --- a/tap.c +++ b/tap.c @@ -874,6 +874,58 @@ append: return in->count; }

+#define IPV6_NH_OPT(nh) \ + ((nh) == 0 || (nh) == 43 || (nh) == 44 || (nh) == 50 || \ + (nh) == 51 || (nh) == 60 || (nh) == 135 || (nh) == 139 || \ + (nh) == 140 || (nh) == 253 || (nh) == 254) + +/** + * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol + * @data: IPv6 packet + * @proto: Filled with L4 protocol number + * @dlen: Data length (payload excluding header extensions), set on return + * + * Return: true if the L4 header is found and @data, @proto, @dlen are set, + * false on error. Outputs are indeterminate on failure. + */ +static bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) +{ + struct ipv6_opt_hdr o_storage; + const struct ipv6_opt_hdr *o; + struct ipv6hdr ip6h_storage; + const struct ipv6hdr *ip6h; + int hdrlen; + uint8_t nh; + + ip6h = IOV_REMOVE_HEADER(data, ip6h_storage); + if (!ip6h) + return false; + + nh = ip6h->nexthdr; + if (!IPV6_NH_OPT(nh)) + goto found; + + while ((o = IOV_PEEK_HEADER(data, o_storage))) { + nh = o->nexthdr; + hdrlen = (o->hdrlen + 1) * 8; + + if (IPV6_NH_OPT(nh)) + iov_drop_header(data, hdrlen); + else + goto found; + } + + return false; + +found: + if (nh == IPPROTO_NONE) + return false; + + *dlen = iov_tail_size(data); + *proto = nh; + return true; +} + /** * tap6_handler() - IPv6 packet handler for tap file descriptor * @c: Execution context diff --git a/util.h b/util.h index c788382..dc14c78 100644 --- a/util.h +++ b/util.h @@ -101,54 +101,6 @@ void abort_with_msg(const char *fmt, ...) #define MAC_UNDEF MAC_BROADCAST #define MAC_IS_UNDEF(addr) (!memcmp((addr), MAC_UNDEF, ETH_ALEN))

-#ifndef __bswap_constant_16 -#define __bswap_constant_16(x) \ - ((uint16_t) ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8))) -#endif - -#ifndef __bswap_constant_32 -#define __bswap_constant_32(x) \ - ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ - (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) -#endif - -#ifndef __bswap_constant_32 -#define __bswap_constant_32(x) \ - ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \ - (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24)) -#endif - -#ifndef __bswap_constant_64 -#define __bswap_constant_64(x) \ - ((((x) & 0xff00000000000000ULL) >> 56) | \ - (((x) & 0x00ff000000000000ULL) >> 40) | \ - (((x) & 0x0000ff0000000000ULL) >> 24) | \ - (((x) & 0x000000ff00000000ULL) >> 8) | \ - (((x) & 0x00000000ff000000ULL) << 8) | \ - (((x) & 0x0000000000ff0000ULL) << 24) | \ - (((x) & 0x000000000000ff00ULL) << 40) | \ - (((x) & 0x00000000000000ffULL) << 56)) -#endif - -#if __BYTE_ORDER == __BIG_ENDIAN -#define htons_constant(x) (x) -#define htonl_constant(x) (x) -#define htonll_constant(x) (x) -#define ntohs_constant(x) (x) -#define ntohl_constant(x) (x) -#define ntohll_constant(x) (x) -#else -#define htons_constant(x) (__bswap_constant_16(x)) -#define htonl_constant(x) (__bswap_constant_32(x)) -#define htonll_constant(x) (__bswap_constant_64(x)) -#define ntohs_constant(x) (__bswap_constant_16(x)) -#define ntohl_constant(x) (__bswap_constant_32(x)) -#define ntohll_constant(x) (__bswap_constant_64(x)) -#endif - -#define ntohll(x) (be64toh((x))) -#define htonll(x) (htobe64((x))) - extern uint8_t eth_pad[ETH_ZLEN];

/**

On 5/3/26 23:55, Stefano Brivio wrote:

From: David Gibson

Implement serialisation of our current forwarding rules in conf.c, deserialising it to display in the pesto client. Doing this requires adding ip.c, inany.c, bitmap.c, lineread.c and fwd_rule.c to the pesto build. With previous preparations that now requires only a trivial change to lineread.c.

Signed-off-by: David Gibson [sbrivio: Use ntohs() for rule->to instead of htons() in fwd_rule_read(), reported by Jon Maloy] Signed-off-by: Stefano Brivio

With the "pc->fwd.count <= MAX_FWD_RULES" check added below, add: Reviewed-by: Laurent Vivier More cosmetics nit below

--- Makefile | 17 +++++++++++++---- conf.c | 12 +++++++++++- fwd_rule.c | 41 +++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 4 ++++ lineread.c | 2 +- pesto.c | 37 ++++++++++++++++++++++++++++++++++--- pesto.h | 6 ++++++ 7 files changed, 110 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile index 6da76b4..057e4eb 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c -PESTO_SRCS = pesto.c serialise.c +PESTO_SRCS = pesto.c bitmap.c fwd_rule.c inany.c ip.c lineread.c serialise.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)

MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1 @@ -62,6 +62,8 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h PESTO_HEADERS = common.h pesto.h log.h serialise.h

Duplicate PESTO_HEADERS ^ v

+PESTO_HEADERS = common.h pesto.h bitmap.h fwd_rule.h inany.h ip.h lineread.h \ + log.h serialise.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -223,15 +225,22 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

passt.cppcheck: BASE_CPPFLAGS += -UPESTO -passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c +passt.cppcheck: CPPCHECK_FLAGS += \ + --suppress=unusedFunction:fwd_rule.c \ + --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h

passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += \ - --suppress=unusedFunction:serialise.c \ - --suppress=staticFunction:serialise.c + --suppress=unusedFunction:bitmap.c \ + --suppress=unusedFunction:inany.h \ + --suppress=unusedFunction:inany.c \ + --suppress=unusedFunction:ip.h \ + --suppress=unusedFunction:fwd_rule.c \ + --suppress=staticFunction:fwd_rule.c \ + --suppress=unusedFunction:serialise.c pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

qrap.cppcheck: BASE_CPPFLAGS += -DARCH=\"$(TARGET_ARCH)\" diff --git a/conf.c b/conf.c index 3b2fe42..5e4e81e 100644 --- a/conf.c +++ b/conf.c @@ -1939,21 +1939,30 @@ static int conf_send_rules(const struct ctx *c, int fd) unsigned pif;

for (pif = 0; pif < PIF_NUM_TYPES; pif++) { + struct fwd_table *fwd = c->fwd[pif]; struct pesto_pif_info info; + unsigned i; int rc;

- if (!c->fwd[pif]) + if (!fwd) continue;

assert(pif != PIF_NONE);

rc = snprintf(info.name, sizeof(info.name), "%s", pif_name(pif)); assert(rc >= 0 && (size_t)rc < sizeof(info.name)); + info.caps = htonl(fwd->caps); + info.count = htonl(fwd->count);

if (write_u8(fd, pif) < 0) return -1; if (write_all_buf(fd, &info, sizeof(info)) < 0) return -1; + + for (i = 0; i < fwd->count; i++) { + if (fwd_rule_write(fd, &fwd->rules[i])) + return -1; + } }

if (write_u8(fd, PIF_NONE) < 0) @@ -2006,6 +2015,7 @@ static void conf_accept(struct ctx *c) .magic = PESTO_SERVER_MAGIC, .version = htonl(PESTO_PROTOCOL_VERSION), .pif_name_size = htonl(PIF_NAME_SIZE), + .ifnamsiz = htonl(IFNAMSIZ), }; union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; struct ucred uc = { 0 }; diff --git a/fwd_rule.c b/fwd_rule.c index 7fd20dd..da9d893 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -24,6 +24,7 @@ #include "fwd_rule.h" #include "lineread.h" #include "log.h" +#include "serialise.h"

/* Ephemeral port range: values from RFC 6335 */ static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); @@ -645,3 +646,43 @@ void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd)

fwd_rule_parse_ports(fwd, proto, addr, ifname, spec); } + + +/** + * fwd_rule_read() - Read serialised rule from an fd + * @fd: fd to serialise to

should be "fd to deserialise from" (or something like that)

+ * @rule: Buffer to store rule into + * + * Return: 0 on success, -1 on error (with errno set) + */ +int fwd_rule_read(int fd, struct fwd_rule *rule) +{ + if (read_all_buf(fd, rule, sizeof(*rule))) + return -1; + + /* Byteswap for host */ + rule->first = ntohs(rule->first); + rule->last = ntohs(rule->last); + rule->to = ntohs(rule->to); + + return 0; +} + +/** + * fwd_rule_write() - Serialise rule to an fd + * @fd: fd to serialise to + * @rule: Rule to send + * + * Return: 0 on success, -1 on error (with errno set) + */ +int fwd_rule_write(int fd, const struct fwd_rule *rule) +{ + struct fwd_rule tmp = *rule; + + /* Byteswap for transport */ + tmp.first = htons(tmp.first); + tmp.last = htons(tmp.last); + tmp.to = htons(tmp.to); + + return write_all_buf(fd, &tmp, sizeof(tmp)); +} diff --git a/fwd_rule.h b/fwd_rule.h index f51f1b4..330d49e 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -29,6 +29,8 @@ #define FWD_CAP_UDP BIT(3) #define FWD_CAP_SCAN BIT(4) #define FWD_CAP_IFNAME BIT(5) +#define FWD_CAP_ALL (FWD_CAP_IPV4 | FWD_CAP_IPV6 | FWD_CAP_TCP | \ + FWD_CAP_UDP | FWD_CAP_SCAN | FWD_CAP_IFNAME)

/** * struct fwd_rule - Forwarding rule governing a range of ports @@ -99,6 +101,8 @@ void fwd_probe_ephemeral(void); const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size); void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd); +int fwd_rule_read(int fd, struct fwd_rule *rule); +int fwd_rule_write(int fd, const struct fwd_rule *rule);

/** * fwd_rules_dump() - Dump forwarding rules diff --git a/lineread.c b/lineread.c index b9ceae1..a4269a6 100644 --- a/lineread.c +++ b/lineread.c @@ -19,8 +19,8 @@ #include #include

+#include "common.h" #include "lineread.h" -#include "util.h"

/** * lineread_init() - Prepare for line by line file reading without allocation diff --git a/pesto.c b/pesto.c index 77244b3..4bf9bd8 100644 --- a/pesto.c +++ b/pesto.c @@ -34,6 +34,7 @@ #include "common.h" #include "seccomp_pesto.h" #include "serialise.h" +#include "fwd_rule.h" #include "pesto.h" #include "log.h"

@@ -66,6 +67,7 @@ static void usage(const char *name, FILE *f, int status) struct pif_configuration { uint8_t pif; char name[PIF_NAME_SIZE]; + struct fwd_table fwd; };

struct configuration { @@ -123,6 +125,7 @@ static bool read_pif_conf(int fd, struct configuration *conf) struct pif_configuration *pc; struct pesto_pif_info info; uint8_t pif; + unsigned i;

if (read_u8(fd, &pif) < 0) die("Error reading from control socket"); @@ -151,8 +154,17 @@ static bool read_pif_conf(int fd, struct configuration *conf) static_assert(sizeof(info.name) == sizeof(pc->name), "Mismatching pif name lengths"); memcpy(pc->name, info.name, sizeof(pc->name)); - - debug("PIF %"PRIu8": %s", pc->pif, pc->name); + pc->fwd.caps = ntohl(info.caps); + pc->fwd.count = ntohl(info.count);

We should check that pc->fwd.count <= MAX_FWD_RULES to avoid overflow while scanning the array.

+ + debug("PIF %"PRIu8": %s, %"PRIu32" rules, capabilities 0x%"PRIx32 + ":%s%s%s%s%s%s", pc->pif, pc->name, pc->fwd.count, pc->fwd.caps, + pc->fwd.caps & FWD_CAP_IPV4 ? " IPv4" : "", + pc->fwd.caps & FWD_CAP_IPV6 ? " IPv6" : "", + pc->fwd.caps & FWD_CAP_TCP ? " TCP" : "", + pc->fwd.caps & FWD_CAP_UDP ? " UDP" : "", + pc->fwd.caps & FWD_CAP_SCAN ? " scan" : "", + pc->fwd.caps & FWD_CAP_IFNAME ? " ifname" : "");

/* O(n^2), but n is bounded by MAX_PIFS */ if (pif_conf_by_num(conf, pc->pif)) @@ -162,6 +174,18 @@ static bool read_pif_conf(int fd, struct configuration *conf) if (pif_conf_by_name(conf, pc->name)) die("Received duplicate interface name");

+ /* NOTE: We read the fwd rules directly into fwd.rules, rather than + * using fwd_rule_add(). This means we can read and display rules even + * if something has gone wrong (in pesto or passt) and we get rules that + * fwd_rule_add() would reject. It does have the side effect that we + * never assign socket space for the fwd rules, but we don't need that + * within pesto. + */ + for (i = 0; i < pc->fwd.count; i++) { + if (fwd_rule_read(fd, &pc->fwd.rules[i]) < 0) + die("Error reading from control socket"); + } + conf->npifs++; return true; } @@ -177,7 +201,8 @@ static void show_conf(const struct configuration *conf) for (i = 0; i < conf->npifs; i++) { const struct pif_configuration *pc = &conf->pif[i]; printf(" %s\n", pc->name); - printf(" TBD\n"); + fwd_rules_dump(printf, pc->fwd.rules, pc->fwd.count, + " ", "\n"); } }

@@ -290,6 +315,12 @@ int main(int argc, char **argv) ntohl(hello.pif_name_size), PIF_NAME_SIZE); }

+ if (ntohl(hello.ifnamsiz) != IFNAMSIZ) { + die("Server has unexpected IFNAMSIZ (%" + PRIu32" not %"PRIu32"\n",

trailing '\n'

+ ntohl(hello.ifnamsiz), IFNAMSIZ); + } +

Trailing tab :)

while (read_pif_conf(s, &conf)) ;

diff --git a/pesto.h b/pesto.h index 1879759..12b0b65 100644 --- a/pesto.h +++ b/pesto.h @@ -26,11 +26,13 @@ * @magic: PESTO_SERVER_MAGIC * @version: Version number * @pif_name_size: Server's value for PIF_NAME_SIZE + * @ifnamsiz: Server's value for IFNAMSIZ */ struct pesto_hello { char magic[8]; uint32_t version; uint32_t pif_name_size; + uint32_t ifnamsiz; } __attribute__ ((__packed__));

static_assert(sizeof(PESTO_SERVER_MAGIC) @@ -40,9 +42,13 @@ static_assert(sizeof(PESTO_SERVER_MAGIC) /** * struct pesto_pif_info - Message with basic metadata about a pif * @name: Name (\0 terminated) + * @caps: Forwarding capabilities for this pif + * @count: Number of forwarding rules for this pif */ struct pesto_pif_info { char name[PIF_NAME_SIZE]; + uint32_t caps; + uint32_t count; } __attribute__ ((__packed__));

#endif /* PESTO_H */

On Mon, 4 May 2026 11:12:16 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

All current pif names are quite short, and we expect them to remain short when/if we allow arbitrary pifs. However, because of the structure of the current code we don't enforce any limit on the length.

This will become more important with dynamic configuration updates, so start enforcing a length limit. Specifically we allow pif names to be up to 128 bytes (PIF_NAME_SIZE), including the terminating \0. This is more or less arbitrary, but seems like it should be comfortably enough for all the cases we have in mind.

Signed-off-by: David Gibson Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier

There is a little typo below

...

--- pif.c | 2 +- pif.h | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/pif.c b/pif.c index 1e80724..d5e3161 100644 --- a/pif.c +++ b/pif.c @@ -17,7 +17,7 @@ #include "inany.h" #include "epoll_ctl.h"

-const char *pif_type_str[] = { +const char pif_type_str[][PIF_NAME_SIZE] = { [PIF_NONE] = "<none>", [PIF_HOST] = "HOST", [PIF_TAP] = "TAP", diff --git a/pif.h b/pif.h index 7bb58e5..90dd3a3 100644 --- a/pif.h +++ b/pif.h @@ -35,7 +35,9 @@ enum pif_type { PIF_NUM_TYPES, };

-extern const char *pif_type_str[]; +/* Maxmimum size of a pif name, including \0 */

"Maximum"

Fixed in v7. -- Stefano

On Mon, 4 May 2026 16:34:40 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

Extend the dynamic update protocol to expose the pif indices and names from a running passt/pasta to the pesto tool. pesto records that data and, if requested with a new --show flag, prints it out.

There is no --show flag

Ah, right, that comes later in the series now. I dropped that from the commit message, and reworded the title.

...

Signed-off-by: David Gibson [sbrivio: In read_pif_conf(), force a redundant termination of the interface name, the existing check isn't obvious enough for static checkers] [sbrivio: Drop @resv_ left-over in description of struct pesto_pif_info, reported by Jon Maloy] Signed-off-by: Stefano Brivio

Reviewed-by: Laurent Vivier

But I've noticed 3 minor cosmetic flaws below.

...

--- Makefile | 1 + common.h | 2 + conf.c | 41 ++++++++++++++++ pesto.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++ pesto.h | 18 ++++++- pif.h | 4 +- serialise.c | 4 ++ serialise.h | 1 + util.h | 2 - 9 files changed, 201 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile index 1718ddb..6da76b4 100644 --- a/Makefile +++ b/Makefile @@ -223,6 +223,7 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

passt.cppcheck: BASE_CPPFLAGS += -UPESTO +passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h

passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h diff --git a/common.h b/common.h index 2f2e6f1..45f66ea 100644 --- a/common.h +++ b/common.h @@ -53,4 +53,6 @@ static inline const char *strerror_(int errnum)

#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead"

+#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) + #endif /* _COMMON_H */ diff --git a/conf.c b/conf.c index 823e08d..3b2fe42 100644 --- a/conf.c +++ b/conf.c @@ -1925,6 +1925,43 @@ void conf(struct ctx *c, int argc, char **argv)

static void conf_accept(struct ctx *c);

+/** + * conf_send_rules() - Send current forwarding rules to config client (pesto) + * @c: Execution context + * @fd: Socket to the client + * + * Return: 0 on success, -1 on failure + * + * FIXME: So far only sends pif ids and names + */ +static int conf_send_rules(const struct ctx *c, int fd) +{ + unsigned pif; + + for (pif = 0; pif < PIF_NUM_TYPES; pif++) { + struct pesto_pif_info info; + int rc; + + if (!c->fwd[pif]) + continue; + + assert(pif != PIF_NONE); + + rc = snprintf(info.name, sizeof(info.name), "%s", pif_name(pif)); + assert(rc >= 0 && (size_t)rc < sizeof(info.name)); + + if (write_u8(fd, pif) < 0) + return -1; + if (write_all_buf(fd, &info, sizeof(info)) < 0) + return -1; + } + + if (write_u8(fd, PIF_NONE) < 0) + return -1; + + return 0; +} + /** * conf_close() - Close configuration / control socket and clean up * @c: Execution context @@ -1968,6 +2005,7 @@ static void conf_accept(struct ctx *c) struct pesto_hello hello = { .magic = PESTO_SERVER_MAGIC, .version = htonl(PESTO_PROTOCOL_VERSION), + .pif_name_size = htonl(PIF_NAME_SIZE), }; union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; struct ucred uc = { 0 }; @@ -2009,6 +2047,9 @@ retry: "Warning: Using experimental unsupported configuration protocol"); }

+ if (conf_send_rules(c, fd) < 0) + goto fail; + return;

fail: diff --git a/pesto.c b/pesto.c index 762cfe9..77244b3 100644 --- a/pesto.c +++ b/pesto.c @@ -60,6 +60,127 @@ static void usage(const char *name, FILE *f, int status) exit(status); }

+/* Maximum number of pifs with rule tables */ +#define MAX_PIFS 3 + +struct pif_configuration { + uint8_t pif; + char name[PIF_NAME_SIZE]; +}; + +struct configuration { + uint32_t npifs; + struct pif_configuration pif[MAX_PIFS]; +}; + +/** + * pif_conf_by_num() - Find a pif's configuration by pif id + * @conf: Configuration description + * @pif: pif id + * + * Return: pointer to the pif_configuration for @pif, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_num(struct configuration *conf, + uint8_t pif) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (conf->pif[i].pif == pif) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pif_conf_by_name() - Find a pif's configuration by name + * @conf: Configuration description + * @name: Interface name + * + * Return: pif_configuration for pif named @name, or NULL if not found + */ +static struct pif_configuration *pif_conf_by_name(struct configuration *conf, + const char *name) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + if (strcmp(conf->pif[i].name, name) == 0) + return &conf->pif[i]; + } + + return NULL; +} + +/** + * pesto_read_rules() - Read rulestate from passt/pasta + * @fd: Control socket + * @conf: Configuration description to update + */ +static bool read_pif_conf(int fd, struct configuration *conf) +{ + struct pif_configuration *pc; + struct pesto_pif_info info; + uint8_t pif; + + if (read_u8(fd, &pif) < 0) + die("Error reading from control socket"); + + if (pif == PIF_NONE) + return false; + + debug("Receiving config for PIF %"PRIu8, pif); + + if (conf->npifs >= ARRAY_SIZE(conf->pif)) { + die("passt has more pifs than pesto can manage (max %d)", + ARRAY_SIZE(conf->pif)); + } + + pc = &conf->pif[conf->npifs]; + pc->pif = pif; + + if (read_all_buf(fd, &info, sizeof(info)) < 0) + die("Error reading from control socket"); + + if (info.name[sizeof(info.name)-1]) + die("Interface name was not NULL terminated"); + /* Redundant, to make static checkers happy */ + info.name[sizeof(info.name) - 1] = '\0'; + + static_assert(sizeof(info.name) == sizeof(pc->name), + "Mismatching pif name lengths"); + memcpy(pc->name, info.name, sizeof(pc->name)); + + debug("PIF %"PRIu8": %s", pc->pif, pc->name); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_num(conf, pc->pif)) + die("Received duplicate interface identifier"); + + /* O(n^2), but n is bounded by MAX_PIFS */ + if (pif_conf_by_name(conf, pc->name)) + die("Received duplicate interface name"); + + conf->npifs++; + return true; +} + +/** + * show_conf() - Show current configuration obtained from passt/pasta + * @conf: Configuration description + */ +static void show_conf(const struct configuration *conf) +{ + unsigned i; + + for (i = 0; i < conf->npifs; i++) { + const struct pif_configuration *pc = &conf->pif[i]; + printf(" %s\n", pc->name); + printf(" TBD\n"); + } +} + /** * main() - Dynamic reconfiguration client main program * @argc: Argument count @@ -80,6 +201,7 @@ int main(int argc, char **argv) { 0 }, }; struct sockaddr_un a = { AF_UNIX, "" }; + struct configuration conf = { 0 }; const char *optstring = "dh"; struct pesto_hello hello; struct sock_fprog prog; @@ -162,6 +284,18 @@ int main(int argc, char **argv) "Warning: Using experimental protocol version, client and server must match\n"); }

+ if (ntohl(hello.pif_name_size) != PIF_NAME_SIZE) { + die("Server has unexpected pif name size (%" + PRIu32" not %"PRIu32"\n",

trailing '\n'

Fixed in v7.

...

+ ntohl(hello.pif_name_size), PIF_NAME_SIZE); + } + + while (read_pif_conf(s, &conf)) + ; + + printf("passt/pasta configuration (%s)\n", a.sun_path); + show_conf(&conf); + if (shutdown(s, SHUT_RDWR) < 0 || close(s) < 0) die_perror("Error shutting down control socket");

diff --git a/pesto.h b/pesto.h index 92d4df3..1879759 100644 --- a/pesto.h +++ b/pesto.h @@ -17,18 +17,32 @@ /* Version 0 is reserved for unreleased / unsupported experimental versions */ #define PESTO_PROTOCOL_VERSION 0

+/* Maxmimum size of a pif name, including \0 */

Typo copied from pif.h, should be "Maximum"

Fixed in v7.

...

+#define PIF_NAME_SIZE (128) +#define PIF_NONE 0 + /** * struct pesto_hello - Server introduction message - * @magic: PESTO_SERVER_MAGIC - * @version: Version number + * @magic: PESTO_SERVER_MAGIC + * @version: Version number + * @pif_name_size: Server's value for PIF_NAME_SIZE */ struct pesto_hello { char magic[8]; uint32_t version; + uint32_t pif_name_size; } __attribute__ ((__packed__));

static_assert(sizeof(PESTO_SERVER_MAGIC) == sizeof(((struct pesto_hello *)0)->magic), "PESTO_SERVER_MAGIC has wrong size");

+/** + * struct pesto_pif_info - Message with basic metadata about a pif + * @name: Name (\0 terminated) + */ +struct pesto_pif_info { + char name[PIF_NAME_SIZE]; +} __attribute__ ((__packed__)); + #endif /* PESTO_H */ diff --git a/pif.h b/pif.h index 90dd3a3..d770860 100644 --- a/pif.h +++ b/pif.h @@ -11,6 +11,7 @@

#include

+#include "pesto.h" #include "epoll_type.h"

union inany_addr; @@ -24,7 +25,7 @@ union sockaddr_inany; */ enum pif_type { /* Invalid or not present pif */ - PIF_NONE = 0, + PIF_NONE_ = PIF_NONE, /* Host socket interface */ PIF_HOST, /* Qemu socket or namespace tuntap interface */ @@ -36,7 +37,6 @@ enum pif_type { };

/* Maxmimum size of a pif name, including \0 */

Perhaps you could remove the above comment too?

Fixed in v7.

...

-#define PIF_NAME_SIZE (128) extern const char pif_type_str[][PIF_NAME_SIZE];

-- Stefano

On Mon, 4 May 2026 18:10:48 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

Implement serialisation of our current forwarding rules in conf.c, deserialising it to display in the pesto client. Doing this requires adding ip.c, inany.c, bitmap.c, lineread.c and fwd_rule.c to the pesto build. With previous preparations that now requires only a trivial change to lineread.c.

Signed-off-by: David Gibson [sbrivio: Use ntohs() for rule->to instead of htons() in fwd_rule_read(), reported by Jon Maloy] Signed-off-by: Stefano Brivio

With the "pc->fwd.count <= MAX_FWD_RULES" check added below, add:

Reviewed-by: Laurent Vivier

More cosmetics nit below

...

--- Makefile | 17 +++++++++++++---- conf.c | 12 +++++++++++- fwd_rule.c | 41 +++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 4 ++++ lineread.c | 2 +- pesto.c | 37 ++++++++++++++++++++++++++++++++++--- pesto.h | 6 ++++++ 7 files changed, 110 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile index 6da76b4..057e4eb 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c -PESTO_SRCS = pesto.c serialise.c +PESTO_SRCS = pesto.c bitmap.c fwd_rule.c inany.c ip.c lineread.c serialise.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)

MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1 @@ -62,6 +62,8 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h PESTO_HEADERS = common.h pesto.h log.h serialise.h

Duplicate PESTO_HEADERS ^ v

...

+PESTO_HEADERS = common.h pesto.h bitmap.h fwd_rule.h inany.h ip.h lineread.h \ + log.h serialise.h

Fixed in v7.

...

C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -223,15 +225,22 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

passt.cppcheck: BASE_CPPFLAGS += -UPESTO -passt.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c +passt.cppcheck: CPPCHECK_FLAGS += \ + --suppress=unusedFunction:fwd_rule.c \ + --suppress=unusedFunction:serialise.c passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h

passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += \ - --suppress=unusedFunction:serialise.c \ - --suppress=staticFunction:serialise.c + --suppress=unusedFunction:bitmap.c \ + --suppress=unusedFunction:inany.h \ + --suppress=unusedFunction:inany.c \ + --suppress=unusedFunction:ip.h \ + --suppress=unusedFunction:fwd_rule.c \ + --suppress=staticFunction:fwd_rule.c \ + --suppress=unusedFunction:serialise.c pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

qrap.cppcheck: BASE_CPPFLAGS += -DARCH=\"$(TARGET_ARCH)\" diff --git a/conf.c b/conf.c index 3b2fe42..5e4e81e 100644 --- a/conf.c +++ b/conf.c @@ -1939,21 +1939,30 @@ static int conf_send_rules(const struct ctx *c, int fd) unsigned pif;

for (pif = 0; pif < PIF_NUM_TYPES; pif++) { + struct fwd_table *fwd = c->fwd[pif]; struct pesto_pif_info info; + unsigned i; int rc;

- if (!c->fwd[pif]) + if (!fwd) continue;

assert(pif != PIF_NONE);

rc = snprintf(info.name, sizeof(info.name), "%s", pif_name(pif)); assert(rc >= 0 && (size_t)rc < sizeof(info.name)); + info.caps = htonl(fwd->caps); + info.count = htonl(fwd->count);

if (write_u8(fd, pif) < 0) return -1; if (write_all_buf(fd, &info, sizeof(info)) < 0) return -1; + + for (i = 0; i < fwd->count; i++) { + if (fwd_rule_write(fd, &fwd->rules[i])) + return -1; + } }

if (write_u8(fd, PIF_NONE) < 0) @@ -2006,6 +2015,7 @@ static void conf_accept(struct ctx *c) .magic = PESTO_SERVER_MAGIC, .version = htonl(PESTO_PROTOCOL_VERSION), .pif_name_size = htonl(PIF_NAME_SIZE), + .ifnamsiz = htonl(IFNAMSIZ), }; union epoll_ref ref = { .type = EPOLL_TYPE_CONF }; struct ucred uc = { 0 }; diff --git a/fwd_rule.c b/fwd_rule.c index 7fd20dd..da9d893 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -24,6 +24,7 @@ #include "fwd_rule.h" #include "lineread.h" #include "log.h" +#include "serialise.h"

/* Ephemeral port range: values from RFC 6335 */ static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); @@ -645,3 +646,43 @@ void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd)

fwd_rule_parse_ports(fwd, proto, addr, ifname, spec); } + + +/** + * fwd_rule_read() - Read serialised rule from an fd + * @fd: fd to serialise to

should be "fd to deserialise from" (or something like that)

Changed in v7 like you suggested.

...

+ * @rule: Buffer to store rule into + * + * Return: 0 on success, -1 on error (with errno set) + */ +int fwd_rule_read(int fd, struct fwd_rule *rule) +{ + if (read_all_buf(fd, rule, sizeof(*rule))) + return -1; + + /* Byteswap for host */ + rule->first = ntohs(rule->first); + rule->last = ntohs(rule->last); + rule->to = ntohs(rule->to); + + return 0; +} + +/** + * fwd_rule_write() - Serialise rule to an fd + * @fd: fd to serialise to + * @rule: Rule to send + * + * Return: 0 on success, -1 on error (with errno set) + */ +int fwd_rule_write(int fd, const struct fwd_rule *rule) +{ + struct fwd_rule tmp = *rule; + + /* Byteswap for transport */ + tmp.first = htons(tmp.first); + tmp.last = htons(tmp.last); + tmp.to = htons(tmp.to); + + return write_all_buf(fd, &tmp, sizeof(tmp)); +} diff --git a/fwd_rule.h b/fwd_rule.h index f51f1b4..330d49e 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -29,6 +29,8 @@ #define FWD_CAP_UDP BIT(3) #define FWD_CAP_SCAN BIT(4) #define FWD_CAP_IFNAME BIT(5) +#define FWD_CAP_ALL (FWD_CAP_IPV4 | FWD_CAP_IPV6 | FWD_CAP_TCP | \ + FWD_CAP_UDP | FWD_CAP_SCAN | FWD_CAP_IFNAME)

/** * struct fwd_rule - Forwarding rule governing a range of ports @@ -99,6 +101,8 @@ void fwd_probe_ephemeral(void); const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size); void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd); +int fwd_rule_read(int fd, struct fwd_rule *rule); +int fwd_rule_write(int fd, const struct fwd_rule *rule);

/** * fwd_rules_dump() - Dump forwarding rules diff --git a/lineread.c b/lineread.c index b9ceae1..a4269a6 100644 --- a/lineread.c +++ b/lineread.c @@ -19,8 +19,8 @@ #include #include

+#include "common.h" #include "lineread.h" -#include "util.h"

/** * lineread_init() - Prepare for line by line file reading without allocation diff --git a/pesto.c b/pesto.c index 77244b3..4bf9bd8 100644 --- a/pesto.c +++ b/pesto.c @@ -34,6 +34,7 @@ #include "common.h" #include "seccomp_pesto.h" #include "serialise.h" +#include "fwd_rule.h" #include "pesto.h" #include "log.h"

@@ -66,6 +67,7 @@ static void usage(const char *name, FILE *f, int status) struct pif_configuration { uint8_t pif; char name[PIF_NAME_SIZE]; + struct fwd_table fwd; };

struct configuration { @@ -123,6 +125,7 @@ static bool read_pif_conf(int fd, struct configuration *conf) struct pif_configuration *pc; struct pesto_pif_info info; uint8_t pif; + unsigned i;

if (read_u8(fd, &pif) < 0) die("Error reading from control socket"); @@ -151,8 +154,17 @@ static bool read_pif_conf(int fd, struct configuration *conf) static_assert(sizeof(info.name) == sizeof(pc->name), "Mismatching pif name lengths"); memcpy(pc->name, info.name, sizeof(pc->name)); - - debug("PIF %"PRIu8": %s", pc->pif, pc->name); + pc->fwd.caps = ntohl(info.caps); + pc->fwd.count = ntohl(info.count);

We should check that pc->fwd.count <= MAX_FWD_RULES to avoid overflow while scanning the array.

Oops, fixed in v7. I missed it as I wasn't really focusing on pesto being robust, but curiously Coverity Scan missed it as well... weird.

...

+ + debug("PIF %"PRIu8": %s, %"PRIu32" rules, capabilities 0x%"PRIx32 + ":%s%s%s%s%s%s", pc->pif, pc->name, pc->fwd.count, pc->fwd.caps, + pc->fwd.caps & FWD_CAP_IPV4 ? " IPv4" : "", + pc->fwd.caps & FWD_CAP_IPV6 ? " IPv6" : "", + pc->fwd.caps & FWD_CAP_TCP ? " TCP" : "", + pc->fwd.caps & FWD_CAP_UDP ? " UDP" : "", + pc->fwd.caps & FWD_CAP_SCAN ? " scan" : "", + pc->fwd.caps & FWD_CAP_IFNAME ? " ifname" : "");

/* O(n^2), but n is bounded by MAX_PIFS */ if (pif_conf_by_num(conf, pc->pif)) @@ -162,6 +174,18 @@ static bool read_pif_conf(int fd, struct configuration *conf) if (pif_conf_by_name(conf, pc->name)) die("Received duplicate interface name");

+ /* NOTE: We read the fwd rules directly into fwd.rules, rather than + * using fwd_rule_add(). This means we can read and display rules even + * if something has gone wrong (in pesto or passt) and we get rules that + * fwd_rule_add() would reject. It does have the side effect that we + * never assign socket space for the fwd rules, but we don't need that + * within pesto. + */ + for (i = 0; i < pc->fwd.count; i++) { + if (fwd_rule_read(fd, &pc->fwd.rules[i]) < 0) + die("Error reading from control socket"); + } + conf->npifs++; return true; } @@ -177,7 +201,8 @@ static void show_conf(const struct configuration *conf) for (i = 0; i < conf->npifs; i++) { const struct pif_configuration *pc = &conf->pif[i]; printf(" %s\n", pc->name); - printf(" TBD\n"); + fwd_rules_dump(printf, pc->fwd.rules, pc->fwd.count, + " ", "\n"); } }

@@ -290,6 +315,12 @@ int main(int argc, char **argv) ntohl(hello.pif_name_size), PIF_NAME_SIZE); }

+ if (ntohl(hello.ifnamsiz) != IFNAMSIZ) { + die("Server has unexpected IFNAMSIZ (%" + PRIu32" not %"PRIu32"\n",

trailing '\n'

Fixed in v7, and I also added the missing ")".

...

+ ntohl(hello.ifnamsiz), IFNAMSIZ); + } +

Trailing tab :)

Fixed in v7.

...

while (read_pif_conf(s, &conf)) ;

diff --git a/pesto.h b/pesto.h index 1879759..12b0b65 100644 --- a/pesto.h +++ b/pesto.h @@ -26,11 +26,13 @@ * @magic: PESTO_SERVER_MAGIC * @version: Version number * @pif_name_size: Server's value for PIF_NAME_SIZE + * @ifnamsiz: Server's value for IFNAMSIZ */ struct pesto_hello { char magic[8]; uint32_t version; uint32_t pif_name_size; + uint32_t ifnamsiz; } __attribute__ ((__packed__));

static_assert(sizeof(PESTO_SERVER_MAGIC) @@ -40,9 +42,13 @@ static_assert(sizeof(PESTO_SERVER_MAGIC) /** * struct pesto_pif_info - Message with basic metadata about a pif * @name: Name (\0 terminated) + * @caps: Forwarding capabilities for this pif + * @count: Number of forwarding rules for this pif */ struct pesto_pif_info { char name[PIF_NAME_SIZE]; + uint32_t caps; + uint32_t count; } __attribute__ ((__packed__));

#endif /* PESTO_H */

-- Stefano

On Mon, 4 May 2026 11:49:06 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

In pesto we're going to want several levels of error/warning messages, much like passt itself. Particularly as we start to share mode code between passt and pesto, we want to use a similar interface to emit those. However we don't want to use the same implementation - logging to a file or syslog doesn't make sense for the command line tool.

To accomplish this loosely share log.h, but not log.c between pesto and passt. In fact, an #ifdef means even most of log.h isn't actually shared, but we do provide similar warn(), die() etc. macros.

This includes the *_perror() variants, which need strerror(). However, we want to avoid allocations for pesto as we do for passt, and strerror() allocates in some libc versions. Therefore, also move our workaround for this to be shared with pesto.

Signed-off-by: Stefano Brivio [dwg: Based on changes part of a larger patch by Stefano] Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

One little nit below

...

--- Makefile | 6 +++++- common.h | 32 ++++++++++++++++++++++++++++++ log.h | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- pesto.c | 14 ++++---------- util.h | 32 ------------------------------ 5 files changed, 99 insertions(+), 44 deletions(-)

diff --git a/Makefile b/Makefile index 030681b..f6cec8a 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \ vhost_user.h virtio.h vu_common.h QRAP_HEADERS = arp.h ip.h passt.h util.h PASST_REPAIR_HEADERS = linux_dep.h -PESTO_HEADERS = common.h pesto.h +PESTO_HEADERS = common.h pesto.h log.h

C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -121,6 +121,7 @@ qrap: $(QRAP_SRCS) $(QRAP_HEADERS)

passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ @@ -221,9 +222,12 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck %.cppcheck: $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

+passt.cppcheck: BASE_CPPFLAGS += -UPESTO passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unmatchedSuppression pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

diff --git a/common.h b/common.h index a9c115a..2f2e6f1 100644 --- a/common.h +++ b/common.h @@ -21,4 +21,36 @@ /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ #define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)

+/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * strerror_() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *strerror_(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" + #endif /* _COMMON_H */ diff --git a/log.h b/log.h index dbab006..1058ca5 100644 --- a/log.h +++ b/log.h @@ -6,8 +6,63 @@ #ifndef LOG_H #define LOG_H

-#include #include +#include +#include + +#ifdef PESTO + +#include + +#include "common.h" + +extern bool debug_flag; + +#define msg(...) \ + do { \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, "\n"); \ + } while (0) + +#define msg_perror(...) \ + do { \ + int errno_ = errno; \ + FPRINTF(stderr, __VA_ARGS__); \ + FPRINTF(stderr, ": %s\n", strerror_(errno_)); \ + } while (0) + +#define die(...) \ + do { \ + msg(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define die_perror(...) \ + do { \ + msg_perror(__VA_ARGS__); \ + exit(EXIT_FAILURE); \ + } while (0) + +#define warn(...) msg(__VA_ARGS__) +#define warn_perror(...) msg_perror(__VA_ARGS__) +#define info(...) msg(__VA_ARGS__) +#define info_perror(...) msg_perror(__VA_ARGS__) + +#define debug(...) \ + do { \ + if (debug_flag) \ + msg(__VA_ARGS__); \ + } while (0) + +#define debug_perror_(...) \

Why is this "debug_perror_()" and not "debug_perror()"?

I'm not sure, but it's not used anyway, so I dropped it altogether in v7.

...

+ do { \ + if (debug_flag) \ + msg_perror(__VA_ARGS__); \ + } while (0) + +#else /* !PESTO */

-- Stefano