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 luigiReceived on Mon Mar 29 2004 - 11:07:16 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:37:49 UTC