⬢ DragonFlyBSD Kernel Audit
DF-3434 / fix.diff
← back to finding ↓ download raw
--- a/sys/net/ipfw3_basic/ip_fw3_state.c
+++ b/sys/net/ipfw3_basic/ip_fw3_state.c
@@ -315,9 +315,26 @@
 		}
 	}
 	if (*the_count <= the_max) {
+		s = RB_FIND(fw3_state_tree, the_tree, k);
+		if (s != NULL) {
+			/*
+			 * Already tracked: limit-type rules (arg3 != 0)
+			 * skip the RB_FIND above, so re-check here instead
+			 * of allocating a duplicate the tree will reject
+			 * (DF-3434: the rejected allocation was leaked and
+			 * kprintf("oops") fired once per packet).
+			 */
+			s->timestamp = time_uptime;
+			goto done;
+		}
 		(*the_count)++;
 		s = kmalloc(LEN_FW3_STATE, M_IPFW3_STATE,
 				M_INTWAIT | M_NULLOK | M_ZERO);
+		if (s == NULL) {
+			/* DF-3435: M_NULLOK permits NULL at the type
+			 * limit; do not write through it */
+			goto done;
+		}
 		s->src_addr = k->src_addr;
 		s->dst_addr = k->dst_addr;
 		s->src_port = k->src_port;
@@ -325,7 +342,7 @@
 		s->stub = *f;
 		s->timestamp = time_uptime;
 		if (RB_INSERT(fw3_state_tree, the_tree, s)) {
-			kprintf("oops\n");
+			kfree(s, M_IPFW3_STATE);
 		}
 	}
 done:
@@ -542,6 +559,11 @@
 	struct ipfw3_state_context *state_ctx = fw3_state_ctx[mycpuid];
 	struct ipfw3_state *s, *tmp;
 
+	if (state_ctx == NULL) {
+		/* DF-3436: fini_dispatch may have run first on this cpu */
+		netisr_forwardmsg_all(&nmsg->base, mycpuid + 1);
+		return;
+	}
 	RB_FOREACH_SAFE(s, fw3_state_tree, &state_ctx->rb_icmp_in, tmp) {
 		if (time_uptime - s->timestamp > sysctl_var_icmp_timeout) {
 			RB_REMOVE(fw3_state_tree, &state_ctx->rb_icmp_in, s);
@@ -688,7 +710,9 @@
 		0, ip_fw3_state_fini_dispatch);
 
 	netisr_domsg(&msg, 0);
-	callout_stop(&ip_fw3_state_cleanup_callout);
+	/* DF-3436: the handler re-arms itself; cancel synchronously and
+	 * disconnect the callout (re-initialized by the next MOD_LOAD) */
+	callout_terminate(&ip_fw3_state_cleanup_callout);
 }
 
 void