svn commit: r215179 - in head: sbin/ipfw sys/netinet sys/netinet/ipfw

[Robert Watson]

On Fri, 12 Nov 2010, Luigi Rizzo wrote:

> --- head/sys/netinet/ipfw/ip_fw2.c	Fri Nov 12 13:02:26 2010	(r215178)
> +++ head/sys/netinet/ipfw/ip_fw2.c	Fri Nov 12 13:05:17 2010	(r215179)
> @@ -1801,6 +1801,39 @@ do {								\
...
> +				/* For incomming packet, lookup up the
> +				inpcb using the src/dest ip/port tuple */
> +				if (inp == NULL) {
> +					INP_INFO_RLOCK(pi);
> +					inp = in_pcblookup_hash(pi,
> +						src_ip, htons(src_port),
> +						dst_ip, htons(dst_port),
> +						0, NULL);
> +					INP_INFO_RUNLOCK(pi);
> +				}
> +
> +				if (inp && inp->inp_socket) {
> +					tablearg = inp->inp_socket->so_user_cookie;
> +					if (tablearg)
> +						match = 1;
> +				}

This locking seems questionable -- what keeps 'inp' valid between 
INP_INFO_RUNLOCK(pi) and dereferencing 'inp' a few lines later to extract 
'tablearg'?  Normally, consumer code locks 'inp' after looking it up.

(In my new version of the code, the caller can request it be returned locked, 
or simply referenced, which changes the function signature, hence my bumping 
into this while doing a merge forward).

Robert