[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #8 from Gerrit Code Review --- Change 30362 merged by Pascal Quantin: RTP.ED-137: Memleak fixed https://code.wireshark.org/review/30362 -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 Pascal Quantin changed: What|Removed |Added Status|CONFIRMED |RESOLVED Resolution|--- |FIXED CC||pascal.quan...@gmail.com -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #7 from Gerrit Code Review --- Change 30362 had a related patch set uploaded by Jirka Novak: RTP.ED-137: Memleak fixed https://code.wireshark.org/review/30362 -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #6 from Peter Wu --- (In reply to Jiri Novak from comment #5) > I prepared patch based on your idea, but it doesn't work. I mean it creates > segfault, because code is: > > tmp = rel_time_to_secs_str(NULL, _time); > > proto_tree_add_uint_format_value( tree, time_item, tvb, hdrext_offset, > 3, time_value, "%s s", tmp); > > wmem_free(NULL, tmp); > > In other words, I allocate tmp_time, but free it once it is formated. If I > allocate it from packed (or any other) wmem, it is double freed and it fails. The idea of using wmem scopes is that it'll be freed when the scope is destroyed. So you can omit wmem_free completely when using wmem_packet_scope(). > Back to the original request - you observed memory leak, but I'm afraid it > is not caused by this piece of code. Do you have more information from where > it can come? It happens in the else branch (which did not have a wmem_free(NULL, tmp) call). While that could simply be added, I thought about going one step further and merge both branches (and use wmem_packet_scope()) since they are equivalent. -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #5 from Jiri Novak --- Hi Peter, > So are both times effectively relative? yes, they are. I prepared patch based on your idea, but it doesn't work. I mean it creates segfault, because code is: tmp = rel_time_to_secs_str(NULL, _time); proto_tree_add_uint_format_value( tree, time_item, tvb, hdrext_offset, 3, time_value, "%s s", tmp); wmem_free(NULL, tmp); In other words, I allocate tmp_time, but free it once it is formated. If I allocate it from packed (or any other) wmem, it is double freed and it fails. Back to the original request - you observed memory leak, but I'm afraid it is not caused by this piece of code. Do you have more information from where it can come? Best regards, Jirka Novak -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #4 from Peter Wu --- > Even one branch is "ntp based", it doesn't mean time is ntp/absolute. It > just says where send got time reference. Time in packet is just counter > derived from ntp, but it don't contain all ntp related items. So are both times effectively relative? > I have sample for both cases. I can attach it to the case. Yes please :-) > Who should commit change - you or me? It would be great if you could push a patch for review, thanks for looking into it! -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 --- Comment #3 from Jiri Novak --- Hi Peter, yes, this fix will work. You are right that both branches are same. I created the first one without sample data and then added content of second one, but didn't cleaned the code. Even one branch is "ntp based", it doesn't mean time is ntp/absolute. It just says where send got time reference. Time in packet is just counter derived from ntp, but it don't contain all ntp related items. I have sample for both cases. I can attach it to the case. Who should commit change - you or me? Best regards, Jirka Novak -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
[Wireshark-bugs] [Bug 15231] [oss-fuzz] ASAN: 22 bytes leaked in epan/dissectors/packet-rtp-ed137.c:688
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15231 Peter Wu changed: What|Removed |Added Summary|[oss-fuzz] ASAN: 22 byte(s) |[oss-fuzz] ASAN: 22 bytes |leaked in 1 allocation(s). |leaked in ||epan/dissectors/packet-rtp- ||ed137.c:688 CC||j.no...@netsystem.cz --- Comment #2 from Peter Wu --- Hi Jiri, the memleak is rather trivial and could be solved by replacing: tmp = rel_time_to_secs_str(NULL, _time); by tmp = rel_time_to_secs_str(wmem_packet_scope(), _time); However I think that the process_time_value function can be further cleaned up. Both branches are equivalent (except for the comment). If that is intentional, then one of them should be removed. Though if the NTP value is absolute, shouldn't it be using abs_time_to_str instead? (That will not display a time in seconds, but in a more human-readable form). Do you have a packet capture file that contains both forms? Any suggestion on the correct form? -- You are receiving this mail because: You are watching all bug changes.___ Sent via:Wireshark-bugs mailing list Archives:https://www.wireshark.org/lists/wireshark-bugs Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe
