From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=SQT4qhHC; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 291915A065C for ; Fri, 10 Apr 2026 03:03:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1775782993; bh=iwasist1SnqdUJbc/5QrEkA4b8yIpcsNj+sSHI88JD4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SQT4qhHCDFQogpnksh0ezB15ZpmfYwpik0/BtzdL5S3+5XU8YKtPIJaq3dJBjkoiz gviy9/kEVlx3m1S3NLTFZS0UbIlA4B8WlS0wQuDYyVOhUeX0L2734lzvxcEjwegsak v1mzeD4cUzXkh3iX5i8/4/ZFc1YJ88UHm0n1vkGfqVZb8fzzT/GSqoTCPy8fiPAgcC /8qeHjoW6o7djMCtV4lzSVsL+lSmKqbQLdsu5LX6vT+YuEc/r8h+Q+zkgf4laDOuVI DZDAsH06nrQSjlvxv8smBxBVJ8TqGn7YMX/JJyn++GZW3Ouih3q+3yDLe/PilkEMcq 8nhhD9q1l5NMg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fsJSx0RJLz4wTd; Fri, 10 Apr 2026 11:03:13 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v2 11/23] fwd: Improve error handling in fwd_rule_add() Date: Fri, 10 Apr 2026 11:02:57 +1000 Message-ID: <20260410010309.736855-12-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260410010309.736855-1-david@gibson.dropbear.id.au> References: <20260410010309.736855-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: OXFNYFONTP7MEM25OOICQJI6R6ZHFKQB X-Message-ID-Hash: OXFNYFONTP7MEM25OOICQJI6R6ZHFKQB X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: fwd_rule_add() sanity checks the given rule, however all errors are fatal: either they're assert()s in the case of things that callers should have already verified, or die()s if we run out of space for the new rule. This won't suffice any more when we allow rule updates from a configuration client. We don't want to trust the input we get from the client any more than we have to. Replace the assert()s and die()s with a return value. Also include warn()s so that the user gets a more specific idea of the problem in the logs or stderr. Signed-off-by: David Gibson --- conf.c | 15 ++++++++++++--- fwd.c | 41 +++++++++++++++++++++++++++++++---------- fwd.h | 2 +- fwd_rule.c | 3 +-- fwd_rule.h | 1 + 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/conf.c b/conf.c index b871646f..5c913820 100644 --- a/conf.c +++ b/conf.c @@ -157,6 +157,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname, .proto = proto, .flags = flags, }; + char rulestr[FWD_RULE_STRLEN]; unsigned delta = to - first; unsigned base, i; @@ -207,20 +208,28 @@ static void conf_ports_range_except(const struct ctx *c, char optname, rulev.addr = inany_loopback4; fwd_rule_conflict_check(&rulev, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rulev); + if (fwd_rule_add(fwd, &rulev) < 0) + goto fail; } if (c->ifi6) { rulev.addr = inany_loopback6; fwd_rule_conflict_check(&rulev, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rulev); + if (fwd_rule_add(fwd, &rulev) < 0) + goto fail; } } else { fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rule); + 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))); } /** diff --git a/fwd.c b/fwd.c index c9637525..3e87169b 100644 --- a/fwd.c +++ b/fwd.c @@ -335,24 +335,44 @@ void fwd_rule_init(struct ctx *c) * 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 */ -void 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; unsigned num = (unsigned)new->last - new->first + 1; unsigned port; - assert(!(new->flags & ~allowed_flags)); - /* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */ - assert(!(new->flags & FWD_DUAL_STACK_ANY) || - inany_equals(&new->addr, &inany_any6)); - assert(new->first <= new->last); + if (new->first > new->last) { + warn("Rule has invalid port range %u-%u", + new->first, new->last); + 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 (fwd->count >= ARRAY_SIZE(fwd->rules)) - die("Too many port forwarding ranges"); - if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) - die("Too many listening sockets"); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + 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++) @@ -360,6 +380,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) fwd->rules[fwd->count++] = *new; fwd->sock_count += num; + return 0; } /** diff --git a/fwd.h b/fwd.h index 96b8c608..43bfeadb 100644 --- a/fwd.h +++ b/fwd.h @@ -85,7 +85,7 @@ struct fwd_scan { #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ void fwd_rule_init(struct ctx *c); -void 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); 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 5bc94efe..47d8df1c 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -45,8 +45,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) */ __attribute__((noinline)) #endif -static const char *fwd_rule_fmt(const struct fwd_rule *rule, - char *dst, size_t size) +const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size) { const char *percent = *rule->ifname ? "%" : ""; const char *weak = "", *scan = ""; diff --git a/fwd_rule.h b/fwd_rule.h index f852be39..8506a0c4 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -51,6 +51,7 @@ struct fwd_rule { + sizeof(" []%:- => - (best effort) (auto-scan)")) 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); -- 2.53.0