dlt-daemon: Fix use after free potential issue

In dlt_daemon_send_all_multiple, if the connection was broken, we closed
it before getting the next available connection. This must be avoided by
having a temporary next pointer.
The same kind of problem is valid for pointers coming from the epoll
interface. The kernel can provide back connection pointer that are not
valid any longer. Therefore, we need to use an ID instead of the pointer
value to retrieve the connections.

Signed-off-by: Frederic Berat <fberat@de.adit-jv.com>
Signed-off-by: Christoph Lipka <clipka@jp.adit-jv.com>

5 files changed, 48 insertions, 3 deletions

diff --git a/src/daemon/dlt_daemon_client.c b/src/daemon/dlt_daemon_client.c
index e605bd5..84ffd5d 100644
--- a/src/daemon/dlt_daemon_client.c
+++ b/src/daemon/dlt_daemon_client.c
@@ -133,6 +133,8 @@ static int dlt_daemon_client_send_all_multiple(DltDaemon *daemon,
     {
         int ret = 0;
         DLT_DAEMON_SEM_LOCK();
+        DltConnection *next = dlt_connection_get_next(temp->next, type_mask);
+
         ret = dlt_connection_send_multiple(temp,
                                           data1,
                                           size1,
@@ -166,7 +168,7 @@ static int dlt_daemon_client_send_all_multiple(DltDaemon *daemon,
             sent = 1;
         }
 
-        temp = dlt_connection_get_next(temp->next, type_mask);
+        temp = next;
     } /* for */
 
     return sent;
diff --git a/src/daemon/dlt_daemon_connection.c b/src/daemon/dlt_daemon_connection.c
index b22f4b7..8d2aa67 100644
--- a/src/daemon/dlt_daemon_connection.c
+++ b/src/daemon/dlt_daemon_connection.c
@@ -47,6 +47,8 @@
 #include "dlt_common.h"
 #include "dlt_gateway.h"
 
+static DltConnectionId connectionId;
+
 /** @brief Generic sending function.
  *
  * We manage different type of connection which have similar send/write
@@ -319,6 +321,7 @@ void *dlt_connection_get_callback(DltConnection *con)
  */
 void dlt_connection_destroy(DltConnection *to_destroy)
 {
+    to_destroy->id = 0;
     close(to_destroy->receiver->fd);
     dlt_connection_destroy_receiver(to_destroy);
     /* connection pointer might be in epoll queue and used even after destroying
@@ -388,6 +391,14 @@ int dlt_connection_create(DltDaemonLocal *daemon_local,
         return -1;
     }
 
+    /* We are single threaded no need for protection. */
+    temp->id = connectionId++;
+    if (!temp->id)
+    {
+        /* Skipping 0 */
+        temp->id = connectionId++;
+    }
+
     temp->type = type;
     temp->status = ACTIVE;
 
diff --git a/src/daemon/dlt_daemon_connection_types.h b/src/daemon/dlt_daemon_connection_types.h
index 1e1186b..8774378 100644
--- a/src/daemon/dlt_daemon_connection_types.h
+++ b/src/daemon/dlt_daemon_connection_types.h
@@ -67,10 +67,13 @@ typedef enum {
 #define DLT_CON_MASK_GATEWAY_TIMER      (1 << DLT_CONNECTION_GATEWAY_TIMER)
 #define DLT_CON_MASK_ALL                (0xffff)
 
+typedef unsigned int DltConnectionId;
+
 /* TODO: squash the DltReceiver structure in there
  * and remove any other duplicates of FDs
  */
 typedef struct DltConnection {
+    DltConnectionId id;
     DltReceiver *receiver; /**< Receiver structure for this connection */
     DltConnectionType type; /**< Represents what type of handle is this (like FIFO, serial, client, server) */
     DltConnectionStatus status; /**< Status of connection */
diff --git a/src/daemon/dlt_daemon_event_handler.c b/src/daemon/dlt_daemon_event_handler.c
index cfe61a4..9051f07 100644
--- a/src/daemon/dlt_daemon_event_handler.c
+++ b/src/daemon/dlt_daemon_event_handler.c
@@ -128,7 +128,9 @@ int dlt_daemon_handle_event(DltEventHandler *pEvent,
     for (i = 0 ; i < nfds ; i++)
     {
         struct epoll_event *ev = &pEvent->events[i];
-        DltConnection *con = (DltConnection *)ev->data.ptr;
+        DltConnectionId id = (DltConnectionId)ev->data.ptr;
+        DltConnection *con = dlt_event_handler_find_connection_by_id(pEvent,
+                                                                     id);
         int fd = 0;
         DltConnectionType type = DLT_CONNECTION_TYPE_MAX;
 
@@ -210,6 +212,31 @@ DltConnection *dlt_event_handler_find_connection(DltEventHandler *ev,
     return temp;
 }
 
+/** @brief Find connection with a specific \a id in the connection list.
+ *
+ * There can be only one event per \a fd. We can then find a specific connection
+ * based on this \a fd. That allows to check if a specific \a fd has already been
+ * registered.
+ *
+ * @param ev The event handler structure where the list of connection is.
+ * @param id The identifier of the connection to be found.
+ *
+ * @return The found connection pointer, NULL otherwise.
+ */
+DltConnection *dlt_event_handler_find_connection_by_id(DltEventHandler *ev,
+                                                       DltConnectionId id)
+{
+
+    DltConnection *temp = ev->connections;
+
+    while ((temp != NULL) && (temp->id != id))
+    {
+        temp = temp->next;
+    }
+
+    return temp;
+}
+
 /** @brief Remove a connection from the list and destroy it.
  *
  * This function will first look for the connection in the event handler list,
@@ -351,7 +378,7 @@ int dlt_connection_check_activate(DltEventHandler *evhdl,
         {
             struct epoll_event ev; /* Content will be copied by the kernel */
             ev.events = con->ev_mask;
-            ev.data.ptr = (void *)con;
+            ev.data.ptr = (void *)con->id;
 
             snprintf(local_str,
                      DLT_DAEMON_TEXTBUFSIZE,
diff --git a/src/daemon/dlt_daemon_event_handler.h b/src/daemon/dlt_daemon_event_handler.h
index ea88da9..dc91b89 100644
--- a/src/daemon/dlt_daemon_event_handler.h
+++ b/src/daemon/dlt_daemon_event_handler.h
@@ -39,6 +39,8 @@
 int dlt_daemon_prepare_event_handling(DltEventHandler *);
 int dlt_daemon_handle_event(DltEventHandler *, DltDaemon *, DltDaemonLocal *);
 
+DltConnection *dlt_event_handler_find_connection_by_id(DltEventHandler *,
+                                                       DltConnectionId);
 DltConnection *dlt_event_handler_find_connection(DltEventHandler *,
                                                int);