Race with Flow->use_cnt access causes Flow leaks

Developers
1

Hi there,
Since creating ticket isn’t likely getting much attention, I’m posting here to see if devs can pick it up. This below bug causes large memory leaks especially with app layer protocol (such as smb) with state that tends hold large temporary memory chunks, such as files.
The flow->use_cnt as documented below is supposed to be accessed with the flow locked (the documentation has a typo too).

    /** how many references exist to this flow *right now*
     *
     *  On receiving a packet the counter is incremented while the flow
     *  bucked is locked, which is also the case on timeout pruning.
     */
    FlowRefCount use_cnt;

However, at least in the following case, the count could be decreased without a lock and would easily race with the increase case from another thread. The call stack shows the code flow.
Basically, when a packet belongs to a flow that’s bypassed, then the flow-worker.c unlocks the flow after the FlowUpdate below. Once the FlowWorker returns, the packet will be outputted by TmqhOutputPacketpool and will then be de-referenced by calling PACKET_RELEASE_REFS, all without the flow locked.

        FlowHandlePacket(tv, &fw->fls, p);
        if (likely(p->flow != NULL)) {
            DEBUG_ASSERT_FLOW_LOCKED(p->flow);
            if (FlowUpdate(tv, fw, p) == TM_ECODE_DONE) {
                FLOWLOCK_UNLOCK(p->flow);
                return TM_ECODE_OK;
            }
        }

TmThreadsSlotProcessPkt
 ->TmThreadsSlotVarRun
   ->FlowWorker
     ->if (p->flags & PKT_WANTS_FLOW)
      ->FlowHandlePacket
       ->FlowGetFlowFromHash
        ->FlowReference
         ->FlowIncrUsecnt
      ->if (likely(p->flow != NULL))
       ->if (FlowUpdate(tv, fw, p) == TM_ECODE_DONE)
        ->FLOWLOCK_UNLOCK(p->flow);
         ->return TM_ECODE_OK;
   ->tv->tmqh_out(tv, p)
    ->TmqhOutputPacketpool
     ->PACKET_RELEASE_REFS(p)

Here is the redmine ticket, Bug #4766: Flow leaked when flow->use_cnt access race happens - Suricata - Open Information Security Foundation

2

Hi,
the redmine ticket is just 6days old, the same as your forum post. Also the ticket got updates from Victor shortly after and a PR. So not sure what you would expect from us.