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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to