Re: potential bug in tcp_hostcache.c

Luigi Rizzo wrote:
> Hi,
> while doing a backport to RELENG_4 of the tcp_hostcache stuff,
> a student of mine found a potential bug in tcp_hc_purge():
> 
> in sys/netinet/tcp_hostcache.c:tcp_hc_purge() near line 714
> 		
>     for (i = 0; i < tcp_hostcache.hashsize; i++) {
> 	THC_LOCK(&tcp_hostcache.hashbase[i].hch_mtx);
> 	TAILQ_FOREACH(hc_entry, &tcp_hostcache.hashbase[i].hch_bucket,
> 		      rmx_q) {
> 		if (all || hc_entry->rmx_expire <= 0) {
> 			TAILQ_REMOVE(&tcp_hostcache.hashbase[i].hch_bucket,
> 				      hc_entry, rmx_q);
> 			uma_zfree(tcp_hostcache.zone, hc_entry);
> 			tcp_hostcache.hashbase[i].hch_length--;
> 			tcp_hostcache.cache_count--;
> 		} else
> 			hc_entry->rmx_expire -= TCP_HOSTCACHE_PRUNE;
> 	}
> 	THC_UNLOCK(&tcp_hostcache.hashbase[i].hch_mtx);
> 
> the TAILQ_FOREACH will dereference hc_entry after the struct has
> been freed by uma_zfree() in the previous loop. While the code
> seems to work because uma does not clobber the record,
> the tcp_hostcache.zone is not locked around the loop so it
> might well happen that some other thread will grab and overwrite
> the record we are trying to use.
> 
> 
> The usual range of possible fixes apply e.g.
> 
> +	struct hc_metrics *tmp = NULL;
> 	...
> 	TAILQ_FOREACH(hc_entry, &tcp_hostcache.hashbase[i].hch_bucket,
> 		      rmx_q) {
> +		if (tmp)
> +			uma_zfree(tcp_hostcache.zone, tmp);
> +		tmp = NULL;
> 		if (all || hc_entry->rmx_expire <= 0) {
> 			TAILQ_REMOVE(&tcp_hostcache.hashbase[i].hch_bucket,
> 				      hc_entry, rmx_q);
> 			uma_zfree(tcp_hostcache.zone, hc_entry);
> 			tcp_hostcache.hashbase[i].hch_length--;
> 			tcp_hostcache.cache_count--;
> 		} else
> 			hc_entry->rmx_expire -= TCP_HOSTCACHE_PRUNE;
> 	}
> +	if (tmp)
> +		uma_zfree(tcp_hostcache.zone, tmp);
> 
> Anyone going to commit a fix (this or something equivalent)
> or i should do it ?

Hi Luigi,

I have not disappeared since you told me about the bug, I just had a very
busy week.  The fix is on my commit list and will go in shortly.

-- 
Andre