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=bpkRRw+o; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 743405A0623 for ; Tue, 07 Apr 2026 05:16:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1775531792; bh=BTId9cfQ5VTrigGn+CQY6CjsYJ37wDsspdqF0DwqJ/o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bpkRRw+of5sW831QEvC5YOTwUsCvlqdlxWwCo8Q9oqPQ1nRWOshiogiyNiRzwOpth 4Inhr5ECCAruAVr9pgj54Hg8hRunK/lRcxQ+mC3Y2dMZpryNriAdHsSmtEI68iZmJZ lvB3NqP5jyfIZmzVfO5/3nc/NwZWB8o5RJrmSf82nMj6e8+DDmJLGNZfKSXop7zhv6 gwBHveEzsax7vP1vLYYhIywEYdtjLuPaAZL60Idk6WgJ3sROiANe5Bka5mqyxjQ3sP M6B8bGlkQM+dvwGeKihh7+266vBgAIkvLnNA2HJ1Uh0agMq80XWtdRZejjS54QYrZG 0hl/tbO6OcVCA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fqWZ85t8Gz4wL6; Tue, 07 Apr 2026 13:16:32 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 11/18] fwd: Improve error handling in fwd_rule_add() Date: Tue, 7 Apr 2026 13:16:23 +1000 Message-ID: <20260407031630.2457081-12-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260407031630.2457081-1-david@gibson.dropbear.id.au> References: <20260407031630.2457081-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: XT4SPFZB6FAEBP6EPQUUZXPSXSTTAD34 X-Message-ID-Hash: XT4SPFZB6FAEBP6EPQUUZXPSXSTTAD34 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 configuraiton 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 | 40 ++++++++++++++++++++++++++++++---------- fwd.h | 2 +- fwd_rule.c | 3 +-- fwd_rule.h | 1 + 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/conf.c b/conf.c index a93837cc..cd30f2f8 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..cee2c7f6 100644 --- a/fwd.c +++ b/fwd.c @@ -335,24 +335,43 @@ 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("Forwarding rule has invalid port range %u-%u", + new->first, new->last); + return -EINVAL; + } + if (new->flags & ~allowed_flags) { + warn("Forwarding rule has invalid flags 0x%x", + new->flags & ~allowed_flags); + return -EINVAL; + } + if (new->flags & FWD_DUAL_STACK_ANY && + !inany_equals(&new->addr, &inany_any6)) { + warn("Dual stack forward must have unspecified address"); + return -EINVAL; + } - 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"); + if (fwd->count >= ARRAY_SIZE(fwd->rules)) { + warn("Too many forwarding rules (maximum %u)", + ARRAY_SIZE(fwd->rules)); + return -ENOSPC; + } + if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { + warn( +"Forwarding 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 +379,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 1d74cbd2..c56749e1 100644 --- a/fwd.h +++ b/fwd.h @@ -83,7 +83,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 4d5048f9..ef9308c2 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -39,8 +39,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) * @dst: Buffer to store output (should have FWD_RULE_STRLEN bytes) * @size: Size of @dst */ -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