core: harden lifecycle and random ID handling

CMakeLists.txt

@@ -115,7 +115,7 @@ check_c_source_compiles("
   #include <sys/random.h>
   int main() {
       unsigned int tmp;
-      getrandom(tmp, sizeof(tmp), GRND_NONBLOCK);
+      getrandom(&tmp, sizeof(tmp), GRND_NONBLOCK);
       return 0;
   }" CTR_HAVE_GETRANDOM)
 if(CTR_HAVE_GETRANDOM)

src/ctr_attributes.c

@@ -40,6 +40,10 @@ struct ctrace_attributes *ctr_attributes_create()
 
 void ctr_attributes_destroy(struct ctrace_attributes *attr)
 {
+    if (attr == NULL) {
+        return;
+    }
+
     if (attr->kv) {
         cfl_kvlist_destroy(attr->kv);
     }

src/ctr_id.c

@@ -37,7 +37,7 @@ struct ctrace_id *ctr_id_create_random(size_t size)
     }
 
     ret = ctr_random_get(buf, size);
-    if (ret < 0) {
+    if (ret != (ssize_t) size) {
         free(buf);
         return NULL;
     }
@@ -50,6 +50,10 @@ struct ctrace_id *ctr_id_create_random(size_t size)
 
 void ctr_id_destroy(struct ctrace_id *cid)
 {
+    if (cid == NULL) {
+        return;
+    }
+
     cfl_sds_destroy(cid->buf);
     free(cid);
 }
@@ -59,7 +63,7 @@ struct ctrace_id *ctr_id_create(void *buf, size_t len)
     int ret;
     struct ctrace_id *cid;
 
-    if (len <= 0) {
+    if (buf == NULL || len == 0) {
         return NULL;
     }
 
@@ -80,15 +84,22 @@ struct ctrace_id *ctr_id_create(void *buf, size_t len)
 
 int ctr_id_set(struct ctrace_id *cid, void *buf, size_t len)
 {
-    if (cid->buf) {
-        cfl_sds_destroy(cid->buf);
+    cfl_sds_t new_buf;
+
+    if (cid == NULL || buf == NULL || len == 0) {
+        return -1;
     }
 
-    cid->buf = cfl_sds_create_len(buf, len);
-    if (!cid->buf) {
+    new_buf = cfl_sds_create_len(buf, len);
+    if (new_buf == NULL) {
         return -1;
     }
 
+    if (cid->buf != NULL) {
+        cfl_sds_destroy(cid->buf);
+    }
+    cid->buf = new_buf;
+
     return 0;
 }
 

src/ctr_link.c

@@ -25,6 +25,10 @@ struct ctrace_link *ctr_link_create(struct ctrace_span *span,
 {
     struct ctrace_link *link;
 
+    if (span == NULL) {
+        return NULL;
+    }
+
     link = calloc(1, sizeof(struct ctrace_link));
     if (!link) {
         ctr_errno();
@@ -78,15 +82,22 @@ struct ctrace_link *ctr_link_create_with_cid(struct ctrace_span *span,
 
 int ctr_link_set_trace_state(struct ctrace_link *link, char *trace_state)
 {
+    cfl_sds_t new_trace_state;
+
     if (!link || !trace_state) {
         return -1;
     }
 
-    link->trace_state = cfl_sds_create(trace_state);
-    if (!link->trace_state) {
+    new_trace_state = cfl_sds_create(trace_state);
+    if (!new_trace_state) {
         return -1;
     }
 
+    if (link->trace_state != NULL) {
+        cfl_sds_destroy(link->trace_state);
+    }
+    link->trace_state = new_trace_state;
+
     return 0;
 }
 
@@ -96,7 +107,12 @@ int ctr_link_set_attributes(struct ctrace_link *link, struct ctrace_attributes *
         return -1;
     }
 
-    link->attr = attr;
+    if (link->attr != attr) {
+        if (link->attr != NULL) {
+            ctr_attributes_destroy(link->attr);
+        }
+        link->attr = attr;
+    }
     return 0;
 }
 
@@ -142,6 +158,3 @@ void ctr_link_destroy(struct ctrace_link *link)
 
 
 
-
-
-

src/ctr_random.c

@@ -29,6 +29,7 @@
 #endif
 
 #ifdef ITS_A_UNIX_FRIEND
+#include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 #else
@@ -41,46 +42,60 @@
 
 #endif
 
-#include <time.h>
-
 ssize_t ctr_random_get(void *buf, size_t len)
 {
-    int i;
-    ssize_t ret = 0;
-    unsigned int s;
-    char *tmp;
+    size_t offset;
+    ssize_t ret;
+
+    if (buf == NULL || len == 0) {
+        return -1;
+    }
 
 #ifdef CTR_HAVE_GETRANDOM
-    /*
-     * On Linux systems getrandom() is preferred, note that our use case it's pretty
-     * simple (no security stuff).
-     */
-    ret = getrandom(buf, len, GRND_NONBLOCK);
-    return ret;
+    offset = 0;
+    while (offset < len) {
+        ret = getrandom((char *) buf + offset, len - offset, 0);
+        if (ret > 0) {
+            offset += ret;
+        }
+        else if (ret < 0 && errno == EINTR) {
+            continue;
+        }
+        else {
+            return -1;
+        }
+    }
+    return (ssize_t) offset;
 #endif
 
     /* if getrandom() is not available and we are on Linux, macOS or BSD, try out /dev/urandom */
 #ifdef ITS_A_UNIX_FRIEND
     int fd;
 
     fd = open("/dev/urandom",  O_RDONLY);
-    if (fd > 0) {
-        ret = read(fd, buf, len);
+    if (fd >= 0) {
+        offset = 0;
+        while (offset < len) {
+            ret = read(fd, (char *) buf + offset, len - offset);
+            if (ret > 0) {
+                offset += ret;
+            }
+            else if (ret < 0 && errno == EINTR) {
+                continue;
+            }
+            else {
+                close(fd);
+                return -1;
+            }
+        }
         close(fd);
-        return ret;
-    }
-
-    s = time(NULL);
-
-    /* fallback... a very slow way to compose a random buffer */
-    tmp = buf;
-    for (i = 0; i < len; i++) {
-        /* fixme: we need a good entropy here */
-        tmp[i] = rand_r(&s);
+        return (ssize_t) offset;
     }
+    return -1;
 #else /* Windows ? */
-    ret = RtlGenRandom(buf, len);
+    if (RtlGenRandom(buf, len)) {
+        return (ssize_t) len;
+    }
+    return -1;
 #endif
-
-    return ret;
 }

src/ctr_resource.c

@@ -59,15 +59,16 @@ struct ctrace_resource *ctr_resource_create_default()
 
 int ctr_resource_set_attributes(struct ctrace_resource *res, struct ctrace_attributes *attr)
 {
-    if (!attr) {
+    if (!res || !attr) {
         return -1;
     }
 
-    if (res->attr) {
-        ctr_attributes_destroy(res->attr);
+    if (res->attr != attr) {
+        if (res->attr != NULL) {
+            ctr_attributes_destroy(res->attr);
+        }
+        res->attr = attr;
     }
-
-    res->attr = attr;
     return 0;
 }
 
@@ -94,6 +95,10 @@ struct ctrace_resource_span *ctr_resource_span_create(struct ctrace *ctx)
 {
     struct ctrace_resource_span *resource_span;
 
+    if (ctx == NULL) {
+        return NULL;
+    }
+
     resource_span = calloc(1, sizeof(struct ctrace_resource_span));
     if (!resource_span) {
         ctr_errno();
@@ -124,15 +129,22 @@ struct ctrace_resource *ctr_resource_span_get_resource(struct ctrace_resource_sp
 /* Set the schema_url for a resource_span */
 int ctr_resource_span_set_schema_url(struct ctrace_resource_span *resource_span, char *url)
 {
-    if (resource_span->schema_url) {
-        cfl_sds_destroy(resource_span->schema_url);
+    cfl_sds_t new_url;
+
+    if (resource_span == NULL || url == NULL) {
+        return -1;
     }
 
-    resource_span->schema_url = cfl_sds_create(url);
-    if (!resource_span->schema_url) {
+    new_url = cfl_sds_create(url);
+    if (new_url == NULL) {
         return -1;
     }
 
+    if (resource_span->schema_url != NULL) {
+        cfl_sds_destroy(resource_span->schema_url);
+    }
+    resource_span->schema_url = new_url;
+
     return 0;
 }
 
@@ -142,6 +154,15 @@ void ctr_resource_span_destroy(struct ctrace_resource_span *resource_span)
     struct cfl_list *head;
     struct ctrace_scope_span *scope_span;
 
+    if (resource_span == NULL) {
+        return;
+    }
+
+    /* Fluent Bit historically unlinked resource spans before destroying them. */
+    if (resource_span->_head.prev != NULL && resource_span->_head.next != NULL) {
+        cfl_list_del(&resource_span->_head);
+    }
+
     /* release resource if set */
     if (resource_span->resource) {
         ctr_resource_destroy(resource_span->resource);

src/ctr_scope.c

@@ -23,6 +23,10 @@ struct ctrace_scope_span *ctr_scope_span_create(struct ctrace_resource_span *res
 {
     struct ctrace_scope_span *scope_span;
 
+    if (resource_span == NULL) {
+        return NULL;
+    }
+
     scope_span = calloc(1, sizeof(struct ctrace_scope_span));
     if (!scope_span) {
         ctr_errno();
@@ -63,15 +67,22 @@ void ctr_scope_span_destroy(struct ctrace_scope_span *scope_span)
 /* Set the schema_url for a resource_span */
 int ctr_scope_span_set_schema_url(struct ctrace_scope_span *scope_span, char *url)
 {
-    if (scope_span->schema_url) {
-        cfl_sds_destroy(scope_span->schema_url);
+    cfl_sds_t new_url;
+
+    if (scope_span == NULL || url == NULL) {
+        return -1;
     }
 
-    scope_span->schema_url = cfl_sds_create(url);
-    if (!scope_span->schema_url) {
+    new_url = cfl_sds_create(url);
+    if (new_url == NULL) {
         return -1;
     }
 
+    if (scope_span->schema_url) {
+        cfl_sds_destroy(scope_span->schema_url);
+    }
+    scope_span->schema_url = new_url;
+
     return 0;
 }
 
@@ -100,9 +111,18 @@ struct ctrace_instrumentation_scope *ctr_instrumentation_scope_create(char *name
 
     if (name) {
         ins_scope->name = cfl_sds_create(name);
+        if (ins_scope->name == NULL) {
+            free(ins_scope);
+            return NULL;
+        }
     }
     if (version) {
         ins_scope->version = cfl_sds_create(version);
+        if (ins_scope->version == NULL) {
+            cfl_sds_destroy(ins_scope->name);
+            free(ins_scope);
+            return NULL;
+        }
     }
 
     ins_scope->dropped_attr_count = dropped_attr_count;
@@ -127,4 +147,3 @@ void ctr_instrumentation_scope_destroy(struct ctrace_instrumentation_scope *ins_
 
     free(ins_scope);
 }
-