[Bro-Dev] newly found segfaults from metrics framework
jsiwek at ncsa.illinois.edu
Tue Aug 16 15:47:33 PDT 2011
> Have you guys tried valgrind? That can often pinpoint memory
> corruoption right at the source. It can be slow, but prefiltering the
> trace for SSL might help (if that's what's triggering it).
Yeah, valgrind is pointing out invalid accesses in CompHash.cc, reproducible for me by running the metrics/ssl-example.bro on the 2009-M57-day11-21.trace from bro-testing filtered for "tcp port 443".
Looked like it tried to stuff a string value someplace inside the CompositeHash::key buffer when it hasn't been sized to hold that much:
==27445== Invalid write of size 4
==27445== at 0x4027C3D: memcpy (mc_replace_strmem.c:635)
==27445== by 0x82229C5: CompositeHash::SingleValHash(int, char*, BroType*, Val*, bool) const (CompHash.cc:211)
I didn't think it could be sized right in the CompositeHash ctor since it's only got Types and not Vals to work with at that point, so I changed the computation to allocate and use a buffer on the fly if the pre-allocated one from the ctor isn't large enough to hold the key computed from the index vals.
$ git diff
diff --git a/src/CompHash.cc b/src/CompHash.cc
index 605949b..c3c65e3 100644
@@ -241,16 +241,13 @@ HashKey* CompositeHash::ComputeHash(const Val* v, int type
char* k = key;
+ int sz = ComputeKeySize(v, type_check);
+ if ( sz == 0 )
+ return 0;
+ type_check = 0; // no need to type-check again.
- if ( ! k )
- int sz = ComputeKeySize(v, type_check);
- if ( sz == 0 )
- return 0;
+ if ( ! k || sz != size )
k = reinterpret_cast<char*>(new double[sz/sizeof(double) + 1]);
- type_check = 0; // no need to type-check again.
const type_list* tl = type->Types();
That made the metrics framework happy for me, but does that seem like a reasonable approach?
More information about the bro-dev