kohler / click

The Click modular router: fast modular packet processing and analysis
Other
735 stars 322 forks source link

click/lexer: fix double free when aliasess are created for elementcla… #412

Closed pallas closed 5 years ago

pallas commented 5 years ago

…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

kohler commented 5 years ago

Thanks so much!!! Please check out b0112ca94074738f35f83a3da0b887136a80489e.

kohler commented 5 years ago

And 6834abae9. :(