[Asterisk-code-review] pjproject bundled: Add patch for pj atomic crashes (asterisk[master])

George Joseph asteriskteam at digium.com
Mon Apr 2 08:19:06 CDT 2018


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/8666 )

Change subject: pjproject_bundled:  Add patch for pj_atomic crashes
......................................................................

pjproject_bundled:  Add patch for pj_atomic crashes

There have been some crashes in the past where something attempts
to use a pj_atomic after it's already been destroyed.  This patch
tries to prevent it by making sure that pj_atomic_destroy sets
its mutex to NULL when it's done.  The pj_mutex functions already check
for a NULL mutex and just return PJ_EINVAL.

Teluu also added some checks to the win32 implementation as well.

Change-Id: Id25f70b79fdedf44ead6e6e1763a4417d3b3f825
---
A third-party/pjproject/patches/0070-os_core_unix-Set-mutex-NULL-in-atomic-destroy-and-ad.patch
1 file changed, 114 insertions(+), 0 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/third-party/pjproject/patches/0070-os_core_unix-Set-mutex-NULL-in-atomic-destroy-and-ad.patch b/third-party/pjproject/patches/0070-os_core_unix-Set-mutex-NULL-in-atomic-destroy-and-ad.patch
new file mode 100644
index 0000000..3aafd69
--- /dev/null
+++ b/third-party/pjproject/patches/0070-os_core_unix-Set-mutex-NULL-in-atomic-destroy-and-ad.patch
@@ -0,0 +1,114 @@
+From 67485f3a6c711f67a40ff46288cb6be1658023bd Mon Sep 17 00:00:00 2001
+From: nanang <nanang at localhost>
+Date: Mon, 26 Mar 2018 10:33:50 +0000
+Subject: [PATCH] Close #2101:  - set atomic's mutex to NULL in atomic destroy 
+ - added few sanity checks to the atomic functions.
+
+---
+ pjlib/src/pj/os_core_unix.c  | 20 ++++++++++++++++++--
+ pjlib/src/pj/os_core_win32.c |  4 ++++
+ 2 files changed, 22 insertions(+), 2 deletions(-)
+
+diff --git a/pjlib/src/pj/os_core_unix.c b/pjlib/src/pj/os_core_unix.c
+index ebfe84348..c17ad4ef0 100644
+--- a/pjlib/src/pj/os_core_unix.c
++++ b/pjlib/src/pj/os_core_unix.c
+@@ -879,9 +879,16 @@ PJ_DEF(pj_status_t) pj_atomic_create( pj_pool_t *pool,
+  */
+ PJ_DEF(pj_status_t) pj_atomic_destroy( pj_atomic_t *atomic_var )
+ {
++    pj_status_t status;
++
+     PJ_ASSERT_RETURN(atomic_var, PJ_EINVAL);
++    
+ #if PJ_HAS_THREADS
+-    return pj_mutex_destroy( atomic_var->mutex );
++    status = pj_mutex_destroy( atomic_var->mutex );
++    if (status == PJ_SUCCESS) {
++        atomic_var->mutex = NULL;
++    }
++    return status;
+ #else
+     return 0;
+ #endif
+@@ -892,10 +899,16 @@ PJ_DEF(pj_status_t) pj_atomic_destroy( pj_atomic_t *atomic_var )
+  */
+ PJ_DEF(void) pj_atomic_set(pj_atomic_t *atomic_var, pj_atomic_value_t value)
+ {
++    pj_status_t status;
++
+     PJ_CHECK_STACK();
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+ 
+ #if PJ_HAS_THREADS
+-    pj_mutex_lock( atomic_var->mutex );
++    status = pj_mutex_lock( atomic_var->mutex );
++    if (status != PJ_SUCCESS) {
++        return;
++    }
+ #endif
+     atomic_var->value = value;
+ #if PJ_HAS_THREADS
+@@ -946,6 +959,7 @@ PJ_DEF(pj_atomic_value_t) pj_atomic_inc_and_get(pj_atomic_t *atomic_var)
+  */
+ PJ_DEF(void) pj_atomic_inc(pj_atomic_t *atomic_var)
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+     pj_atomic_inc_and_get(atomic_var);
+ }
+ 
+@@ -974,6 +988,7 @@ PJ_DEF(pj_atomic_value_t) pj_atomic_dec_and_get(pj_atomic_t *atomic_var)
+  */
+ PJ_DEF(void) pj_atomic_dec(pj_atomic_t *atomic_var)
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+     pj_atomic_dec_and_get(atomic_var);
+ }
+ 
+@@ -1005,6 +1020,7 @@ PJ_DEF(pj_atomic_value_t) pj_atomic_add_and_get( pj_atomic_t *atomic_var,
+ PJ_DEF(void) pj_atomic_add( pj_atomic_t *atomic_var,
+                             pj_atomic_value_t value )
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+     pj_atomic_add_and_get(atomic_var, value);
+ }
+ 
+diff --git a/pjlib/src/pj/os_core_win32.c b/pjlib/src/pj/os_core_win32.c
+index 1cb6004d3..8c934b34d 100644
+--- a/pjlib/src/pj/os_core_win32.c
++++ b/pjlib/src/pj/os_core_win32.c
+@@ -750,6 +750,7 @@ PJ_DEF(pj_status_t) pj_atomic_destroy( pj_atomic_t *var )
+ PJ_DEF(void) pj_atomic_set( pj_atomic_t *atomic_var, pj_atomic_value_t value)
+ {
+     PJ_CHECK_STACK();
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+ 
+     InterlockedExchange(&atomic_var->value, value);
+ }
+@@ -784,6 +785,7 @@ PJ_DEF(pj_atomic_value_t) pj_atomic_inc_and_get(pj_atomic_t *atomic_var)
+  */
+ PJ_DEF(void) pj_atomic_inc(pj_atomic_t *atomic_var)
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+     pj_atomic_inc_and_get(atomic_var);
+ }
+ 
+@@ -806,6 +808,7 @@ PJ_DEF(pj_atomic_value_t) pj_atomic_dec_and_get(pj_atomic_t *atomic_var)
+  */
+ PJ_DEF(void) pj_atomic_dec(pj_atomic_t *atomic_var)
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+     pj_atomic_dec_and_get(atomic_var);
+ }
+ 
+@@ -815,6 +818,7 @@ PJ_DEF(void) pj_atomic_dec(pj_atomic_t *atomic_var)
+ PJ_DEF(void) pj_atomic_add( pj_atomic_t *atomic_var,
+ 			    pj_atomic_value_t value )
+ {
++    PJ_ASSERT_ON_FAIL(atomic_var, return);
+ #if defined(PJ_WIN32_WINNT) && PJ_WIN32_WINNT >= 0x0400
+     InterlockedExchangeAdd( &atomic_var->value, value );
+ #else
+-- 
+2.14.3
+

-- 
To view, visit https://gerrit.asterisk.org/8666
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id25f70b79fdedf44ead6e6e1763a4417d3b3f825
Gerrit-Change-Number: 8666
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180402/3f3b4797/attachment-0001.html>


More information about the asterisk-code-review mailing list