[Oisf-devel] Suricata 2.0dev + PF_RING 5.6.0 sporadic crashes in HTPCallbackRequest

[Victor Julien]

On 07/22/2013 07:28 PM, Chris Wakelin wrote:
> On 22/07/13 18:01, Chris Wakelin wrote:
>> On 22/07/13 17:54, Victor Julien wrote:
>>>>>>> Still crashing sporadically I'm afraid, but now it's mostly in
>>>>>>> htp_validate_hostname. I've attached another backtrace - does the frame
>>>>>>> in ReceivePfringLoop make any sense?
>>>>>>
>>>>>> I agree the pfring data looks weird. Might be uninitialized, maybe
>>>>>> pfring doesn't init it if it doesn't use it. Despite this value, suri
>>>>>> figured out it's TCP.
>>>>>>
>>>>>
>>>>> I did as Ivan suggested and upgraded to latest libhtp git (I was running
>>>>> master from Friday 19th July) and latest Suricata too (likewise)
>>>>>
>>>>> Here's a backtrace for another crash we've been seeing a bit less
>>>>> frequently but has just re-occurred (with the latest git releases).
>>>>>
>>>>> Again the ReceivePfringLoop looks a bit suspect to me :)
>>>>
>>>> I've pinged Luca about it, will let you know.
>>>>
>>>
>>> I was told:
>>>
>>> "in order to optimise the computation the pfring header does not contain
>>> parsing info by default:
>>> - with standard (kernel-based) drivers you should add
>>> PF_RING_LONG_HEADER to the pfring_open() flags.
>>> - with dna we parse the packet in 1-copy mode only (passing a buffer
>>> with a buffer-len > 0 to pfring_recv()) for the same reason.
>>> You can explicitly call the parsing routine calling pfring_parse_pkt()"
>>>
>>> So the seemingly random/weird data is the result of us not using
>>> PF_RING_LONG_HEADER and pfring_parse_pkt(). So it's not an indication of
>>> something bad.
>>>
>>> Cheers,
>>> Victor
>>>
>>
>> I was wanting to find the traffic that caused suricata to segfault.
>> Perhaps I should add the "PF_RING_LONG_HEADER" flag to my debug version
>> (the one compiled with CFLAGS="-ggdb -O0"), though I suspect it
>> decreases performance slightly.
>>
>> I guess we should also have told him I'm using PF_RING with DNA and
>> libzero (though if you told him it was me, he probably already knows :-) )
>>
>> Best Wishes,
>> Chris
>>
> 
> Actually, we already have PF_RING_LONG_HEADER in the flags in
> src/source-pfring.c - and I should have read his reply more carefully;
> it seems it doesn't apply to DNA anyway. I'll look at adding an explicit
> pfring_parse_pkt() call when I work out what parameters it needs and
> where to put it!

Ah, good point. I missed us using LONG_HEADER :) So for the dna case we
should call pfring_parse_pkt() maybe. I guess if we somehow need the
LONG_HEADER, so we'd need the parse_pkt then too.

-- 
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------