From nobody Sun Aug 3 09:53:04 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bvw3h6NGVz637xr; Sun, 03 Aug 2025 09:53:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bvw3h43Tzz3s0g; Sun, 03 Aug 2025 09:53:04 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1754214784; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=UttMw9tNvzWe8NFQJe+FMMH9bpMr4XuAOT6O2bqF+hA=; b=D8Pp0LcI8v7K75SvvHH7hYvESoil5Wkvzd0GMDS0A8VbsjQEDwtHbRi+iFbmbippNytAXD rUZQrjy8WTMzoLMRCiT3ECLEC4uu0JxsJ+NkCawfm9Ru7ryIRUtPaNDhXxJqPEDAYB1erz 9FKEWzE6kGeBRYe5gcBZGUp36I8h+iDimuHlkJYIyrBurnY8UQ+SECxQQng9ZLKeFurs8M c5Hh5S5wpIN+mqsne6+qxkuXMW7YWpbjoVpyjvMGBdDM6YZsNQ1XmLq5c147U/n3HEI7PM eqRMvaBgOF2ntpX2QyF8xot82vMt5s7Tzp+bOdgEa0b6oSOKs0dGBMGajpFzvQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1754214784; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=UttMw9tNvzWe8NFQJe+FMMH9bpMr4XuAOT6O2bqF+hA=; b=Jknt3Q5wimpYTg1Z+nOtwXAWNXRniGP3QWrzQ0x645Z/ccSEQoeoXS+qNAH2+ZPyCzmUkx DzdpIzLJqM1oxZebEfy9+vPcSI3NoI7aL7AgfdNQZ/RtxNd5MzqbX85Qdi8DVcIpNi/DSX hIkV4U+8vruiEyf3t66+NevY0gMtUr1gvv87IyQhFaBGUfg6x3YPoH5OIG0gx6CZNS0BtP QcbFYCfJtPoPrA+cQjKJpnwodOXtJ2q6Fm+a+qTJir5njr4fAopEaBedouNavKU0BiIn4N 2e2YwMsS3E2hQ6yN2CE6EaQqlLCAu10vLrA9rp3hSqsDwP7kIr3ev+u9nXI0+A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1754214784; a=rsa-sha256; cv=none; b=ro3v5ZTXVLxbo9FC/+RYYR76cFDBoH7qXkFTjml6sqOC7RywgIf4opr/ZBH5omuYYnh60q AhYlgWcEYNSixl2quYY8PtGbBlR9t2QKEx9u8x2IhxAre0EwaiSCxpIyngelRMHNgJA9Fk zi9ZsdYoMz9oZbf/gGZs7Rg/tJoZRsXfT8gU2AGF19EByk9Of3KfKMjQb4SzKTU6dbv4gb 84Ud9DtifgZnppZReq7IX4yCw5hN1ziPxHjgW2Fv2UzwvXKKewFL7u6nBm2vO63g3sG2J8 dB8Q3Hgpldr4sxT99GcFzjCl35ueNiE27+nkVUMDvGHImouo4i0vXNup0KXweg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4bvw3h3cq5zyWr; Sun, 03 Aug 2025 09:53:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 5739r4TP074970; Sun, 3 Aug 2025 09:53:04 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 5739r4Z0074967; Sun, 3 Aug 2025 09:53:04 GMT (envelope-from git) Date: Sun, 3 Aug 2025 09:53:04 GMT Message-Id: <202508030953.5739r4Z0074967@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Andrey V. Elsukov" Subject: git: 91ed876385d4 - main - ipfw: forbid adding keep-state rules that depend on tablearg List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: ae X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 91ed876385d4531b6ab2f9176be969318e1aefc1 Auto-Submitted: auto-generated The branch main has been updated by ae: URL: https://cgit.FreeBSD.org/src/commit/?id=91ed876385d4531b6ab2f9176be969318e1aefc1 commit 91ed876385d4531b6ab2f9176be969318e1aefc1 Author: Andrey V. Elsukov AuthorDate: 2025-07-22 08:02:17 +0000 Commit: Andrey V. Elsukov CommitDate: 2025-08-03 09:46:51 +0000 ipfw: forbid adding keep-state rules that depend on tablearg tablearg value is determined after making table lookup. When we applying rule action that uses dynamic state, such lookup was not done and thus rule action can not determine what table and what value should be used as tablearg. To prevent this add check for such rules and return error when they are added. Obtained from: Yandex LLC Sponsored by: Yandex LLC Differential Revision: https://reviews.freebsd.org/D51458 --- sys/netpfil/ipfw/ip_fw_private.h | 1 + sys/netpfil/ipfw/ip_fw_sockopt.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/sys/netpfil/ipfw/ip_fw_private.h b/sys/netpfil/ipfw/ip_fw_private.h index 79b3ed43f63b..c490d2849a7d 100644 --- a/sys/netpfil/ipfw/ip_fw_private.h +++ b/sys/netpfil/ipfw/ip_fw_private.h @@ -489,6 +489,7 @@ struct obj_idx { struct rule_check_info { uint16_t flags; /* rule-specific check flags */ +#define IPFW_RCIFLAG_HAS_STATE 0x0001 uint16_t object_opcodes; /* num of opcodes referencing objects */ uint16_t urule_numoff; /* offset of rulenum in bytes */ uint8_t version; /* rule version */ diff --git a/sys/netpfil/ipfw/ip_fw_sockopt.c b/sys/netpfil/ipfw/ip_fw_sockopt.c index 19f5fff2749a..5d57759ffb00 100644 --- a/sys/netpfil/ipfw/ip_fw_sockopt.c +++ b/sys/netpfil/ipfw/ip_fw_sockopt.c @@ -1311,6 +1311,9 @@ ipfw_check_rule(struct ip_fw_rule *rule, size_t size, return (check_ipfw_rule_body(rule->cmd, rule->cmd_len, ci)); } +#define CHECK_TARG(a, c) \ + ((a) == IP_FW_TARG && ((c)->flags & IPFW_RCIFLAG_HAS_STATE)) + enum ipfw_opcheck_result ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) { @@ -1326,6 +1329,7 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) if (cmdlen != F_INSN_SIZE(ipfw_insn_kidx)) return (BAD_SIZE); ci->object_opcodes++; + ci->flags |= IPFW_RCIFLAG_HAS_STATE; break; case O_PROTO: case O_IP_SRC_ME: @@ -1410,6 +1414,8 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) cmd->arg1 & 0x7FFF); return (FAILED); } + if (CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); case O_UID: @@ -1518,11 +1524,16 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) case O_QUEUE: if (cmdlen != F_INSN_SIZE(ipfw_insn)) return (BAD_SIZE); + if (CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); case O_FORWARD_IP: if (cmdlen != F_INSN_SIZE(ipfw_insn_sa)) return (BAD_SIZE); + if (insntoc(cmd, sa)->sa.sin_addr.s_addr == INADDR_ANY && + (ci->flags & IPFW_RCIFLAG_HAS_STATE)) + goto bad_targ; return (CHECK_ACTION); #ifdef INET6 case O_FORWARD_IP6: @@ -1537,6 +1548,8 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) return (FAILED); if (cmdlen != F_INSN_SIZE(ipfw_insn)) return (BAD_SIZE); + if (CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); case O_NETGRAPH: case O_NGTEE: @@ -1544,12 +1557,16 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) return (FAILED); if (cmdlen != F_INSN_SIZE(ipfw_insn)) return (BAD_SIZE); + if (CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); case O_NAT: if (!IPFW_NAT_LOADED) return (FAILED); if (cmdlen != F_INSN_SIZE(ipfw_insn_nat)) return (BAD_SIZE); + if (CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); case O_SKIPTO: @@ -1557,6 +1574,11 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) case O_SETMARK: if (cmdlen != F_INSN_SIZE(ipfw_insn_u32)) return (BAD_SIZE); + /* O_CALLRETURN + F_NOT means 'return' opcode. */ + if (cmd->opcode != O_CALLRETURN || (cmd->len & F_NOT) == 0) { + if (CHECK_TARG(insntoc(cmd, u32)->d[0], ci)) + goto bad_targ; + } return (CHECK_ACTION); case O_CHECK_STATE: @@ -1577,6 +1599,8 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) case O_REASS: if (cmdlen != F_INSN_SIZE(ipfw_insn)) return (BAD_SIZE); + if (cmd->opcode == O_SETDSCP && CHECK_TARG(cmd->arg1, ci)) + goto bad_targ; return (CHECK_ACTION); #ifdef INET6 case O_IP6_SRC: @@ -1627,6 +1651,13 @@ ipfw_check_opcode(ipfw_insn **pcmd, int *plen, struct rule_check_info *ci) } } return (SUCCESS); +bad_targ: + /* + * For dynamic states we can not correctly initialize tablearg value, + * because we don't go through rule's opcodes except rule action. + */ + printf("ipfw: tablearg is not allowed with dynamic states\n"); + return (FAILED); } static __noinline int