[ https://bro-tracker.atlassian.net/browse/BIT-1297?page=com.atlassian.jira.p… ]
Daniel Thayer commented on BIT-1297:
------------------------------------
I've added a set of tests in branch topic/dnthayer/ticket1297 in the trace-summary repo.
> trace-summary needs tests
> -------------------------
>
> Key: BIT-1297
> URL: https://bro-tracker.atlassian.net/browse/BIT-1297
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: trace-summary
> Reporter: Daniel Thayer
> Fix For: 2.4
>
>
> There are no tests in the trace-summary repo.
--
This message was sent by Atlassian JIRA
(v6.4-OD-12-026#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Robin Sommer edited comment on BIT-1298 at 12/12/14 4:53 PM:
-------------------------------------------------------------
{quote}
I was going for the "copy-and-swap" idiom (or I think also called "unified assignment" for C++11 since it takes the place of both copy and move ctor) --
{quote}
Ah, thanks for the explanation. Expect more C++ old-timer questions in the future. :)
was (Author: jsiwek):
{quote}
I would be more comfortable if the copy operation were a method one has to call explicitly, rather a copy constructor that's easy to have run implicitly. What do you think?
{quote}
Yeah, I think should be ok.
{quote}
Nit: You lost me on the swap() operation: why's the copy constructor doing a swap on the fields rather than just an assignment?
{quote}
I was going for the "copy-and-swap" idiom (or I think also called "unified assignment" for C++11 since it takes the place of both copy and move ctor) --
the copy assignment operator takes a parameter by value which implies the copy ctor in this case and then swaps the current object with that copy and so completing the assignment, then the temporary goes out of scope and cleans up whatever existed before the assignment. At least that was the intention, maybe there was something more specific you're trying to point out that I'm not seeing wrong with it (but I guess this all goes away doing the explicit copy method).
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Robin Sommer
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Robin Sommer updated BIT-1298:
------------------------------
Resolution: Merged (was: Fixed)
Status: Closed (was: Merge Request)
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Robin Sommer
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Jon Siwek commented on BIT-1298:
--------------------------------
Changed to the explicit copy method; same branch.
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Robin Sommer
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Jon Siwek reassigned BIT-1298:
------------------------------
Assignee: Robin Sommer (was: Jon Siwek)
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Robin Sommer
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Jon Siwek commented on BIT-1298:
--------------------------------
{quote}
I would be more comfortable if the copy operation were a method one has to call explicitly, rather a copy constructor that's easy to have run implicitly. What do you think?
{quote}
Yeah, I think should be ok.
{quote}
Nit: You lost me on the swap() operation: why's the copy constructor doing a swap on the fields rather than just an assignment?
{quote}
I was going for the "copy-and-swap" idiom (or I think also called "unified assignment" for C++11 since it takes the place of both copy and move ctor) --
the copy assignment operator takes a parameter by value which implies the copy ctor in this case and then swaps the current object with that copy and so completing the assignment, then the temporary goes out of scope and cleans up whatever existed before the assignment. At least that was the intention, maybe there was something more specific you're trying to point out that I'm not seeing wrong with it (but I guess this all goes away doing the explicit copy method).
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Jon Siwek
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Jon Siwek reassigned BIT-1298:
------------------------------
Assignee: Jon Siwek (was: Robin Sommer)
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Jon Siwek
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1299?page=com.atlassian.jira.p… ]
Robin Sommer updated BIT-1299:
------------------------------
Resolution: Merged (was: Fixed)
Status: Closed (was: Merge Request)
> topic/jsiwek/fix-ipv6
> ---------------------
>
> Key: BIT-1299
> URL: https://bro-tracker.atlassian.net/browse/BIT-1299
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: pysubnettree
> Reporter: Jon Siwek
> Assignee: Robin Sommer
> Fix For: 2.4
>
>
> This fixes problems with mixing both IPv4 and IPv6 addresses in the same trie/subnettree by converting IPv4 to the IPv4-mapped-IPv6 format before using it. (Bro's use of the underlying patricia trie code already does things like this, but the python module didn't).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
[ https://bro-tracker.atlassian.net/browse/BIT-1298?page=com.atlassian.jira.p… ]
Robin Sommer commented on BIT-1298:
-----------------------------------
- I think the original reason for not buffering the header was a performance concern: this code can execute a lot. However, measuring execution time on the test suite with this branch, I don't see a noticeable increase, so that looks fine.
- the copy ctor for the IP header worries me a bit: the constraint that it must not be truncated seems easy to miss. One way around that would be having the header store the capture length of the packet as well, so that it knows how much data is valid. On the other hand, not sure that effort/memory is justified. Alternatively, I would be more comfortable if the copy operation were a method one has to call explicitly, rather a copy constructor that's easy to have run implicitly. What do you think?
- Nit: You lost me on the swap() operation: why's the copy constructor doing a swap on the fields rather than just an assignment?
> IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
> -------------------------------------------------------------------------------------------------------
>
> Key: BIT-1298
> URL: https://bro-tracker.atlassian.net/browse/BIT-1298
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: 2.3
> Environment: Debian
> Reporter: Eric Asselin
> Assignee: Robin Sommer
> Priority: High
> Labels: analyzer
> Attachments: core.zip, ntp-synchronized.pcap
>
>
> From a child analyzer like NTP, the IP_Hdr pointer in the DeliverPacket method is empty and unusable causing a segmentation fault as soon as you try to access it.
> To recreate the bug, just add an "assert(ip)" inside the DeliverPacket method of a UDP child analyzer and the execution will fail (instead of the segmentation fault).
--
This message was sent by Atlassian JIRA
(v6.4-OD-11-014#64007)
Open Merge Requests
===================
ID Component Reporter Assignee Updated For Version Priority Summary
------------ ------------ ------------ ------------ ---------- ------------- ---------- -------------------------------------------------------------------------------------------------------
BIT-1299 [1] pysubnettree Jon Siwek Robin Sommer 2014-12-11 2.4 Normal topic/jsiwek/fix-ipv6 [2]
BIT-1298 [3] Bro Eric Asselin Robin Sommer 2014-12-10 - High IP_Hdr pointer do not propagate from udp to child analyzers via DeliverPacket method causing a segfault
[1] BIT-1299 https://bro-tracker.atlassian.net/browse/BIT-1299
[2] fix-ipv6 https://github.com/bro/pysubnettree/tree/topic/jsiwek/fix-ipv6
[3] BIT-1298 https://bro-tracker.atlassian.net/browse/BIT-1298