Changeset 5794

Timestamp:

May 18, 2018 7:12:41 AM (6 years ago)

Author:

nanang

Message:

Re #2096:

• Fixed memory leak due to not destroyed DTLS-SRTP keying (when it is not active).
• DTLS-SRTP must not destroy itself, keying destroy must be initiated by SRTP.

Location:

pjproject/trunk/pjmedia/src/pjmedia

Files:

• transport_srtp.c (modified) (3 diffs)
• transport_srtp_dtls.c (modified) (4 diffs)

pjproject/trunk/pjmedia/src/pjmedia/transport_srtp.c

@@ -1270 +1270 @@
     PJ_ASSERT_RETURN(tp, PJ_EINVAL);
 
-    /* Close keying */
-    for (i=0; i < srtp->keying_cnt; i++)
-        pjmedia_transport_close(srtp->keying[i]);
+    /* Close all keying. Note that any keying should not be destroyed before
+     * SRTP transport is destroyed as re-INVITE may initiate new keying method
+     * without destroying SRTP transport.
+     */
+    for (i=0; i < srtp->all_keying_cnt; i++)
+        pjmedia_transport_close(srtp->all_keying[i]);
 
     /* Close member if configured */
@@ -1612 +1615 @@
             !srtp_crypto_empty(&srtp->rx_policy_neg))
         {
-            /* SRTP nego is done, let's destroy any other keying. */
-            unsigned j;
-            for (j = 0; j < srtp->keying_cnt; ++j) {
-                if (j != i)
-                    pjmedia_transport_close(srtp->keying[j]);
-            }
+            /* SRTP nego is done */
             srtp->keying_cnt = 1;
             srtp->keying[0] = srtp->keying[i];
@@ -1683 +1681 @@
             !srtp_crypto_empty(&srtp->rx_policy_neg))
         {
-            /* SRTP nego is done, let's destroy any other keying. */
-            unsigned j;
-            for (j = 0; j < srtp->keying_cnt; ++j) {
-                if (j != i)
-                    pjmedia_transport_close(srtp->keying[j]);
-            }
+            /* SRTP nego is done */
             srtp->keying_cnt = 1;
             srtp->keying[0] = srtp->keying[i];

pjproject/trunk/pjmedia/src/pjmedia/transport_srtp_dtls.c

@@ -1062 +1062 @@
         switch (ds->srtp->setting.use) {
             case PJMEDIA_SRTP_DISABLED:
-                if (attr_setup)
-                    return PJMEDIA_SRTP_ESDPINTRANSPORT;
+                if (attr_setup) {
+                    status = PJMEDIA_SRTP_ESDPINTRANSPORT;
+                    goto on_return;
+                }
                 break;
             case PJMEDIA_SRTP_OPTIONAL:
                 break;
             case PJMEDIA_SRTP_MANDATORY:
-                if (!attr_setup)
-                    return PJMEDIA_SRTP_ESDPINTRANSPORT;
+                if (!attr_setup) {
+                    status = PJMEDIA_SRTP_ESDPINTRANSPORT;
+                    goto on_return;
+                }
                 break;
         }
@@ -1078 +1082 @@
 
 on_return:
+#if DTLS_DEBUG
     if (status != PJ_SUCCESS) {
         pj_perror(4, ds->base.name, status, "dtls_media_create() failed");
-        dtls_destroy(tp);
-    }
+    }
+#endif
     return status;
 }
@@ -1256 +1261 @@
 
 on_return:
+#if DTLS_DEBUG
     if (status != PJ_SUCCESS) {
         pj_perror(4, ds->base.name, status, "dtls_encode_sdp() failed");
-        dtls_destroy(tp);
-    }
+    }
+#endif
     return status;
 }
@@ -1393 +1399 @@
 
 on_return:
+#if DTLS_DEBUG
     if (status != PJ_SUCCESS) {
         pj_perror(4, ds->base.name, status, "dtls_media_start() failed");
-        dtls_destroy(tp);
-    }
+    }
+#endif
     return status;
 }