Testing

This is not OS dependant
This is not middleware dependant

This is a subtle race condition which could not be recreated directly with a MAMA application.

We were able to recreate it consistently with an enterprise application and can confirm that

The issue is resolved.

From d99fba4a10275c97f0c302149f05f5bc801107f7 Mon Sep 17 00:00:00 2001

From: Gary Molloy <gmolloy@...>

Date: Mon, 23 Jun 2014 11:18:26 +0100

Subject: [PATCH 1/7] Update to the SyncResponder to prevent free'd memory being read.

[OMAMA-271]

Description – When a sync request is received symbol names are copied retrieved from the transport list.

However this is a not a deep copy and if a subscription is removed from the list before the response is sent

The response can be corrupted.

Taking a deep copy of the symbol name(s) will prevent this.

Signed-off-by: Gary Molloy <gmolloy@...>

---

mama/c_cpp/src/c/syncresponder.c | 8 +++++++-

mama/c_cpp/src/c/transport.c     | 4 ++--

2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/mama/c_cpp/src/c/syncresponder.c b/mama/c_cpp/src/c/syncresponder.c

index 5141e7d..abec008 100644

--- a/mama/c_cpp/src/c/syncresponder.c

+++ b/mama/c_cpp/src/c/syncresponder.c

@@ -50,7 +50,7 @@ typedef struct

     mamaTimer        mTimer;

     int              mCurIndex;

     mama_u16_t       mTopicsPerMsg;

-    const char**     mTopics;

+    const char**     mTopics;                       /* Array of deep copies of the symbol names. */

     int              mTopicsLen;

     mama_i32_t*      mTypes;

     double           mResponseInterval;

@@ -163,6 +163,7 @@ setup (syncCommand* impl)

  * Invoked by CmManager once it finsishes its clean up */

void syncCommand_dtor(void *handle)

{

+    int ii = 0;

     syncCommand *impl = (syncCommand*)handle;

     mama_log (MAMA_LOG_LEVEL_FINE, "Destroying sync command.");

@@ -175,6 +176,11 @@ void syncCommand_dtor(void *handle)

         impl->mTimer = NULL;

     }

+    for (ii = 0; ii < impl->mTopicsLen; ++ii)

+    {

+        free ((void*)impl->mTopics [ii]);

+    }

+

     free ((void*)impl->mTopics);

     free (impl->mTypes);

diff --git a/mama/c_cpp/src/c/transport.c b/mama/c_cpp/src/c/transport.c

index 4743a12..339dbc0 100644

--- a/mama/c_cpp/src/c/transport.c

+++ b/mama/c_cpp/src/c/transport.c

@@ -1860,7 +1860,7 @@ struct topicsForSourceClosure

{

     int curIdx;

     const char* source;

-    const char** topics;

+    const char** topics;    /* Array of deep copies of the symbol names. */

     mama_i32_t*  types;

     /* This is index of the sub transport bridge that the subscription is

@@ -1897,7 +1897,7 @@ topicsForSourceIterator (wList list, void* element, void* c)

                 mamaSubscriptionType subscType = MAMA_SUBSC_TYPE_NORMAL;

                 mamaSubscription_getSubscriptionType (subsc->mSubscription, &subscType);

                 closure->types[closure->curIdx] = subscType;

-                closure->topics[closure->curIdx++] = topic;

+                closure->topics[closure->curIdx++] = strdup (topic);

             }

         }

     }

--

1.8.3.1