Date: Mon, 29 Mar 2004 23:20:58 +0200 From: Andre Oppermann <andre@freebsd.org> To: Luigi Rizzo <rizzo@icir.org> Cc: current@freebsd.org Subject: Re: potential bug in tcp_hostcache.c Message-ID: <4068933A.5090302@freebsd.org> In-Reply-To: <20040329130716.A26409@xorpc.icir.org> References: <20040329130716.A26409@xorpc.icir.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4068933A.5090302>