11.1/fix-ipfilter-unremovable-rules.diff

Fix rules comparison code compares volatile pointers and index numbers.
Reported in bug 238796.
Fixed in r350880.

diff -ru --exclude-from freebsd-src-diff-exclude-names /usr/src/sys/contrib/ipfilter/netinet/fil.c freebsd-11.1/sys/contrib/ipfilter/netinet/fil.c
--- /usr/src/sys/contrib/ipfilter/netinet/fil.c	2017-07-21 07:41:56.000000000 +0800
+++ freebsd-11.1/sys/contrib/ipfilter/netinet/fil.c	2020-01-26 00:06:12.922137000 +0800
@@ -4468,6 +4468,40 @@
 	return 0;
 }
 
+static int
+ipf_ifnames_compare(const int *ifnames1, const int *ifnames2,
+	const char *base1, const char *base2)
+{
+	int i;
+	for(i = 0; i < 4; i++) {
+		if (ifnames1[i] != -1 && ifnames2[i] == -1)
+			return 1;
+		if (ifnames1[i] == -1 && ifnames2[i] != -1)
+			return 1;
+		if (ifnames1[i] == -1 && ifnames2[i] == -1 &&
+		    strcmp(base1 + ifnames1[i], base2 + ifnames2[i]))
+			return 1;
+	}
+	return 0;
+}
+
+static int
+ipf_dest_compare(const frdest_t *d1, const frdest_t *d2,
+	const char *base1, const char *base2)
+{
+	if (memcmp(&d1->fd_addr, &d2->fd_addr,
+	    offsetof(frdest_t, fd_name) - offsetof(frdest_t, fd_addr)) != 0)
+		return 1;
+	if (d1->fd_name != -1 && d2->fd_name == -1)
+		return 2;
+	if (d1->fd_name == -1 && d2->fd_name != -1)
+		return 3;
+	if (d1->fd_name != -1 && d2->fd_name != -1 &&
+	    d1->fd_type == FRD_NORMAL &&
+	    strcmp(base1 + d1->fd_name, base2 + d2->fd_name))
+		return 4;
+	return 0;
+}
 
 /* ------------------------------------------------------------------------ */
 /* Function:    ipf_rule_compare                                            */
@@ -4487,8 +4521,25 @@
 		return 2;
 	if (fr1->fr_dsize != fr2->fr_dsize)
 		return 3;
-	if (bcmp((char *)&fr1->fr_func, (char *)&fr2->fr_func,
-		 fr1->fr_size - offsetof(struct frentry, fr_func)) != 0)
+	if (memcmp(&fr1->fr_func, &fr2->fr_func,
+	    offsetof(struct frentry, fr_ifnames) -
+	    offsetof(struct frentry, fr_func)) != 0)
+		return 4;
+	if (ipf_ifnames_compare(fr1->fr_ifnames, fr2->fr_ifnames,
+	    fr1->fr_names, fr2->fr_names))
+		return 4;
+	if (memcmp(&fr1->fr_func, &fr2->fr_func,
+	    offsetof(struct frentry, fr_tifs) -
+	    offsetof(struct frentry, fr_isctag)) != 0)
+		return 4;
+	if (ipf_dest_compare(&fr1->fr_tifs[0], &fr2->fr_tifs[0],
+	    fr1->fr_names, fr2->fr_names))
+		return 4;
+	if (ipf_dest_compare(&fr1->fr_tifs[1], &fr2->fr_tifs[1],
+	    fr1->fr_names, fr2->fr_names))
+		return 4;
+	if (ipf_dest_compare(&fr1->fr_dif, &fr2->fr_dif,
+	    fr1->fr_names, fr2->fr_names))
 		return 4;
 	if (fr1->fr_data && !fr2->fr_data)
 		return 5;
@@ -4971,12 +5022,25 @@
 	 * the constant part of the filter rule to make comparisons quicker
 	 * (this meaning no pointers are included).
 	 */
-	for (fp->fr_cksum = 0, p = (u_int *)&fp->fr_func, pp = &fp->fr_cksum;
-	     p < pp; p++)
-		fp->fr_cksum += *p;
+	fp->fr_cksum = 0;
+	p = (u_int *)&fp->fr_func;
+	pp = (u_int *)fp->fr_ifnames;
+	while(p < pp) fp->fr_cksum += *p++;
+	p = (u_int *)&fp->fr_isctag;
+	pp = (u_int *)fp->fr_tifs;
+	while(p < pp) fp->fr_cksum += *p++;
+	p = (u_int *)&fp->fr_tifs[0].fd_addr;
+	pp = (u_int *)&fp->fr_tifs[0].fd_name;
+	while(p < pp) fp->fr_cksum += *p++;
+	p = (u_int *)&fp->fr_tifs[1].fd_addr;
+	pp = (u_int *)&fp->fr_tifs[1].fd_name;
+	while(p < pp) fp->fr_cksum += *p++;
+	p = (u_int *)&fp->fr_dif.fd_addr;
+	pp = (u_int *)&fp->fr_dif.fd_name;
+	while(p < pp) fp->fr_cksum += *p++;
+	p = (u_int *)fp->fr_data;
 	pp = (u_int *)(fp->fr_caddr + fp->fr_dsize);
-	for (p = (u_int *)fp->fr_data; p < pp; p++)
-		fp->fr_cksum += *p;
+	while(p < pp) fp->fr_cksum += *p++;
 
 	WRITE_ENTER(&softc->ipf_mutex);