kern/172963 (was: svn commit: r249660 - in releng/8.4/sys:netinet netinet6 (fwd))

[Robert Watson]

The following reply was made to PR kern/172963; it has been noted by GNATS.

From: Robert Watson <rwatson at FreeBSD.org>
To: bug-followup at FreeBSD.org
Cc:  
Subject: Re: kern/172963 (was: svn commit: r249660 - in releng/8.4/sys:netinet
 netinet6 (fwd))
Date: Thu, 25 Apr 2013 21:16:56 +0100 (BST)

 ---------- Forwarded message ----------
 Date: Fri, 19 Apr 2013 21:08:56 +0000 (UTC)
 From: Robert Watson <rwatson at FreeBSD.org>
 To: src-committers at freebsd.org, svn-src-all at freebsd.org,
      svn-src-releng at freebsd.org
 Subject: svn commit: r249660 - in releng/8.4/sys: netinet netinet6
 
 Author: rwatson
 Date: Fri Apr 19 21:08:56 2013
 New Revision: 249660
 URL: http://svnweb.freebsd.org/changeset/base/249660
 
 Log:
    Merge r249478 from stable/8 to releng/8.4:
 
      FreeBSD 8.0 introduced inpcb reference counting, and FreeBSD 8.1 began using
      that reference count to protect inpcb stability in udp_pcblist() and other
      monitoring functions, preventing the inpcb from being garbage collected
      across potentially sleeping copyout() operations despite the inpcb zone
      becoming shrinkable.
 
      However, this introduced a race condition in which inp->inp_socket() might
      become NULL as a result of the socket being freed, but before the inpcb we
      removed from the global list of connections, allowing it to be exposed to a
      third thread invoking udp_input() or udp6_input() which would try to
      indirect through inp_socket without testing it for NULL.  This might occur
      with particular regularity on systems that frequently run netstat, or which
      use SNMP for connection monitoring.
 
      Later FreeBSD releases use a different reference/destruction model, but
      stable/8 remained affected in FreeBSD 8.2 and 8.3; the problem could be
      spotted on very high-load UDP services, such as top-level name servers.
 
      An Errata Note for 8.x branches under continuing support might be
      appropriate.  Regardless, this fix should be merged to releng/8.4 prior to
      8.4-RELEASE.
 
      PR:           172963
      Submitted by: Vincent Miller <vmiller at verisign.com>
      Submitted by: Julien Charbon <jcharbon at verisign.com>
      Submitted by: Marc De La Gueronniere <mdelagueronniere at verisign.com>
 
    Approved by:    re (rodrigc)
 
 Modified:
    releng/8.4/sys/netinet/udp_usrreq.c
    releng/8.4/sys/netinet6/udp6_usrreq.c
 Directory Properties:
    releng/8.4/sys/   (props changed)
    releng/8.4/sys/netinet/   (props changed)
    releng/8.4/sys/netinet6/   (props changed)
 
 Modified: releng/8.4/sys/netinet/udp_usrreq.c
 ==============================================================================
 --- releng/8.4/sys/netinet/udp_usrreq.c	Fri Apr 19 21:08:21 2013	(r249659)
 +++ releng/8.4/sys/netinet/udp_usrreq.c	Fri Apr 19 21:08:56 2013	(r249660)
 @@ -495,6 +495,15 @@ udp_input(struct mbuf *m, int off)
   			INP_RLOCK(inp);
 
   			/*
 +			 * Detached PCBs can linger in the list if someone
 +			 * holds a reference. (e.g. udp_pcblist)
 +			 */
 +			if (inp->inp_socket == NULL) {
 +				INP_RUNLOCK(inp);
 +				continue;
 +			}
 +
 +			/*
   			 * Handle socket delivery policy for any-source
   			 * and source-specific multicast. [RFC3678]
   			 */
 @@ -620,6 +629,15 @@ udp_input(struct mbuf *m, int off)
   	 */
   	INP_RLOCK(inp);
   	INP_INFO_RUNLOCK(&V_udbinfo);
 +
 +	/*
 +	 * Detached PCBs can linger in the hash table if someone holds a
 +	 * reference. (e.g. udp_pcblist)
 +	 */
 +	if (inp->inp_socket == NULL) {
 +		INP_RUNLOCK(inp);
 +		goto badunlocked;
 +	}
   	if (inp->inp_ip_minttl && inp->inp_ip_minttl > ip->ip_ttl) {
   		INP_RUNLOCK(inp);
   		goto badunlocked;
 
 Modified: releng/8.4/sys/netinet6/udp6_usrreq.c
 ==============================================================================
 --- releng/8.4/sys/netinet6/udp6_usrreq.c	Fri Apr 19 21:08:21 2013	(r249659)
 +++ releng/8.4/sys/netinet6/udp6_usrreq.c	Fri Apr 19 21:08:56 2013	(r249660)
 @@ -273,6 +273,13 @@ udp6_input(struct mbuf **mp, int *offp,
   			}
 
   			/*
 +			 * Detached PCBs can linger in the list if someone
 +			 * holds a reference. (e.g. udp_pcblist)
 +			 */
 +			if (inp->inp_socket == NULL)
 +				continue;
 +
 +			/*
   			 * Handle socket delivery policy for any-source
   			 * and source-specific multicast. [RFC3678]
   			 */
 @@ -396,6 +403,15 @@ udp6_input(struct mbuf **mp, int *offp,
   	}
   	INP_RLOCK(inp);
   	INP_INFO_RUNLOCK(&V_udbinfo);
 +
 +	/*
 +	 * Detached PCBs can linger in the hash table if someone holds a
 +	 * reference. (e.g. udp_pcblist)
 +	 */
 +	if (inp->inp_socket == NULL) {
 +		INP_RUNLOCK(inp);
 +		goto badunlocked;
 +	}
   	up = intoudpcb(inp);
   	if (up->u_tun_func == NULL) {
   		udp6_append(inp, m, off, &fromsa);