pfctl: fix how source and state limiters are wired into rbtrees

i messed up when we added support for names on these things. the
id and names are each supposed to be unique, which is checked by
putting the one limiter into an rb tree based on their id and another
based on their name. unfortunately i used the same RBT_ENTRY fields
for both trees, which meant using both trees on the same limiter
corrupted the topology, which goes badly when you want to use
multiple limiters.

found by, tested, and ok dgl@ (who is not me, this is not a typo)
ok jmatthew@

Obtained from:	OpenBSD, dlg <dlg@openbsd.org>, f951d642cc
Sponsored by:	Rubicon Communications, LLC ("Netgate")

2 files changed, 16 insertions, 11 deletions

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index a7bba4055b06..48e6a053a842 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -152,13 +152,13 @@ int	 pfctl_call_cleartables(int, int, struct pfr_anchoritem *);
 int	 pfctl_call_clearanchors(int, int, struct pfr_anchoritem *);
 int	 pfctl_call_showtables(int, int, struct pfr_anchoritem *);
 
-RB_PROTOTYPE(pfctl_statelim_ids, pfctl_statelim, entry,
+RB_PROTOTYPE(pfctl_statelim_ids, pfctl_statelim, id_entry,
     pfctl_statelim_id_cmp);
-RB_PROTOTYPE(pfctl_statelim_nms, pfctl_statelim, entry,
+RB_PROTOTYPE(pfctl_statelim_nms, pfctl_statelim, nm_entry,
     pfctl_statelim_nm_cmp);
-RB_PROTOTYPE(pfctl_sourcelim_ids, pfctl_sourcelim, entry,
+RB_PROTOTYPE(pfctl_sourcelim_ids, pfctl_sourcelim, id_entry,
     pfctl_sourcelim_id_cmp);
-RB_PROTOTYPE(pfctl_sourcelim_nms, pfctl_sourcelim, entry,
+RB_PROTOTYPE(pfctl_sourcelim_nms, pfctl_sourcelim, nm_entry,
     pfctl_sourcelim_nm_cmp);
 
 enum showopt_id {
@@ -4187,7 +4187,8 @@ pfctl_statelim_id_cmp(const struct pfctl_statelim *a,
 	return (0);
 }
 
-RB_GENERATE(pfctl_statelim_ids, pfctl_statelim, entry, pfctl_statelim_id_cmp);
+RB_GENERATE(pfctl_statelim_ids, pfctl_statelim, id_entry,
+    pfctl_statelim_id_cmp);
 
 static inline int
 pfctl_statelim_nm_cmp(const struct pfctl_statelim *a,
@@ -4196,7 +4197,8 @@ pfctl_statelim_nm_cmp(const struct pfctl_statelim *a,
 	return (strcmp(a->ioc.name, b->ioc.name));
 }
 
-RB_GENERATE(pfctl_statelim_nms, pfctl_statelim, entry, pfctl_statelim_nm_cmp);
+RB_GENERATE(pfctl_statelim_nms, pfctl_statelim, nm_entry,
+    pfctl_statelim_nm_cmp);
 
 int
 pfctl_add_statelim(struct pfctl *pf, struct pfctl_statelim *stlim)
@@ -4253,7 +4255,7 @@ pfctl_sourcelim_id_cmp(const struct pfctl_sourcelim *a,
 	return (0);
 }
 
-RB_GENERATE(pfctl_sourcelim_ids, pfctl_sourcelim, entry,
+RB_GENERATE(pfctl_sourcelim_ids, pfctl_sourcelim, id_entry,
     pfctl_sourcelim_id_cmp);
 
 static inline int
@@ -4263,7 +4265,7 @@ pfctl_sourcelim_nm_cmp(const struct pfctl_sourcelim *a,
 	return (strcmp(a->ioc.name, b->ioc.name));
 }
 
-RB_GENERATE(pfctl_sourcelim_nms, pfctl_sourcelim, entry,
+RB_GENERATE(pfctl_sourcelim_nms, pfctl_sourcelim, nm_entry,
     pfctl_sourcelim_nm_cmp);
 
 int
@@ -4272,8 +4274,9 @@ pfctl_add_sourcelim(struct pfctl *pf, struct pfctl_sourcelim *srlim)
 	struct pfctl_sourcelim *osrlim;
 
 	osrlim = RB_INSERT(pfctl_sourcelim_ids, &pf->sourcelim_ids, srlim);
-	if (osrlim != NULL)
+	if (osrlim != NULL) {
 		return (-1);
+	}
 
 	osrlim = RB_INSERT(pfctl_sourcelim_nms, &pf->sourcelim_nms, srlim);
 	if (osrlim != NULL) {
diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h
index 8934238da148..631a6b9a32ea 100644
--- a/sbin/pfctl/pfctl_parser.h
+++ b/sbin/pfctl/pfctl_parser.h
@@ -77,7 +77,8 @@ struct pfr_buffer;	/* forward definition */
 
 struct pfctl_statelim {
 	struct pfctl_state_lim		 ioc;
-	RB_ENTRY(pfctl_statelim)	 entry;
+	RB_ENTRY(pfctl_statelim)	 id_entry;
+	RB_ENTRY(pfctl_statelim)	 nm_entry;
 };
 
 RB_HEAD(pfctl_statelim_ids, pfctl_statelim);
@@ -85,7 +86,8 @@ RB_HEAD(pfctl_statelim_nms, pfctl_statelim);
 
 struct pfctl_sourcelim {
 	struct pfctl_source_lim		 ioc;
-	RB_ENTRY(pfctl_sourcelim)	 entry;
+	RB_ENTRY(pfctl_sourcelim)	 id_entry;
+	RB_ENTRY(pfctl_sourcelim)	 nm_entry;
 };
 
 RB_HEAD(pfctl_sourcelim_ids, pfctl_sourcelim);