Subject: Re: kern/28875: 2.0: ipf crash in fr_coalesce
To: None <hubert.feyrer@informatik.fh-regensburg.de,>
From: Christos Zoulas <christos@zoulas.com>
List: netbsd-bugs
Date: 01/05/2005 13:38:30
On Jan 5,  6:09pm, hubert@feyrer.de (Hubert Feyrer) wrote:
-- Subject: kern/28875: 2.0: ipf crash in fr_coalesce

| >Number:         28875
| >Category:       kern
| >Synopsis:       2.0: ipf crash in fr_coalesce
| >Confidential:   no
| >Severity:       critical
| >Priority:       high
| >Responsible:    kern-bug-people
| >State:          open
| >Class:          sw-bug
| >Submitter-Id:   net
| >Arrival-Date:   Wed Jan 05 18:09:00 +0000 2005
| >Originator:     Hubert Feyrer
| >Release:        NetBSD 2.0_STABLE
| >Organization:
| bla!
| >Environment:

This bug has been fixed on head, please apply the following fix
which goes from:

1.61.2.9 -> 1.7 [different directory]

It will fix not only your pointer crashes but the keep state rules as
a bonus :-) Please request this to be pulled up to the 2.0 branch.

christos

--- fil.c-2.0	2004-12-24 14:49:14.000000000 -0500
+++ fil.c	2004-12-05 21:59:23.000000000 -0500
@@ -419,7 +419,7 @@
 			 * Actually, hop by hop header is only allowed right
 			 * after IPv6 header!
 			 */
-			if (coalesced == 0) {
+			if ((fin->fin_m != NULL) && (coalesced == 0)) {
 				coalesced = fr_coalesce(fin);
 				if (coalesced == -1)
 					return;
@@ -430,7 +430,7 @@
 				frpr_hopopts6(fin);
 			break;
 		case IPPROTO_DSTOPTS :
-			if (coalesced == 0) {
+			if ((fin->fin_m != NULL) && (coalesced == 0)) {
 				coalesced = fr_coalesce(fin);
 				if (coalesced == -1)
 					return;
@@ -438,7 +438,7 @@
 			frpr_dstopts6(fin);
 			break;
 		case IPPROTO_ROUTING :
-			if (coalesced == 0) {
+			if ((fin->fin_m != NULL) && (coalesced == 0)) {
 				coalesced = fr_coalesce(fin);
 				if (coalesced == -1)
 					return;
@@ -460,7 +460,7 @@
 			go = 0;
 			break;
 		case IPPROTO_FRAGMENT :
-			if (coalesced == 0) {
+			if ((fin->fin_m != NULL) && (coalesced == 0)) {
 				coalesced = fr_coalesce(fin);
 				if (coalesced == -1)
 					return;
@@ -2021,10 +2021,19 @@
 			ATOMIC_INCL(frstats[out].fr_ads);
 		} else {
 			ATOMIC_INCL(frstats[out].fr_bads);
+#ifdef notdef
+			/*
+			 * This blocks ICMP ECHOREPLY. fr_addstate returning
+			 * NULL is not necessary a bad thing because there
+			 * is no state to be added on some packets, eg.
+			 * icmp reply packets. XXX: but for others this
+			 * is wrong.
+			 */
 			if (FR_ISPASS(pass)) {
 				pass &= ~FR_CMDMASK;
 				pass |= FR_BLOCK;
 			}
+#endif
 		}
 	}
 
@@ -2756,7 +2765,11 @@
 	m->m_off += hlen;
 #   endif
 	m->m_len -= hlen;
+#ifdef INET
 	sum2 = in_cksum(m, slen);
+#else
+	sum2 = 0;
+#endif
 	m->m_len += hlen;
 #   if BSD >= 199103
 	m->m_data -= hlen;
@@ -3967,13 +3980,12 @@
 	frentry_t frd, *fp, *f, **fprev, **ftail;
 	int error = 0, in, v;
 	u_int *p, *pp;
-	frgroup_t *fg;
+	frgroup_t *fg = NULL;
 	char *group;
-	void *ptr;
+	void *ptr = NULL;
 
-	fg = NULL;
-	fp = &frd;
 	if (makecopy != 0) {
+		fp = &frd;
 		error = fr_inobj(data, fp, IPFOBJ_FRENTRY);
 		if (error)
 			return EFAULT;
@@ -4020,7 +4032,6 @@
 			return error;
 	}
 
-	ptr = NULL;
 	/*
 	 * Check that the group number does exist and that its use (in/out)
 	 * matches what the rule is.