Subject: Re: kern/37037
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Martti Kuparinen <martti.kuparinen@iki.fi>
List: netbsd-bugs
Date: 09/29/2007 20:35:02
The following reply was made to PR kern/37037; it has been noted by GNATS.

From: Martti Kuparinen <martti.kuparinen@iki.fi>
To: gnats-bugs@netbsd.org
Cc: juan@xtrarom.org, darrenr@netbsd.org
Subject: Re: kern/37037
Date: Sat, 29 Sep 2007 21:37:22 +0300 (EEST)

 After some help from Darren, I created this patch? Does it help?
 
 Darren: does it look correct? I took 4.1.27 and added our local changes
 to it (caddr_t -> void * etc.)
 
 
 Index: ip_frag.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_frag.c,v
 retrieving revision 1.7
 diff -u -r1.7 ip_frag.c
 --- ip_frag.c	16 Jun 2007 10:52:28 -0000	1.7
 +++ ip_frag.c	29 Sep 2007 18:29:12 -0000
 @@ -106,7 +106,7 @@
   __KERNEL_RCSID(0, "$NetBSD: ip_frag.c,v 1.7 2007/06/16 10:52:28 martin Exp $");
   #else
   static const char sccsid[] = "@(#)ip_frag.c	1.11 3/24/96 (C) 1993-2000 Darren Reed";
 -static const char rcsid[] = "@(#)Id: ip_frag.c,v 2.77.2.9 2007/05/27 11:13:44 darrenr Exp";
 +static const char rcsid[] = "@(#)Id: ip_frag.c,v 2.77.2.12 2007/09/20 12:51:51 darrenr Exp $";
   #endif
   #endif
 
 @@ -943,16 +943,16 @@
   	} else {
   		bzero(&zero, sizeof(zero));
   		next = &zero;
 -		token->ipt_data = (void *)-1;
 +		token->ipt_data = NULL;
   	}
   	RWLOCK_EXIT(lock);
 
   	if (frag != NULL) {
 -		WRITE_ENTER(lock);
 -		frag->ipfr_ref--;
 -		if (frag->ipfr_ref <= 0)
 -			fr_fragfree(frag);
 -		RWLOCK_EXIT(lock);
 +#ifdef USE_MUTEXES
 +		fr_fragderef(&frag, lock);
 +#else
 +		fr_fragderef(&frag);
 +#endif
   	}
 
   	error = COPYOUT(next, itp->igi_data, sizeof(*next));
 Index: ip_state.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_state.c,v
 retrieving revision 1.29
 diff -u -r1.29 ip_state.c
 --- ip_state.c	17 Sep 2007 06:56:15 -0000	1.29
 +++ ip_state.c	29 Sep 2007 18:29:15 -0000
 @@ -117,7 +117,7 @@
   __KERNEL_RCSID(0, "$NetBSD: ip_state.c,v 1.29 2007/09/17 06:56:15 martti Exp $");
   #else
   static const char sccsid[] = "@(#)ip_state.c	1.8 6/5/96 (C) 1993-2000 Darren Reed";
 -static const char rcsid[] = "@(#)Id: ip_state.c,v 2.186.2.69 2007/05/26 13:05:14 darrenr Exp";
 +static const char rcsid[] = "@(#)$Id: ip_state.c,v 2.186.2.78 2007/09/20 12:51:53 darrenr Exp $";
   #endif
   #endif
 
 @@ -657,8 +657,8 @@
   	int error;
 
   	error = fr_inobj(data, &ips, IPFOBJ_STATESAVE);
 -	if (error)
 -		return EFAULT;
 +	if (error != 0)
 +		return error;
 
   	isn = ips.ips_next;
   	if (isn == NULL) {
 @@ -687,9 +687,7 @@
   		bcopy((char *)isn->is_rule, (char *)&ips.ips_fr,
   		      sizeof(ips.ips_fr));
   	error = fr_outobj(data, &ips, IPFOBJ_STATESAVE);
 -	if (error)
 -		return EFAULT;
 -	return 0;
 +	return error;
   }
 
 
 @@ -1444,7 +1442,7 @@
   			is->is_state[!source] = IPF_TCPS_CLOSED;
   			fr_movequeue(&is->is_sti, is->is_sti.tqe_ifq,
   				     &ips_deletetq);
 -			MUTEX_ENTER(&is->is_lock);
 +			MUTEX_EXIT(&is->is_lock);
   			return 0;
   		}
   	}
 @@ -2308,8 +2306,6 @@
   	ipstate_t **isp;
   	u_int hvm;
 
 -	ASSERT(rw_read_locked(&ipf_state.ipf_lk) == 0);
 -
   	hvm = is->is_hv;
   	/*
   	 * Remove the hash from the old location...
 @@ -2897,8 +2893,6 @@
   int why;
   {
 
 -	ASSERT(rw_read_locked(&ipf_state.ipf_lk) == 0);
 -
   	/*
   	 * Since we want to delete this, remove it from the state table,
   	 * where it can be found & used, first.
 @@ -4072,7 +4066,7 @@
   	if (itp->igi_data == NULL)
   		return EFAULT;
 
 -	if (itp->igi_nitems == 0)
 +	if (itp->igi_nitems < 1)
   		return ENOSPC;
 
   	if (itp->igi_type != IPFGENITER_STATE)
 @@ -4094,33 +4088,29 @@
   		next = is->is_next;
   	}
 
 -	for (count = itp->igi_nitems; count > 0; count--) {
 +	count = itp->igi_nitems;
 +	for (;;) {
   		if (next != NULL) {
   			/*
   			 * If we find a state entry to use, bump its
   			 * reference count so that it can be used for
   			 * is_next when we come back.
   			 */
 -			MUTEX_ENTER(&next->is_lock);
 -			next->is_ref++;
 -			MUTEX_EXIT(&next->is_lock);
 -			token->ipt_data = next;
 +			if (count == 1) {
 +				MUTEX_ENTER(&next->is_lock);
 +				next->is_ref++;
 +				MUTEX_EXIT(&next->is_lock);
 +				token->ipt_data = next;
 +			}
   		} else {
   			bzero(&zero, sizeof(zero));
   			next = &zero;
 -			token->ipt_data = (void *)-1;
   			count = 1;
 +			token->ipt_data = NULL;
   		}
   		RWLOCK_EXIT(&ipf_state);
 
   		/*
 -		 * If we had a prior pointer to a state entry, release it.
 -		 */
 -		if (is != NULL) {
 -			fr_statederef(&is);
 -		}
 -
 -		/*
   		 * This should arguably be via fr_outobj() so that the state
   		 * structure can (if required) be massaged going out.
   		 */
 @@ -4131,9 +4121,14 @@
   			break;
 
   		dst += sizeof(*next);
 +		count--;
 +
   		READ_ENTER(&ipf_state);
 -		is = next;
 -		next = is->is_next;
 +		next = next->is_next;
 +	}
 +
 +	if (is != NULL) {
 +		fr_statederef(&is);
   	}
 
   	return error;