Attachment 206385 Details for Bug 238796 – Refactored and refined.

[patch] Refactored and refined.

PR238796-fix-refined.diff (text/plain), 4.48 KB, created by Cy Schubert on 2019-08-09 03:41:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Cy Schubert

Created: 2019-08-09 03:41:35 UTC

Size: 4.48 KB

patch

obsolete

>diff --git a/sys/contrib/ipfilter/netinet/fil.c b/sys/contrib/ipfilter/netinet/fil.c
>index f1c2e34e0cf..26b7d54ceee 100644
>--- a/sys/contrib/ipfilter/netinet/fil.c
>+++ b/sys/contrib/ipfilter/netinet/fil.c
>@@ -4418,6 +4418,28 @@ ipf_matchicmpqueryreply(v, ic, icmp, rev)
> }
> 
> 
>+/*
>+ * IFNAMES are located in the variable length field starting at
>+ * frentry.fr_names. As pointers within the struct cannot be passed
>+ * to the kernel from ipf(8), an offset is used. An offset of -1 means it
>+ * is unused (invalid). If it is used (valid) it is an offset to the
>+ * character string of an interface name or a comment. The following
>+ * macros will assist those who follow to understand the code.
>+ */
>+#define IPF_IFNAME_VALID(_a)	(_a != -1)
>+#define IPF_IFNAME_INVALID(_a)	(_a == -1)
>+#define IPF_IFNAMES_DIFFERENT(_a)	\
>+	!((IPF_IFNAME_INVALID(fr1->_a) &&	\
>+	IPF_IFNAME_INVALID(fr2->_a)) ||	\
>+	(IPF_IFNAME_VALID(fr1->_a) &&	\
>+	IPF_IFNAME_VALID(fr2->_a) &&	\
>+	!strcmp(FR_NAME(fr1, _a), FR_NAME(fr2, _a))))
>+#define IPF_FRDEST_DIFFERENT(_a)	\
>+	(memcmp(&fr1->_a.fd_addr, &fr2->_a.fd_addr,	\
>+	offsetof(frdest_t, fd_name) - offsetof(frdest_t, fd_addr)) ||	\
>+	IPF_IFNAMES_DIFFERENT(_a.fd_name))
>+
>+
> /* ------------------------------------------------------------------------ */
> /* Function:    ipf_rule_compare                                            */
> /* Parameters:  fr1(I) - first rule structure to compare                    */
>@@ -4430,22 +4452,50 @@ ipf_matchicmpqueryreply(v, ic, icmp, rev)
> static int
> ipf_rule_compare(frentry_t *fr1, frentry_t *fr2)
> {
>+	int i;
>+
> 	if (fr1->fr_cksum != fr2->fr_cksum)
> 		return (1);
> 	if (fr1->fr_size != fr2->fr_size)
> 		return (2);
> 	if (fr1->fr_dsize != fr2->fr_dsize)
> 		return (3);
>-	if (bcmp((char *)&fr1->fr_func, (char *)&fr2->fr_func, FR_CMPSIZ(fr1))
>+	if (bcmp((char *)&fr1->fr_func, (char *)&fr2->fr_func, FR_CMPSIZ)
> 	    != 0)
> 		return (4);
>+	/*
>+	 * XXX:	There is still a bug here as different rules with the
>+	 *	the same interfaces but in a different order will compare
>+	 *	differently. But since multiple interfaces in a rule doesn't
>+	 *	work anyway a simple straightforward compare is performed
>+	 *	here. Ultimately frentry_t creation will need to be
>+	 *	revisited in ipf_y.y. While the other issue, recognition
>+	 *	of only the first interface in a list of interfaces will
>+	 *	need to be separately addressed along with why only four.
>+	 */
>+	for (i = 0; i < FR_NUM(fr1->fr_ifnames); i++) {
>+		/*
>+		 * XXX:	It's either the same index or uninitialized.
>+		 * 	We assume this because multiple interfaces
>+		 *	referenced by the same rule doesn't work anyway.
>+		 */
>+		if (IPF_IFNAMES_DIFFERENT(fr_ifnames[i]))
>+			return(5);
>+	}
>+
>+	if (IPF_FRDEST_DIFFERENT(fr_tif))
>+		return (6);
>+	if (IPF_FRDEST_DIFFERENT(fr_rif))
>+		return (7);
>+	if (IPF_FRDEST_DIFFERENT(fr_dif))
>+		return (8);
> 	if (!fr1->fr_data && !fr2->fr_data)
> 		return (0);	/* move along, nothing to see here */
> 	if (fr1->fr_data && fr2->fr_data) {
> 		if (bcmp(fr1->fr_caddr, fr2->fr_caddr, fr1->fr_dsize) == 0)
> 			return (0);	/* same */
> 	}
>-	return (5);
>+	return (9);
> }
> 
> 
>diff --git a/sys/contrib/ipfilter/netinet/ip_fil.h b/sys/contrib/ipfilter/netinet/ip_fil.h
>index f4ffa53391c..81906af7c2e 100644
>--- a/sys/contrib/ipfilter/netinet/ip_fil.h
>+++ b/sys/contrib/ipfilter/netinet/ip_fil.h
>@@ -735,12 +735,9 @@ typedef	struct	frentry {
> 	u_char	fr_icode;	/* return ICMP code */
> 	int	fr_group;	/* group to which this rule belongs */
> 	int	fr_grhead;	/* group # which this rule starts */
>-	int	fr_ifnames[4];
> 	int	fr_isctag;
> 	int	fr_rpc;		/* XID Filtering */ 
> 	ipftag_t fr_nattag;
>-	frdest_t fr_tifs[2];	/* "to"/"reply-to" interface */
>-	frdest_t fr_dif;	/* duplicate packet interface */
> 	/*
> 	 * These are all options related to stateful filtering
> 	 */
>@@ -749,6 +746,12 @@ typedef	struct	frentry {
> 	int	fr_statemax;	/* max reference count */
> 	int	fr_icmphead;	/* ICMP group  for state options */
> 	u_int	fr_age[2];	/* non-TCP state timeouts */
>+	/*
>+	 * These are compared separately.
>+	 */
>+	int	fr_ifnames[4];
>+	frdest_t fr_tifs[2];	/* "to"/"reply-to" interface */
>+	frdest_t fr_dif;	/* duplicate packet interface */
> 	/*
> 	 * How big is the name buffer at the end?
> 	 */
>@@ -827,9 +830,10 @@ typedef	struct	frentry {
> 
> #define	FR_NOLOGTAG	0
> 
>-#define	FR_CMPSIZ(_f)	((_f)->fr_size - \
>-			 offsetof(struct frentry, fr_func))
>+#define	FR_CMPSIZ	(offsetof(struct frentry, fr_ifnames) - \
>+			offsetof(struct frentry, fr_func))
> #define	FR_NAME(_f, _n)	(_f)->fr_names + (_f)->_n
>+#define FR_NUM(_a)	(sizeof(_a) / sizeof(*_a))
> 
> 
> /*

Actions: View | Diff

Attachments on bug 238796: 205322 | 205341 | 205744 | 205808 | 205851 | 206344 | 206385