>Number:         172675
>Category:       misc
>Synopsis:       sysctl_tcp_hc_list (net.inet.tcp.hostcache.list) race 
>condition causing memory corruption
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Oct 13 22:00:00 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Jason Wolfe
>Release:        8.3-RELEASE-p4
>Organization:
Limelight Networks
>Environment:
FreeBSD xxx 8.3-RELEASE-p4 FreeBSD 8.3-RELEASE-p4 #0: Mon Aug 20 18:41:21 MST 
2012     jason@xxx:/usr/obj/usr/src/sys/SIXFOUR  amd64
>Description:
It appears that in sysctl_tcp_hc_list, the function used for 'sysctl 
net.inet.tcp.hostcache.list', there is a somewhat rare chance of cache_count 
being lower than the actual number of entries.  Not sure if this is happening 
by a race condition after execution time, or if the value just isn't updated 
constantly enough.  On heavily loaded boxes (1000+ more or less unique req/s) 
it's not too tough to cause memory corruption and a panic running 'sysctl 
net.inet.tcp.hostcache.list' with the current code.

A colleague of mine had spotted the issue, and believes this patch would do the 
trick.  I've been testing it by simply running the hostcache.list in a loop, 
and have had success where prior it would have caused a panic in fairly short 
order.

Patch by: Nils McCarthy <n...@shkoo.com>
>How-To-Repeat:
Run sysctl net.inet.tcp.hostcache.list continuously.
>Fix:
Possible patch supplied

Patch attached with submission follows:

--- src.stock/sys/netinet/tcp_hostcache.c       2012-03-02 23:15:13.000000000 
-0700
+++ src/sys/netinet/tcp_hostcache.c     2012-10-12 19:54:55.000000000 -0700
@@ -360,7 +360,7 @@
                }
                TAILQ_REMOVE(&hc_head->hch_bucket, hc_entry, rmx_q);
                V_tcp_hostcache.hashbase[hash].hch_length--;
-               V_tcp_hostcache.cache_count--;
+               atomic_subtract_int(&V_tcp_hostcache.cache_count, 1);
                TCPSTAT_INC(tcps_hc_bucketoverflow);
 #if 0
                uma_zfree(V_tcp_hostcache.zone, hc_entry);
@@ -392,7 +392,7 @@
         */
        TAILQ_INSERT_HEAD(&hc_head->hch_bucket, hc_entry, rmx_q);
        V_tcp_hostcache.hashbase[hash].hch_length++;
-       V_tcp_hostcache.cache_count++;
+       atomic_add_int(&V_tcp_hostcache.cache_count, 1);
        TCPSTAT_INC(tcps_hc_added);
 
        return hc_entry;
@@ -587,6 +587,7 @@
 static int
 sysctl_tcp_hc_list(SYSCTL_HANDLER_ARGS)
 {
+       int alloclines;
        int bufsize;
        int linesize = 128;
        char *p, *buf;
@@ -595,8 +596,10 @@
 #ifdef INET6
        char ip6buf[INET6_ADDRSTRLEN];
 #endif
-
-       bufsize = linesize * (V_tcp_hostcache.cache_count + 1);
+       
+       /* overallocate in case the host cache size changes while we're reading 
it */
+       alloclines = V_tcp_hostcache.cache_count*3/2;
+       bufsize = linesize * (alloclines + 1);
 
        p = buf = (char *)malloc(bufsize, M_TEMP, M_WAITOK|M_ZERO);
 
@@ -606,7 +609,7 @@
        p += len;
 
 #define msec(u) (((u) + 500) / 1000)
-       for (i = 0; i < V_tcp_hostcache.hashsize; i++) {
+       for (i = 0; i < V_tcp_hostcache.hashsize && alloclines > 0; i++) {
                THC_LOCK(&V_tcp_hostcache.hashbase[i].hch_mtx);
                TAILQ_FOREACH(hc_entry, &V_tcp_hostcache.hashbase[i].hch_bucket,
                              rmx_q) {
@@ -624,7 +627,7 @@
                            msec(hc_entry->rmx_rtt *
                                (RTM_RTTUNIT / (hz * TCP_RTT_SCALE))),
                            msec(hc_entry->rmx_rttvar *
-                               (RTM_RTTUNIT / (hz * TCP_RTT_SCALE))),
+                               (RTM_RTTUNIT / (hz * TCP_RTTVAR_SCALE))),
                            hc_entry->rmx_bandwidth * 8,
                            hc_entry->rmx_cwnd,
                            hc_entry->rmx_sendpipe,
@@ -633,6 +636,10 @@
                            hc_entry->rmx_updates,
                            hc_entry->rmx_expire);
                        p += len;
+                       alloclines--;
+                       if (alloclines == 0) {
+                               break;
+                       }
                }
                THC_UNLOCK(&V_tcp_hostcache.hashbase[i].hch_mtx);
        }
@@ -660,7 +667,7 @@
                                              hc_entry, rmx_q);
                                uma_zfree(V_tcp_hostcache.zone, hc_entry);
                                V_tcp_hostcache.hashbase[i].hch_length--;
-                               V_tcp_hostcache.cache_count--;
+                               
atomic_subtract_int(&V_tcp_hostcache.cache_count, 1);
                        } else
                                hc_entry->rmx_expire -= V_tcp_hostcache.prune;
                }


>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"

Reply via email to