From e9a2778efa6e4ee9940cdb56face621dc319787f Mon Sep 17 00:00:00 2001
From: Florian Dold <florian.dold@gmail.com>
Date: Mon, 9 Dec 2013 20:27:32 +0000
Subject: - fix use after free in set union (#3178)

---
 src/set/gnunet-service-set.c         | 17 ++++++++++++++---
 src/set/gnunet-service-set.h         |  8 ++++++--
 src/set/gnunet-service-set_union.c   |  5 +++++
 src/set/test_set_union_result_full.c | 15 ++++++++++-----
 4 files changed, 35 insertions(+), 10 deletions(-)

(limited to 'src/set')

diff --git a/src/set/gnunet-service-set.c b/src/set/gnunet-service-set.c
index cf03b06aa..428a9d002 100644
--- a/src/set/gnunet-service-set.c
+++ b/src/set/gnunet-service-set.c
@@ -955,8 +955,8 @@ handle_client_evaluate (void *cls,
   GNUNET_CONTAINER_DLL_insert (set->ops_head, set->ops_tail, op);
 
   op->channel = GNUNET_MESH_channel_create (mesh, op, &msg->target_peer,
-                                          GNUNET_APPLICATION_TYPE_SET,
-                                          GNUNET_MESH_OPTION_RELIABLE);
+                                            GNUNET_APPLICATION_TYPE_SET,
+                                            GNUNET_MESH_OPTION_RELIABLE);
 
   op->mq = GNUNET_MESH_mq_create (op->channel);
 
@@ -1261,17 +1261,28 @@ channel_new_cb (void *cls,
  */
 static void
 channel_end_cb (void *cls,
-               const struct GNUNET_MESH_Channel *channel, void *channel_ctx)
+                const struct GNUNET_MESH_Channel *channel, void *channel_ctx)
 {
   struct Operation *op = channel_ctx;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "channel end cb called\n");
   op->channel = NULL;
+  /* the vt can be null if a client already requested canceling op. */
   if (NULL != op->vt)
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "calling peer disconnect due to channel end\n");
     op->vt->peer_disconnect (op);
+  }
+
+  if (GNUNET_YES == op->keep)
+    return;
+
   /* mesh will never call us with the context again! */
   GNUNET_free (channel_ctx);
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "channel end cb finished\n");
 }
 
 
diff --git a/src/set/gnunet-service-set.h b/src/set/gnunet-service-set.h
index d82c72b16..95a24119b 100644
--- a/src/set/gnunet-service-set.h
+++ b/src/set/gnunet-service-set.h
@@ -311,8 +311,6 @@ struct Operation
   /**
    * GNUNET_YES if this is not a "real" set operation yet, and we still
    * need to wait for the other peer to give us more details.
-   *
-   * //TODO: replace with state-enum
    */
   int is_incoming;
 
@@ -346,6 +344,12 @@ struct Operation
     * a linked list.
     */
   struct Operation *prev;
+
+  /**
+   * Set to GNUNET_YES if the set service should not free
+   * the operation, as it is still needed (e.g. in some scheduled task).
+   */
+  int keep;
 };
 
 
diff --git a/src/set/gnunet-service-set_union.c b/src/set/gnunet-service-set_union.c
index be50ec8d6..6bb28471a 100644
--- a/src/set/gnunet-service-set_union.c
+++ b/src/set/gnunet-service-set_union.c
@@ -982,12 +982,15 @@ send_done_and_destroy (void *cls)
   struct Operation *op = cls;
   struct GNUNET_MQ_Envelope *ev;
   struct GNUNET_SET_ResultMessage *rm;
+  int keep = op->keep;
   ev = GNUNET_MQ_msg (rm, GNUNET_MESSAGE_TYPE_SET_RESULT);
   rm->request_id = htonl (op->spec->client_request_id);
   rm->result_status = htons (GNUNET_SET_STATUS_DONE);
   rm->element_type = htons (0);
   GNUNET_MQ_send (op->spec->set->client_mq, ev);
   _GSS_operation_destroy (op);
+  if (GNUNET_YES == keep)
+    GNUNET_free (op);
 }
 
 
@@ -1060,6 +1063,8 @@ finish_and_destroy (struct Operation *op)
 
   if (GNUNET_SET_RESULT_FULL == op->spec->result_mode)
   {
+    /* prevent that the op is free'd by the tunnel end handler */
+    op->keep = GNUNET_YES;
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "sending full result set\n");
     GNUNET_assert (NULL == op->state->full_result_iter);
     op->state->full_result_iter =
diff --git a/src/set/test_set_union_result_full.c b/src/set/test_set_union_result_full.c
index e54332b8b..2ddd65c7c 100644
--- a/src/set/test_set_union_result_full.c
+++ b/src/set/test_set_union_result_full.c
@@ -28,6 +28,8 @@
 #include "gnunet_set_service.h"
 
 
+static int ret;
+
 static struct GNUNET_PeerIdentity local_id;
 
 static struct GNUNET_HashCode app_id;
@@ -50,6 +52,7 @@ result_cb_set1 (void *cls, const struct GNUNET_SET_Element *element,
       break;
     case GNUNET_SET_STATUS_FAILURE:
       printf ("set 1: failure\n");
+      ret = 1;
       break;
     case GNUNET_SET_STATUS_DONE:
       printf ("set 1: done\n");
@@ -72,6 +75,7 @@ result_cb_set2 (void *cls, const struct GNUNET_SET_Element *element,
       break;
     case GNUNET_SET_STATUS_FAILURE:
       printf ("set 2: failure\n");
+      ret = 1;
       break;
     case GNUNET_SET_STATUS_DONE:
       printf ("set 2: done\n");
@@ -245,11 +249,12 @@ run (void *cls,
 int
 main (int argc, char **argv)
 {
-  int ret;
-
-  ret = GNUNET_TESTING_peer_run ("test_set_api",
-                                 "test_set.conf",
-                                 &run, NULL);
+  if (0 != GNUNET_TESTING_peer_run ("test_set_api",
+                                    "test_set.conf",
+                                    &run, NULL))
+  {
+    return 0;
+  }
   return ret;
 }
 
-- 
cgit v1.2.3