potential bug in tcp_hostcache.c
Luigi Rizzo
rizzo at icir.org
Mon Mar 29 13:07:16 PST 2004
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
More information about the freebsd-current
mailing list