Skip to content

Commit

Permalink
click/lexer: fix double free when aliasess are created for elementcla…
Browse files Browse the repository at this point in the history
…sses

In the Lexer, when an elementclass is encountered, the parser saves state
and begins parsing the inner configuration.  The ParserState is pushed onto
a stack and a Compound is attached as a thunk.  When you exit the
elementclass, the ParserState is popped and the Compound is destroyed.

However, when an elementclass is aliased, a synonym references the Compound
by pointer.  When the synonym is destroyed, it also deletes the thunk; this
causes a double free EVERY time the Lexer encounters an aliased
elementclass.

In kclick, this manifests as a corrupted SLUB freelist, which often results
in a memory leak, an infinite loop in kmalloc, or a panic.

This change adds a refcount to Compound and performs get/put operations,
deleting it when there are zero refs.  Prior to this change, AFL was able to
generate a config that would double free in remove_element_type.  The config
no longer double frees after this change.  I used valgrind to verify that no
memory leaks were caused as a result of refcounting.

Originally I modified add_element_type to get a ref; however, only one call
site actually needs to do this, so code to put the extra ref needed to be
added after all the other call sites.  Therefore, I just added the get
before that specific call site.

I did not add refs when Compounds are chained & it doesn't seem to affect
this particular test.

Thanks, American Fuzzy Lop

Change-Id: Idf20188d14f392aa5ab5f4bd1ad48763eb3a7929
  • Loading branch information
Derrick Pallas committed Sep 28, 2018
1 parent a88048d commit 6ec2530
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions lib/lexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class Lexer::TunnelEnd {
class Lexer::Compound : public Element { public:

Compound(const String &, const String &, VariableEnvironment *parent);
~Compound() { assert(!_refcount); }

const String &name() const { return _name; }
const char *printable_name_c_str();
Expand Down Expand Up @@ -163,6 +164,9 @@ class Lexer::Compound : public Element { public:
String signature() const;
static String signature(const String &name, const Vector<String> *formal_types, int nargs, int ninputs, int noutputs);

Compound & get() { ++_refcount; return *this; }
bool put() { return 0 == --_refcount; }

private:

String _name;
Expand All @@ -184,6 +188,8 @@ class Lexer::Compound : public Element { public:
Vector<int> _element_nports[2];
int _anonymous_offset;

unsigned _refcount;

Vector<Router::Connection> _conn;

friend class Lexer;
Expand All @@ -194,7 +200,7 @@ Lexer::Compound::Compound(const String &name, const String &lm, VariableEnvironm
: _name(name), _landmark(lm), _overload_type(-1),
_scope(parent),
_nformals(0), _ninputs(0), _noutputs(0), _scope_order_error(false),
_element_map(-1), _anonymous_offset(0)
_element_map(-1), _anonymous_offset(0), _refcount(1)
{
}

Expand Down Expand Up @@ -539,7 +545,7 @@ Lexer::~Lexer()
for (int t = 0; t < _element_types.size(); t++)
if (_element_types[t].factory == compound_element_factory) {
Lexer::Compound *compound = (Lexer::Compound *) _element_types[t].thunk;
delete compound;
if (compound && compound->put()) delete compound;
}
}

Expand Down Expand Up @@ -571,7 +577,7 @@ Lexer::end_parse(int cookie)
}
_tunnels.clear();

delete _c;
if (_c && _c->put()) delete _c;
_c = 0;
delete _ps;
_ps = 0;
Expand Down Expand Up @@ -949,6 +955,7 @@ Lexer::add_element_type(const String &name, ElementFactory factory, uintptr_t th
tid = _free_element_type;
_free_element_type = _element_types[tid].next;
}

_element_types[tid].factory = factory;
_element_types[tid].thunk = thunk;
#ifdef CLICK_LINUXMODULE
Expand Down Expand Up @@ -1027,7 +1034,7 @@ Lexer::remove_element_type(int removed, int *prev_hint)
// remove stuff
if (_element_types[removed].factory == compound_element_factory) {
Lexer::Compound *compound = (Lexer::Compound *) _element_types[removed].thunk;
delete compound;
if (compound && compound->put()) delete compound;
}
_element_types[removed].factory = 0;
_element_types[removed].name = String();
Expand Down Expand Up @@ -1645,6 +1652,8 @@ Lexer::yelementclass()
} else if (tnext.is(lexIdent)) {
// define synonym type
int t = force_element_type(tnext.string());
if (_element_types[t].factory == compound_element_factory && _element_types[t].thunk)
((Lexer::Compound *)_element_types[t].thunk)->get();
ADD_ELEMENT_TYPE(name, _element_types[t].factory, _element_types[t].thunk, true);

} else {
Expand Down

0 comments on commit 6ec2530

Please sign in to comment.