Package: vde2
Version: 2.3.2-4.2
Severity: important
Tags: upstream patch
Hello,
We're using the vde2-switch as network switch for several qemu-processes.
Lately we
noticed a lot of segfaults from the vde2-switch process. We were able to
trigger the
segfaults more often when we generate a lot of random mac-addresses on the
network,
combined with a lot of traffic.
After diving into the code, I noticed that the hash_gc() method is called from
the
SIGALRM signal handler, which could happen at the same time as a find_in_hash()
or
find_in_hash_update() lookup. The hash_gc() can then invalidate a pointer which
the
find_in_hash() or find_in_hash_update() call is still using, which causes a
segfault.
By simply delaying the hash_gc() to the next find_in_hash() or
find_in_hash_update()
call, it is no longer possible to have invalid pointers. The suggested patch
does this
by setting the new 'delayed_hash_gc' flag.
ps. Afaics, it is now also safe to remove all qtime_csenter()/qtime_csexit()
calls
in hash.c, but I'll leave that to the author of vde2 to verify that.
Regards,
Bas van Sisseren
-- System Information:
Debian Release: jessie/sid
APT prefers squeeze-lts
APT policy: (500, 'squeeze-lts'), (500, 'unstable'), (500, 'testing'), (500,
'stable'), (500, 'oldstable')
Architecture: i386 (x86_64)
Foreign Architectures: amd64
Kernel: Linux 3.16-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/bash
Versions of packages vde2 depends on:
ii adduser 3.113+nmu3
ii libc6 2.19-11
ii libpcap0.8 1.6.2-1
ii libvde0 2.3.2-4.2
ii libvdeplug2 2.3.2-4.2
vde2 recommends no packages.
Versions of packages vde2 suggests:
ii qemu 2.1+dfsg-5
pn qemu-kvm <none>
pn vde2-cryptcab <none>
-- no debconf information
diff -ru vde2-2.3.2.orig/src/vde_switch/hash.c vde2-2.3.2/src/vde_switch/hash.c
--- vde2-2.3.2.orig/src/vde_switch/hash.c 2011-11-23 17:41:17.000000000 +0100
+++ vde2-2.3.2/src/vde_switch/hash.c 2014-10-07 13:51:35.396596610 +0200
@@ -49,8 +49,11 @@
u_int64_t dst;
};
+static int delayed_hash_gc;
static struct hash_entry **h;
+static void hash_gc(void *arg); // forward declaration
+
static int calc_hash(u_int64_t src)
{
register int x = src * 0x030507090b0d1113LL;
@@ -89,6 +92,7 @@
* port */
int find_in_hash(unsigned char *dst,int vlan)
{
+ if (delayed_hash_gc) hash_gc(NULL);
struct hash_entry *e = find_entry(extmac(dst,vlan));
if(e == NULL) return -1;
return(e->port);
@@ -102,6 +106,7 @@
int k = calc_hash(esrc);
int oldport;
time_t now;
+ if (delayed_hash_gc) hash_gc(NULL);
for(e = h[k]; e && e->dst != esrc; e = e->next)
;
if(e == NULL) {
@@ -267,9 +272,17 @@
static void hash_gc(void *arg)
{
time_t t = qtime();
+ delayed_hash_gc = 0;
for_all_hash(&gc, &t);
}
+/* actual handler which is called every GC_INTERVAL seconds. only set a flag
+ to stay "thread-safe" */
+static void hash_gc_flag(void *arg)
+{
+ delayed_hash_gc++;
+}
+
#define HASH_INIT(BIT) \
({ hash_bits=(BIT);\
hash_mask=HASH_SIZE-1;\
@@ -310,7 +323,7 @@
{
qtimer_del(gc_timerno);
gc_interval=p;
- gc_timerno=qtimer_add(gc_interval,0,hash_gc,NULL);
+ gc_timerno=qtimer_add(gc_interval,0,hash_gc_flag,NULL);
return 0;
}
@@ -410,7 +423,7 @@
gc_interval=GC_INTERVAL;
gc_expire=GC_EXPIRE;
- gc_timerno=qtimer_add(gc_interval,0,hash_gc,NULL);
+ gc_timerno=qtimer_add(gc_interval,0,hash_gc_flag,NULL);
ADDCL(cl);
#ifdef DEBUGOPT
ADDDBGCL(dl);
