Fix file descriptor leak in RTP code.

SIP requests that offered codecs incompatible with configured values
could result in the allocation of RTP and RTCP ports that would not get
reclaimed later.

ASTERISK-24666 #close
Reported by Y Ateya

Review: https://reviewboard.asterisk.org/r/4323

AST-2015-001

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@431300 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 5049327e518de92c079fcbc589292a123d74fb73..dd0015dc672fe1ecbaf98271e4474d663c7131e8 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1192,6 +1192,7 @@ static void stream_destroy(struct ast_sip_session_media *session_media)
                ast_rtp_instance_stop(session_media->rtp);
                ast_rtp_instance_destroy(session_media->rtp);
        }
+       session_media->rtp = NULL;
 }
 
 /*! \brief SDP handler for 'audio' media stream */

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 7178732a9dc51d9cfd1cdbc114beaa99053c93a7..b900f15d009fd5a672f473ee174a8471aa00fe04 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -186,6 +186,26 @@ void ast_sip_session_unregister_sdp_handler(struct ast_sip_session_sdp_handler *
        ao2_callback_data(sdp_handlers, OBJ_KEY | OBJ_UNLINK | OBJ_NODATA, remove_handler, (void *)stream_type, handler);
 }
 
+/*!
+ * \brief Set an SDP stream handler for a corresponding session media.
+ *
+ * \note Always use this function to set the SDP handler for a session media.
+ *
+ * This function will properly free resources on the SDP handler currently being
+ * used by the session media, then set the session media to use the new SDP
+ * handler.
+ */
+static void session_media_set_handler(struct ast_sip_session_media *session_media,
+               struct ast_sip_session_sdp_handler *handler)
+{
+       ast_assert(session_media->handler != handler);
+
+       if (session_media->handler) {
+               session_media->handler->stream_destroy(session_media);
+       }
+       session_media->handler = handler;
+}
+
 static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sdp_session *sdp)
 {
        int i;
@@ -235,6 +255,9 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd
                        continue;
                }
                AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       if (handler == session_media->handler) {
+                               continue;
+                       }
                        ast_debug(1, "Negotiating incoming SDP media stream '%s' using %s SDP handler\n",
                                session_media->stream_type,
                                handler->id);
@@ -249,7 +272,7 @@ static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sd
                                        session_media->stream_type,
                                        handler->id);
                                /* Handled by this handler. Move to the next stream */
-                               session_media->handler = handler;
+                               session_media_set_handler(session_media, handler);
                                handled = 1;
                                break;
                        }
@@ -317,6 +340,9 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
                        continue;
                }
                AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       if (handler == session_media->handler) {
+                               continue;
+                       }
                        ast_debug(1, "Applying negotiated SDP media stream '%s' using %s SDP handler\n",
                                session_media->stream_type,
                                handler->id);
@@ -331,7 +357,7 @@ static int handle_negotiated_sdp_session_media(void *obj, void *arg, int flags)
                                        session_media->stream_type,
                                        handler->id);
                                /* Handled by this handler. Move to the next stream */
-                               session_media->handler = handler;
+                               session_media_set_handler(session_media, handler);
                                return CMP_MATCH;
                        }
                }
@@ -751,6 +777,9 @@ static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_
                        continue;
                }
                AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       if (handler == session_media->handler) {
+                               continue;
+                       }
                        if (!handler->defer_incoming_sdp_stream) {
                                continue;
                        }
@@ -760,15 +789,15 @@ static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_
                        case AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED:
                                continue;
                        case AST_SIP_SESSION_SDP_DEFER_ERROR:
-                               session_media->handler = handler;
+                               session_media_set_handler(session_media, handler);
                                return 0;
                        case AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED:
                                /* Handled by this handler. */
-                               session_media->handler = handler;
+                               session_media_set_handler(session_media, handler);
                                break;
                        case AST_SIP_SESSION_SDP_DEFER_NEEDED:
                                /* Handled by this handler. */
-                               session_media->handler = handler;
+                               session_media_set_handler(session_media, handler);
                                return 1;
                        }
                        /* Move to the next stream */
@@ -929,9 +958,21 @@ static int datastore_cmp(void *obj, void *arg, int flags)
 static void session_media_dtor(void *obj)
 {
        struct ast_sip_session_media *session_media = obj;
-       if (session_media->handler) {
-               session_media->handler->stream_destroy(session_media);
+       struct sdp_handler_list *handler_list;
+       /* It is possible for SDP handlers to allocate memory on a session_media but
+        * not end up getting set as the handler for this session_media. This traversal
+        * ensures that all memory allocated by SDP handlers on the session_media is
+        * cleared (as well as file descriptors, etc.).
+        */
+       handler_list = ao2_find(sdp_handlers, session_media->stream_type, OBJ_KEY);
+       if (handler_list) {
+               struct ast_sip_session_sdp_handler *handler;
+
+               AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+                       handler->stream_destroy(session_media);
+               }
        }
+       ao2_cleanup(handler_list);
        if (session_media->srtp) {
                ast_sdp_srtp_destroy(session_media->srtp);
        }
@@ -2051,6 +2092,9 @@ static int add_sdp_streams(void *obj, void *arg, void *data, int flags)
 
        /* no handler for this stream type and we have a list to search */
        AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
+               if (handler == session_media->handler) {
+                       continue;
+               }
                res = handler->create_outgoing_sdp_stream(session, session_media, answer);
                if (res < 0) {
                        /* catastrophic error */
@@ -2058,7 +2102,7 @@ static int add_sdp_streams(void *obj, void *arg, void *data, int flags)
                }
                if (res > 0) {
                        /* Handled by this handler. Move to the next stream */
-                       session_media->handler = handler;
+                       session_media_set_handler(session_media, handler);
                        return CMP_MATCH;
                }
        }

diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c
index b63a607e31be932895c6298fcba4891615481112..10cd3d0174b40025cfee94dddf1a35a0a9e10580 100644 (file)
--- a/res/res_pjsip_t38.c
+++ b/res/res_pjsip_t38.c
@@ -816,6 +816,7 @@ static void stream_destroy(struct ast_sip_session_media *session_media)
        if (session_media->udptl) {
                ast_udptl_destroy(session_media->udptl);
        }
+       session_media->udptl = NULL;
 }
 
 /*! \brief SDP handler for 'image' media stream */