diff options
author | Hadriel Kaplan <hadrielk@yahoo.com> | 2014-04-02 02:07:16 -0400 |
---|---|---|
committer | Anders Broman <a.broman58@gmail.com> | 2014-04-03 04:40:20 +0000 |
commit | 04c05a21e34cec326f1aff2f5f8a6e74e1ced984 (patch) | |
tree | 6db27e328578c12b6c1c4841a708e12eec0f4c24 /epan/dissectors/packet-rtp.h | |
parent | df80f3133cc3b128ea989ad6830511c378fa0b63 (diff) |
Fix Bug 9920 Buildbot crash due to SDP/RTP mismatch
For details see comments in Bug 9920.
The executive summary:
Bug 9920 is a crash caused by a couple of issues:
1) The memory ownership model for the rtp_dyn_payload hashtable is split: SDP
creates the rtp_dyn_payload hashtable, but RTP can free it. Since there isn't
*one* pointer to the hashtable, RTP freeing it means SDP has a dangling
pointer.
2) Either the SDP dissector shouldn't be creating two separate, unique
hashtables for multiple media channels of the same addr:port, or RTP shouldn't
be free'ing the previous one.
Change-Id: I436e67de6882f84aa82dcbdfe60bf313fe4fd99c
Reviewed-on: https://code.wireshark.org/review/918
Reviewed-by: Hadriel Kaplan <hadrielk@yahoo.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Diffstat (limited to 'epan/dissectors/packet-rtp.h')
-rw-r--r-- | epan/dissectors/packet-rtp.h | 82 |
1 files changed, 69 insertions, 13 deletions
diff --git a/epan/dissectors/packet-rtp.h b/epan/dissectors/packet-rtp.h index 28f54baf9f..f2454d4fcd 100644 --- a/epan/dissectors/packet-rtp.h +++ b/epan/dissectors/packet-rtp.h @@ -100,6 +100,72 @@ struct srtp_info #endif }; +/* an opaque object holding the hash table - use accessor functions to create/destroy/find */ +typedef struct _rtp_dyn_payload_t rtp_dyn_payload_t; + +/* RTP dynamic payload handling - use the following to create, insert, lookup, and free the + dynamic payload information. Internally, RTP creates the GHashTable with a wmem file scope + and increments the ref_count when it saves the info to conversations later. The calling + dissector (SDP, H.245, etc.) uses these functions as an interface. If the calling dissector + is done with the rtp_dyn_payload_t* for good, it should call rtp_dyn_payload_free() which + will decrement the ref_count and free's it if the ref_count is 0. In the worst case, it + will get free'd when the wmem file scope is over. + + This was changed because there were too many bugs with SDP's handling of memory ownership + of the GHashTable, with RTP freeing things SDP didn't think were free'ed. And also because + the GHashTables never got free'd in many cases by several dissectors. + */ + +/* creates a new hashtable and sets ref_count to 1, returning the newly created object */ +WS_DLL_PUBLIC +rtp_dyn_payload_t* rtp_dyn_payload_new(void); + +/* Inserts the given payload type key, for the encoding name and sample rate, into the hash table. + This makes copies of the encoding name, scoped to the life of the capture file or sooner if + rtp_dyn_payload_free is called. */ +WS_DLL_PUBLIC +void rtp_dyn_payload_insert(rtp_dyn_payload_t *rtp_dyn_payload, + const guint8 pt, + const gchar* encoding_name, + const int sample_rate); + +/* Replaces the given payload type key in the hash table, with the encoding name and sample rate. + This makes copies of the encoding name, scoped to the life of the capture file or sooner if + rtp_dyn_payload_free is called. The replaced encoding name is free'd immediately. */ +WS_DLL_PUBLIC +void rtp_dyn_payload_replace(rtp_dyn_payload_t *rtp_dyn_payload, + const guint8 pt, + const gchar* encoding_name, + const int sample_rate); + +/* removes the given payload type */ +WS_DLL_PUBLIC +gboolean rtp_dyn_payload_remove(rtp_dyn_payload_t *rtp_dyn_payload, const guint8 pt); + +/* retrieves the encoding name for the given payload type; the string returned is only valid + until the entry is replaced, removed, or the hash table is destroyed, so duplicate it if + you need it long. */ +WS_DLL_PUBLIC +const gchar* rtp_dyn_payload_get_name(rtp_dyn_payload_t *rtp_dyn_payload, const guint8 pt); + +/* retrieves the encoding name and sample rate for the given payload type, returning TRUE if + successful, else FALSE. The encoding string pointed to is only valid until the entry is + replaced, removed, or the hash table is destroyed, so duplicate it if you need it long. */ +WS_DLL_PUBLIC +gboolean rtp_dyn_payload_get_full(rtp_dyn_payload_t *rtp_dyn_payload, const guint8 pt, + const gchar **encoding_name, int *sample_rate); + +/* Free's and destroys the dyn_payload hash table; internally this decrements the ref_count + and only free's it if the ref_count == 0. */ +WS_DLL_PUBLIC +void rtp_dyn_payload_free(rtp_dyn_payload_t *rtp_dyn_payload); + + +#ifdef DEBUG_CONVERSATION +/* used for printing out debugging info, if DEBUG_CONVERSATION is defined */ +void rtp_dump_dyn_payload(rtp_dyn_payload_t *rtp_dyn_payload); +#endif + /* Info to save in RTP conversation / packet-info */ #define MAX_RTP_SETUP_METHOD_SIZE 7 struct _rtp_conversation_info @@ -107,7 +173,7 @@ struct _rtp_conversation_info gchar method[MAX_RTP_SETUP_METHOD_SIZE + 1]; guint32 frame_number; /* the frame where this conversation is started */ gboolean is_video; - GHashTable *rtp_dyn_payload; /* a hash table with the dynamic RTP payload */ + rtp_dyn_payload_t *rtp_dyn_payload; /* the dynamic RTP payload info - see comments above */ guint32 extended_seqno; /* the sequence number, extended to a 32-bit * int to guarantee it increasing monotonically @@ -121,11 +187,6 @@ struct _rtp_conversation_info btvdp_codec_info_t *btvdp_info; }; -typedef struct { - char *encoding_name; - int sample_rate; -} encoding_name_and_rate_t; - /* Add an RTP conversation with the given details */ WS_DLL_PUBLIC void rtp_add_address(packet_info *pinfo, @@ -134,7 +195,7 @@ void rtp_add_address(packet_info *pinfo, const gchar *setup_method, guint32 setup_frame_number, gboolean is_video, - GHashTable *rtp_dyn_payload); + rtp_dyn_payload_t *rtp_dyn_payload); /* Add an SRTP conversation with the given details */ WS_DLL_PUBLIC @@ -144,7 +205,7 @@ void srtp_add_address(packet_info *pinfo, const gchar *setup_method, guint32 setup_frame_number, gboolean is_video, - GHashTable *rtp_dyn_payload, + rtp_dyn_payload_t *rtp_dyn_payload, struct srtp_info *srtp_info); /* Add an Bluetooth conversation with the given details */ @@ -152,8 +213,3 @@ void bluetooth_add_address(packet_info *pinfo, address *addr, const gchar *setup_method, guint32 setup_frame_number, gboolean is_video, void *data); - -/* Free and destroy the dyn_payload hash table */ -WS_DLL_PUBLIC -void rtp_free_hash_dyn_payload(GHashTable *rtp_dyn_payload); - |