Eliminate use of asprintf

Asprintf is convenient but causes mixing of OPENSSL and standard memory management in the code base which can quickly lead to bad problems, undetected until an application changes the openssl allocators. Signed-off-by: Simo Sorce <simo@redhat.com>
simo5 · Jan 13, 2023 · 083b72d · 083b72d
1 parent 9ea7c27
commit 083b72d
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 5 deletions.
diff --git a/src/session.c b/src/session.c
@@ -1,7 +1,6 @@
 /* Copyright (C) 2022 Simo Sorce <simo@redhat.com>
    SPDX-License-Identifier: Apache-2.0 */
 
-#define _GNU_SOURCE
 #include "provider.h"
 #include <string.h>
 
@@ -126,6 +125,7 @@ static void trim_padded_field(CK_UTF8CHAR *field, ssize_t n)
 }
 
 #define trim(x) trim_padded_field(x, sizeof(x))
+static const char slot_desc_fmt[] = "PKCS#11 Token (Slot %lu - %s)";
 
 CK_RV p11prov_init_slots(P11PROV_CTX *ctx, P11PROV_SLOTS_CTX **slots)
 {
@@ -207,9 +207,11 @@ CK_RV p11prov_init_slots(P11PROV_CTX *ctx, P11PROV_SLOTS_CTX **slots)
 
         slot->id = slotid[i];
 
-        err = asprintf(&slot->login_info, "PKCS#11 Token (Slot %lu - %s)",
-                       slot->id, slot->slot.slotDescription);
-        if (err < 0) {
+        /* upper bound = slot_desc_fmt + LONG_MAX chars + MAX SLOT DESC */
+        slot->login_info = p11prov_alloc_sprintf(
+            sizeof(slot_desc_fmt) + 20 + sizeof(slot->slot.slotDescription) + 1,
+            slot_desc_fmt, slot->id, slot->slot.slotDescription);
+        if (!slot->login_info) {
             ret = CKR_HOST_MEMORY;
             goto done;
         }
@@ -264,7 +266,7 @@ void p11prov_free_slots(P11PROV_SLOTS_CTX *sctx)
             OPENSSL_clear_free(sctx->slots[i]->bad_pin,
                                strlen(sctx->slots[i]->bad_pin));
         }
-        free(sctx->slots[i]->login_info);
+        OPENSSL_free(sctx->slots[i]->login_info);
         OPENSSL_cleanse(sctx->slots[i], sizeof(P11PROV_SLOT));
     }
     OPENSSL_free(sctx->slots);

diff --git a/src/util.c b/src/util.c
@@ -623,3 +623,41 @@ bool p11prov_x509_names_are_equal(CK_ATTRIBUTE *a, CK_ATTRIBUTE *b)
     X509_NAME_free(xb);
     return cmp == 0;
 }
+
+char *p11prov_alloc_sprintf(int size_hint, const char *format, ...)
+{
+    char *buf = NULL;
+    va_list args;
+    int repeat = 1;
+    int ret;
+
+again:
+    if (repeat-- < 0) {
+        ret = -1;
+        goto done;
+    }
+
+    if (size_hint) {
+        buf = OPENSSL_malloc(size_hint);
+    }
+
+    va_start(args, format);
+    ret = vsnprintf(buf, size_hint, format, args);
+    va_end(args);
+
+    if (ret >= size_hint) {
+        size_hint = ret + 1;
+        OPENSSL_free(buf);
+        buf = NULL;
+        goto again;
+    }
+
+done:
+    if (ret < 0) {
+        OPENSSL_free(buf);
+        buf = NULL;
+    } else if (size_hint > ret + 1) {
+        buf = OPENSSL_realloc(buf, ret + 1);
+    }
+    return buf;
+}
diff --git a/src/util.h b/src/util.h
@@ -104,5 +104,6 @@ CK_RV p11prov_token_sup_attr(P11PROV_CTX *ctx, CK_SLOT_ID id, int action,
                              CK_ATTRIBUTE_TYPE attr, CK_BBOOL *data);
 CK_RV p11prov_copy_attr(CK_ATTRIBUTE *dst, CK_ATTRIBUTE *src);
 bool p11prov_x509_names_are_equal(CK_ATTRIBUTE *a, CK_ATTRIBUTE *b);
+char *p11prov_alloc_sprintf(int size_hint, const char *format, ...);
 
 #endif /* _UTIL_H */