The branch main has been updated by ae:
URL:
https://cgit.FreeBSD.org/src/commit/?id=3b67473b97574d13eef8302c61c7245b3b3c52c1
commit 3b67473b97574d13eef8302c61c7245b3b3c52c1
Author: Andrey V. Elsukov <a...@freebsd.org>
AuthorDate: 2025-07-22 08:12:36 +0000
Commit: Andrey V. Elsukov <a...@freebsd.org>
CommitDate: 2025-08-03 09:54:39 +0000
ipfw: add additional handling for orphaned states
When parent rule of dynamic state is deleted and
net.inet.ip.fw.dyn_keep_states is enabled, dynamic states are kept
working and such states are called ORPHANED.
Orphaned states still keep pointer to original parent rule. And in
case when rule action is skipto this can lead to unpredictable
consequences. To avoid this problem add special handling for skipto
action when we have found ORPHANED state.
Check that new rule has the same opcode and skipto number for
O_SKIPTO rule action.
Obtained from: Yandex LLC
Sponsored by: Yandex LLC
Differential Revision: https://reviews.freebsd.org/D51459
---
sys/netpfil/ipfw/ip_fw_dynamic.c | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/sys/netpfil/ipfw/ip_fw_dynamic.c b/sys/netpfil/ipfw/ip_fw_dynamic.c
index 40598cef8076..9694c145e112 100644
--- a/sys/netpfil/ipfw/ip_fw_dynamic.c
+++ b/sys/netpfil/ipfw/ip_fw_dynamic.c
@@ -1323,6 +1323,33 @@ dyn_lookup_ipv6_parent_locked(const struct ipfw_flow_id
*pkt, uint32_t zoneid,
#endif /* INET6 */
+static int
+dyn_handle_orphaned(struct ip_fw *old_rule, struct dyn_data *data)
+{
+ struct ip_fw *rule;
+ const ipfw_insn *cmd, *old_cmd;
+
+ old_cmd = ACTION_PTR(old_rule);
+ switch (old_cmd->opcode) {
+ case O_SETMARK:
+ case O_SKIPTO:
+ /*
+ * Rule pointer was changed. For O_SKIPTO action it can be
+ * dangerous to keep use old rule. If new rule has the same
+ * action and the same destination number, then use this dynamic
+ * state. Otherwise it is better to create new one.
+ */
+ rule = V_layer3_chain.map[data->f_pos];
+ cmd = ACTION_PTR(rule);
+ if (cmd->opcode != old_cmd->opcode ||
+ cmd->len != old_cmd->len || cmd->arg1 != old_cmd->arg1 ||
+ insntoc(cmd, u32)->d[0] != insntoc(old_cmd, u32)->d[0])
+ return (-1);
+ break;
+ }
+ return (0);
+}
+
/*
* Lookup dynamic state.
* pkt - filled by ipfw_chk() ipfw_flow_id;
@@ -1426,8 +1453,13 @@ ipfw_dyn_lookup_state(const struct ip_fw_args *args,
const void *ulp,
* changed to point to the penultimate rule.
*/
MPASS(V_layer3_chain.n_rules > 1);
- data->chain_id = V_layer3_chain.id;
- data->f_pos = V_layer3_chain.n_rules - 2;
+ if (dyn_handle_orphaned(rule, data) == 0) {
+ data->chain_id = V_layer3_chain.id;
+ data->f_pos = V_layer3_chain.n_rules -
2;
+ } else {
+ rule = NULL;
+ info->direction = MATCH_NONE;
+ }
} else {
rule = NULL;
info->direction = MATCH_NONE;
