Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 29 Mar 2004 13:07:16 -0800
From:      Luigi Rizzo <rizzo@icir.org>
To:        current@freebsd.org
Subject:   potential bug in tcp_hostcache.c
Message-ID:  <20040329130716.A26409@xorpc.icir.org>
next in thread | raw e-mail | index | archive | help 
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 ?

	thanks
	luigi

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040329130716.A26409>