Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 21 May 2011 23:26:36 +0100 (BST)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        Luigi Rizzo <luigi@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r215179 - in head: sbin/ipfw sys/netinet sys/netinet/ipfw
Message-ID:  <alpine.BSF.2.00.1105212323450.35370@fledge.watson.org>
In-Reply-To: <201011121305.oACD5Htm009484@svn.freebsd.org>
References:  <201011121305.oACD5Htm009484@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1105212323450.35370>