Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 40312: /trunk/ /trunk/epan/dissectors/: packet-bt-dht.c

["Maynard, Chris" <Christopher.Maynard@xxxxxxxxx>]

There are 2 other files that I believe also have tvb_get_string()-related memory leaks, but they are somewhat confusing to me, and so I would appreciate it if someone could take a look at them.

1) packet-ldap.c:dissect_ldap_Mechanism(): The comment starting at line 1160 states the following:
        /*
         * If the mechanism in this request is an empty string (which is
         * returned as a null pointer), use the saved mechanism instead.
         * Otherwise, if the saved mechanism is an empty string (null),
         * save this mechanism.
         */

But that doesn't look like what's going on.  Instead, it looks like if the mechanism is NULL then it is assigned to the saved mechanism ... which is never used, so what's the point?  And if it's NOT NULL then the saved mechanism is freed and assigned to the new mechanism regardless of whether the saved mechanism was NULL or not.  Since the code doesn't match the comment, I'm confused.  And I'm also not entirely sure if there is a possible tvb_get_string() memory leak here or not.

2) packet-wsp.c: At line 1281, there is a nice NOTE warning about ensuring to call g_free() since tvb_get_stringz() returns g_malloc()ed memory.  There is actually only 1 instance of tvb_get_stringz(); however, it is called from the get_text_string() macro.  get_text_string(), in turn, is called by a bunch of other macros, such as get_token_text(), get_extension_media(), get_text_value(), get_quoted_string(), get_uri_value() and get_version_value().  Because all of those therefore end up calling tvb_get_stringz(), I believe we need to see some g_free()'s after all of them.

This doesn't look like the case, but before I went to try to fix them, I noticed this confusing code/comment at line 2243:
            get_token_text(val_str, tvb, off, len, ok); \
            /* As we're using val_str, it is automatically g_free()d */ \

There is no g_free() following that code, so I think there is a leak here and that the comment can't be true ... can it?  Am I missing something here?  I stopped looking for leaks at this point for this dissector until I can clear up this confusion.

Thanks.
- Chris

> -----Original Message-----
> From: wireshark-commits-bounces@xxxxxxxxxxxxx [mailto:wireshark-
> commits-bounces@xxxxxxxxxxxxx] On Behalf Of cmaynard@xxxxxxxxxxxxx
> Sent: Wednesday, December 28, 2011 11:37 AM
> To: wireshark-commits@xxxxxxxxxxxxx
> Subject: [Wireshark-commits] rev 40312: /trunk/
> /trunk/epan/dissectors/: packet-bt-dht.c packet-gopher.c packet-
> gsm_ipa.c packet-meta.c packet-mux27010.c packet-nfs.c packet-rdp.c
> packet-sametime.c packet-ua.c packet-xtp.c ...
> 
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=40312
> 
> User: cmaynard
> Date: 2011/12/28 08:36 AM
> 
> Log:
>  Fix memory leaks involving tvb_get_string[z]().
> 
> Directory: /trunk/epan/dissectors/
>   Changes    Path                 Action
>   +5 -5      packet-bt-dht.c      Modified
>   +1 -0      packet-gopher.c      Modified
>   +1 -1      packet-gsm_ipa.c     Modified
>   +3 -0      packet-meta.c        Modified
>   +7 -5      packet-mux27010.c    Modified
>   +3 -0      packet-nfs.c         Modified
>   +6 -6      packet-rdp.c         Modified
>   +1 -1      packet-sametime.c    Modified
>   +4 -4      packet-ua.c          Modified
>   +1 -0      packet-xtp.c         Modified
> 
> 
> (1 file not shown)
> _______________________________________________________________________
> ____
> Sent via:    Wireshark-commits mailing list <wireshark-
> commits@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-commits
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
>              mailto:wireshark-commits-
> request@xxxxxxxxxxxxx?subject=unsubscribe

-- 

CONFIDENTIALITY NOTICE: The information contained in this email message is intended only for use of the intended recipient. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately delete it from your system and notify the sender by replying to this email.  Thank you.