[Bro] ssl binpac analyzer -- patches

[Tobias Kiesling]

On 5/29/07, jmzhou.ml at gmail.com <jmzhou.ml at gmail.com> wrote:
>
> Some comments about the patch set
>
> Patch 4: I can confirm that this is a bug. I had the same problem and
> same fix at my side.
>
> Patch 5: I think the new call (DoGenParseCode) in pac_type.cc should be
> replaced by GenParseCode3. Imagine that a record field (identifier f1)
> is empty type and there are other fields (identifier f2, f3, ...) after
> field f1. If you call DoGenParseCode, the identifier f1 will not be
> evaluated like in GenParseCode3 (pac_type.cc:754-755). This will result
> in re-entrance of RecordDataField::GenParseCode (pac_record.cc:420)
> because
> when f2->prev()-GenParseCode (pac_record.cc:428) is called,
> env->Evaluated(
> id()) will return false. This is an infinite loop - till at some point of
> binpac there will be an assertion failure.


I think I can see your point and also that it might generally be more safe
to call GenParseCode3 instead (I attached the updated patch file).


> Patch 6: I think there is another issue here: if some buffering request
> has been sent to the flow buffer, and the flow buffer is not ready with
> partial data, we will end up with entering the loop - a waste of CPU
> cycles.


Maybe you are right, but what would you suggest as change? Do you want to
check whether the buffer is ready before entering the loop? But then it has
to be ensured that the buffer is properly initialized in any case.  At the
moment I cannot see all the consequences of such a change. And do you think
that the impact on performance is really relevant?

Thanks for the comments,
Tobias

_______________________________________________
> Bro mailing list
> bro at bro-ids.org
> http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/bro
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.ICSI.Berkeley.EDU/pipermail/bro/attachments/20070529/02bd855a/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: binpac-5.patch
Type: application/octet-stream
Size: 2166 bytes
Desc: not available
Url : http://mailman.ICSI.Berkeley.EDU/pipermail/bro/attachments/20070529/02bd855a/attachment.obj