[Oisf-devel] Packet Array in 1.0.x

[Joel Ebrahimi]

I think the problem with this safety catch and the initialization is
that when Suricata starts my max_pending_packets is 0, so this will
initialize pcap_max_read_packets to 0. 

So this 0 value is then passed in as the cnt variable to pcap_distpatch.
This 0 causes pcap_distpatch to grab all the packets in the buffer.
Where the buffer can be any number and in my cases I see it vary between
12000 and 15000, but any number beyond 256 causes the application to
segfault. 

In looking at the initialization function, if I add an extra check for
pcap_max_read_packets at 0 and change it, this actually fixes the
problem and Suricata does not segfaults anymore. Basically I changed
this check:

    pcap_max_read_packets = (PCAP_FILE_MAX_PKTS < max_pending_packets) ?
        PCAP_FILE_MAX_PKTS : max_pending_packets;

to:

    pcap_max_read_packets = (PCAP_FILE_MAX_PKTS < max_pending_packets) ?
        PCAP_FILE_MAX_PKTS : max_pending_packets;
    if(pcap_max_read_packets == 0)
        pcap_max_read_packets = PCAP_FILE_MAX_PKTS;

Which corrects for the case where pcap_max_read_packets is 0. Im not
sure if this is the best way to solve for the problem or is it better to
initialize max_pending_packets to 1, which would still prevent
pcap_dispatch from grabbing more than 256 packets as well.

// Joel 




>-----Original Message-----
>From: Victor Julien [mailto:victor at inliniac.net]
>Sent: Wednesday, September 08, 2010 6:47 AM
>To: Joel Ebrahimi
>Cc: oisf-devel at openinfosecfoundation.org
>Subject: Re: [Oisf-devel] Packet Array in 1.0.x
>
>Joel Ebrahimi wrote:
>> I have looked over the code and I don't see any checks that help. Let
me
>> explain the 2 problems I see.
>>
>> First when pcap_dispatch is called I have a large number of packets
that
>> are available. So this in turn calls PcapCallback. Now something
>> PcapCallback is called 15,000 times in a row which will instantiate a
>> lot of memory with the calls to PacketGetFromQueueOrAlloc(), so in
this
>> case I exhaust the entire memory of the system creating buffers for
>> 15,000 packets. So this results in Linux killing the process.
>>
>> In the other case I do not exhaust all the memory, lets say
PcapCallback
>> is only called 12,000 times. Now in this case I return from the
callback
>> and move into the for loop. So at this point array_idx is 12,000. So
>> when I enter the for loop cnt is then incremented up to 12,000. The
>> problem here though is that the ptv->array only has 256 slots
allocated
>> for it, so since cnt exceeds 256 and there is an access to the
variable
>> ptv->array[cnt]; this will cause a segfault.
>>
>> Does that explain whats happening a little clearer? Im not sure why
no
>> one else has seen this yet, unless their buffers are small and the
>> PcapCallback routine is called less than 256 times before processing.
>
>The safety check that should catch this is on line source-pcap.c 197:
>
>        r = pcap_dispatch(ptv->pcap_handle, (pcap_max_read_packets <
>packet_q_len) ? pcap_max_read_packets : packet_q_len,
>            (pcap_handler)PcapCallback, (u_char *)ptv);
>
>pcap_max_read_packets should be equal to or lower than the array size.
>It is initialized on line 252:
>
>    /* use max_pending_packets as pcap read size unless it's bigger
than
>     * our size limit */
>    pcap_max_read_packets = (PCAP_FILE_MAX_PKTS < max_pending_packets)
?
>        PCAP_FILE_MAX_PKTS : max_pending_packets;
>
>If all things operate like they should, pcap_max_read_packets should be
>256 (the default) in your case. If max_pending_packets is set to
>something like 12000, pcap_max_read_packets should be 256 as it is
>smaller than 12000.
>
>In the call to pcap_dispatch we check this variable against the
>available packets in packet_q_len. If packet_q_len is bigger than
>pcap_max_read_packets the value passed to pcap_dispatch should be that
>of pcap_max_read_packets.
>
>In the backtrace of the segv you see, what are the values of
>pcap_max_read_packets, packet_q_len, max_pending_packets, and what is
>passed to pcap_dispatch?
>
>Cheers,
>Victor
>
>--
>---------------------------------------------
>Victor Julien
>http://www.inliniac.net/
>PGP: http://www.inliniac.net/victorjulien.asc
>---------------------------------------------