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>