Project

General

Profile

Actions

Bug #5368

closed

bypass: Memory leak of some flow bypass objects.

Added by Lukas Sismis over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

During my bypass testing, I've come across this memory leak.
I had set up Suricata with address sanitizers and an XDP bypass. I've used only two rules, which essentially should bypass all encountered traffic:

alert udp any any -> any any (msg: "UDP Packet!"; flow: to_server; bypass;  sid: 999; rev:1;)
alert tcp any any -> any any (msg: "TCP Packet!"; flow: to_server; bypass;  sid: 1000; rev:2;)

After transmitting some traffic against Suricata (to be very specific, it was 1M packets where Suricata detected 44724 flows) and then exiting, Asan has complained with following:

=================================================================                                                                                                             [52/6538]
==13985==ERROR: LeakSanitizer: detected memory leaks                                                                                                                                  

Direct leak of 233576 byte(s) in 4171 object(s) allocated from:                                                                                                                       
    #0 0x7f072436eb77 in calloc (/lib64/libasan.so.6+0xb4b77)                                                                                                                         
    #1 0x979196 in SCCallocFunc /home/local/xsismi01/suricata/build/src/util-mem.c:57                                                                                                 
    #2 0x9cbdae in PacketBypassCallback /home/local/xsismi01/suricata/build/src/decode.c:433                                                                                          
    #3 0xb75e08 in DetectBypassMatch /home/local/xsismi01/suricata/build/src/detect-bypass.c:96                                                                                       
    #4 0xb7d699 in DetectRunPostMatch /home/local/xsismi01/suricata/build/src/detect.c:177                                                                                            
    #5 0xb7f18c in DetectRulePacketRules /home/local/xsismi01/suricata/build/src/detect.c:805                                                                                         
    #6 0xb7d474 in DetectRun /home/local/xsismi01/suricata/build/src/detect.c:136
    #7 0xb81c43 in DetectFlow /home/local/xsismi01/suricata/build/src/detect.c:1683
    #8 0xb81ef5 in Detect /home/local/xsismi01/suricata/build/src/detect.c:1755
    #9 0xa8473c in FlowWorker /home/local/xsismi01/suricata/build/src/flow-worker.c:552
    #10 0x95979c in TmThreadsSlotVarRun /home/local/xsismi01/suricata/build/src/tm-threads.c:131
    #11 0xac3d85 in TmThreadsSlotProcessPkt /home/local/xsismi01/suricata/build/src/tm-threads.h:195
    #12 0xac5962 in AFPParsePacketV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:983
    #13 0xac5a69 in AFPWalkBlock /home/local/xsismi01/suricata/build/src/source-af-packet.c:996
    #14 0xac5cbe in AFPReadFromRingV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:1043
    #15 0xac6b8b in ReceiveAFPLoop /home/local/xsismi01/suricata/build/src/source-af-packet.c:1393
    #16 0x95a098 in TmThreadsSlotPktAcqLoop /home/local/xsismi01/suricata/build/src/tm-threads.c:345
    #17 0x7f0722e90189 in start_thread (/lib64/libpthread.so.0+0x8189)

Indirect leak of 72 byte(s) in 3 object(s) allocated from:
    #0 0x7f072436eb77 in calloc (/lib64/libasan.so.6+0xb4b77)
    #1 0x979196 in SCCallocFunc /home/local/xsismi01/suricata/build/src/util-mem.c:57
    #2 0xac8ca6 in AFPSetFlowStorage /home/local/xsismi01/suricata/build/src/source-af-packet.c:2134
    #3 0xac9960 in AFPXDPBypassCallback /home/local/xsismi01/suricata/build/src/source-af-packet.c:2401
    #4 0x9cbdfe in PacketBypassCallback /home/local/xsismi01/suricata/build/src/decode.c:440
    #5 0xb75e08 in DetectBypassMatch /home/local/xsismi01/suricata/build/src/detect-bypass.c:96
    #6 0xb7d699 in DetectRunPostMatch /home/local/xsismi01/suricata/build/src/detect.c:177
    #7 0xb7f18c in DetectRulePacketRules /home/local/xsismi01/suricata/build/src/detect.c:805
    #8 0xb7d474 in DetectRun /home/local/xsismi01/suricata/build/src/detect.c:136
    #9 0xb81c43 in DetectFlow /home/local/xsismi01/suricata/build/src/detect.c:1683
    #10 0xb81ef5 in Detect /home/local/xsismi01/suricata/build/src/detect.c:1755
    #11 0xa8473c in FlowWorker /home/local/xsismi01/suricata/build/src/flow-worker.c:552
    #12 0x95979c in TmThreadsSlotVarRun /home/local/xsismi01/suricata/build/src/tm-threads.c:131
    #13 0xac3d85 in TmThreadsSlotProcessPkt /home/local/xsismi01/suricata/build/src/tm-threads.h:195                                                                          [15/6538]
    #14 0xac5962 in AFPParsePacketV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:983
    #15 0xac5a69 in AFPWalkBlock /home/local/xsismi01/suricata/build/src/source-af-packet.c:996
    #16 0xac5cbe in AFPReadFromRingV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:1043
    #17 0xac6b8b in ReceiveAFPLoop /home/local/xsismi01/suricata/build/src/source-af-packet.c:1393
    #18 0x95a098 in TmThreadsSlotPktAcqLoop /home/local/xsismi01/suricata/build/src/tm-threads.c:345
    #19 0x7f0722e90189 in start_thread (/lib64/libpthread.so.0+0x8189)

Indirect leak of 48 byte(s) in 3 object(s) allocated from:
    #0 0x7f072436eb77 in calloc (/lib64/libasan.so.6+0xb4b77)
    #1 0x979196 in SCCallocFunc /home/local/xsismi01/suricata/build/src/util-mem.c:57
    #2 0xac966d in AFPXDPBypassCallback /home/local/xsismi01/suricata/build/src/source-af-packet.c:2351
    #3 0x9cbdfe in PacketBypassCallback /home/local/xsismi01/suricata/build/src/decode.c:440
    #4 0xb75e08 in DetectBypassMatch /home/local/xsismi01/suricata/build/src/detect-bypass.c:96
    #5 0xb7d699 in DetectRunPostMatch /home/local/xsismi01/suricata/build/src/detect.c:177
    #6 0xb7f18c in DetectRulePacketRules /home/local/xsismi01/suricata/build/src/detect.c:805
    #7 0xb7d474 in DetectRun /home/local/xsismi01/suricata/build/src/detect.c:136
    #8 0xb81c43 in DetectFlow /home/local/xsismi01/suricata/build/src/detect.c:1683
    #9 0xb81ef5 in Detect /home/local/xsismi01/suricata/build/src/detect.c:1755
    #10 0xa8473c in FlowWorker /home/local/xsismi01/suricata/build/src/flow-worker.c:552
    #11 0x95979c in TmThreadsSlotVarRun /home/local/xsismi01/suricata/build/src/tm-threads.c:131
    #12 0xac3d85 in TmThreadsSlotProcessPkt /home/local/xsismi01/suricata/build/src/tm-threads.h:195
    #13 0xac5962 in AFPParsePacketV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:983
    #14 0xac5a69 in AFPWalkBlock /home/local/xsismi01/suricata/build/src/source-af-packet.c:996
    #15 0xac5cbe in AFPReadFromRingV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:1043
    #16 0xac6b8b in ReceiveAFPLoop /home/local/xsismi01/suricata/build/src/source-af-packet.c:1393
    #17 0x95a098 in TmThreadsSlotPktAcqLoop /home/local/xsismi01/suricata/build/src/tm-threads.c:345
    #18 0x7f0722e90189 in start_thread (/lib64/libpthread.so.0+0x8189)

Indirect leak of 48 byte(s) in 3 object(s) allocated from:
    #0 0x7f072436eb77 in calloc (/lib64/libasan.so.6+0xb4b77)
    #1 0x979196 in SCCallocFunc /home/local/xsismi01/suricata/build/src/util-mem.c:57
    #2 0xac97d6 in AFPXDPBypassCallback /home/local/xsismi01/suricata/build/src/source-af-packet.c:2379
    #3 0x9cbdfe in PacketBypassCallback /home/local/xsismi01/suricata/build/src/decode.c:440
    #4 0xb75e08 in DetectBypassMatch /home/local/xsismi01/suricata/build/src/detect-bypass.c:96
    #5 0xb7d699 in DetectRunPostMatch /home/local/xsismi01/suricata/build/src/detect.c:177
    #6 0xb7f18c in DetectRulePacketRules /home/local/xsismi01/suricata/build/src/detect.c:805
    #7 0xb7d474 in DetectRun /home/local/xsismi01/suricata/build/src/detect.c:136
    #8 0xb81c43 in DetectFlow /home/local/xsismi01/suricata/build/src/detect.c:1683
    #9 0xb81ef5 in Detect /home/local/xsismi01/suricata/build/src/detect.c:1755
    #10 0xa8473c in FlowWorker /home/local/xsismi01/suricata/build/src/flow-worker.c:552
    #11 0x95979c in TmThreadsSlotVarRun /home/local/xsismi01/suricata/build/src/tm-threads.c:131
    #12 0xac3d85 in TmThreadsSlotProcessPkt /home/local/xsismi01/suricata/build/src/tm-threads.h:195
    #13 0xac5962 in AFPParsePacketV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:983
    #14 0xac5a69 in AFPWalkBlock /home/local/xsismi01/suricata/build/src/source-af-packet.c:996
    #15 0xac5cbe in AFPReadFromRingV3 /home/local/xsismi01/suricata/build/src/source-af-packet.c:1043
    #16 0xac6b8b in ReceiveAFPLoop /home/local/xsismi01/suricata/build/src/source-af-packet.c:1393
    #17 0x95a098 in TmThreadsSlotPktAcqLoop /home/local/xsismi01/suricata/build/src/tm-threads.c:345
    #18 0x7f0722e90189 in start_thread (/lib64/libpthread.so.0+0x8189)

SUMMARY: AddressSanitizer: 233744 byte(s) leaked in 4180 allocation(s).

I've tried to add the following snippet to `FlowQueuePrivateAppendFlow()` as I thought this function is used in (all) flow deletions.
This however didn't help to solve the issue.

    if (f->flow_state == FLOW_STATE_CAPTURE_BYPASSED) {
        FlowBypassInfo *fc = FlowGetStorageById(f, GetFlowBypassInfoID());
        if (fc && fc->BypassFree && fc->bypass_data) {
            fc->BypassFree(fc->bypass_data);
            fc->bypass_data = NULL;
            fc->BypassFree = NULL;
        }
        SCFree(fc);
        FlowSetStorageById(f, GetFlowBypassInfoID(), NULL);
    }


Subtasks 1 (0 open1 closed)

Bug #5422: bypass: Memory leak of some flow bypass objects. (6.0.x backport)ClosedVictor JulienActions
Actions #1

Updated by Lukas Sismis over 2 years ago

Probably a better function to use the snippet would be MoveToWorkQueue() but that did not help either.

I am using the following snippet because as per Asan output, decode.c on line 433 allocates bypass object but Asan thinks the object is not freed.

Actions #2

Updated by Lukas Sismis over 2 years ago

PR submitted.

Actions #3

Updated by Victor Julien over 2 years ago

  • Status changed from New to Resolved
  • Assignee changed from OISF Dev to Lukas Sismis
  • Target version changed from TBD to 7.0.0-beta1
  • Label Needs backport to 6.0 added
Actions #4

Updated by Victor Julien over 2 years ago

  • Status changed from Resolved to Closed
Actions #5

Updated by Victor Julien over 2 years ago

  • Label deleted (Needs backport to 6.0)
Actions

Also available in: Atom PDF