From: Emil Velikov Date: Sun, 15 Feb 2026 21:34:01 +0000 (+0000) Subject: libkmod/libkmod-signature: rework struct kmod_signature_info X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ef7ade1d11d1203dc0f8767f2cdc578c39ed759;p=thirdparty%2Fkmod.git libkmod/libkmod-signature: rework struct kmod_signature_info Currently, we use a stack allocated instance which bolts of misc private data via a void *, while also having an optional free callback. In kmod we opt to pre-calculate the total size, do a one-off allocation, copy the data as needed, adjusting the pointers. Doing the same here, gives us a mixed bag of benefits: - shorter and simpler code - smaller binary - ~100 bytes off - fewer instructions - ~40 per module - few allocations - ~2 per module - extra bytes are allocated - ~180 per module The updated code seems far more natural and consistent with the code-base. Although, if we really want to squeeze more cycles we could use a reasonably large stack buffer and fallback to heap. v2: - don't leak on d2i_PKCS7_bio failure - use +1 (instead of sizeof('\0')) for the null terminator Signed-off-by: Emil Velikov Link: https://github.com/kmod-project/kmod/pull/427 Signed-off-by: Lucas De Marchi --- diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h index d1177102..cc681e22 100644 --- a/libkmod/libkmod-internal.h +++ b/libkmod/libkmod-internal.h @@ -173,11 +173,8 @@ struct kmod_signature_info { const char *hash_algo, *id_type; const char *sig; size_t sig_len; - void (*free)(void *); - void *private; }; -_must_check_ _nonnull_all_ bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info); -_nonnull_all_ void kmod_module_signature_info_free(struct kmod_signature_info *sig_info); +_must_check_ _nonnull_all_ bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info **sig_info); /* libkmod-builtin.c */ _nonnull_all_ ssize_t kmod_builtin_get_modinfo(struct kmod_ctx *ctx, const char *modname, char ***modinfo); diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 72beec14..e849dd9f 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -1869,7 +1869,7 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, { char **strings; int i, count, ret = -ENOMEM; - struct kmod_signature_info sig_info = {}; + struct kmod_signature_info *sig_info = NULL; if (mod == NULL || list == NULL) return -ENOENT; @@ -1918,36 +1918,36 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_list *n; n = kmod_module_info_append(list, "sig_id", strlen("sig_id"), - sig_info.id_type, strlen(sig_info.id_type)); + sig_info->id_type, strlen(sig_info->id_type)); if (n == NULL) goto list_error; count++; n = kmod_module_info_append(list, "signer", strlen("signer"), - sig_info.signer, sig_info.signer_len); + sig_info->signer, sig_info->signer_len); if (n == NULL) goto list_error; count++; n = kmod_module_info_append_hex(list, "sig_key", strlen("sig_key"), - sig_info.key_id, sig_info.key_id_len); + sig_info->key_id, sig_info->key_id_len); if (n == NULL) goto list_error; count++; n = kmod_module_info_append(list, "sig_hashalgo", strlen("sig_hashalgo"), - sig_info.hash_algo, - strlen(sig_info.hash_algo)); + sig_info->hash_algo, + strlen(sig_info->hash_algo)); if (n == NULL) goto list_error; count++; /* - * Omit sig_info.algo for now, as these + * Omit sig_info->algo for now, as these * are currently constant. */ n = kmod_module_info_append_hex(list, "signature", strlen("signature"), - sig_info.sig, sig_info.sig_len); + sig_info->sig, sig_info->sig_len); if (n == NULL) goto list_error; @@ -1957,7 +1957,7 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, list_error: /* aux structures freed in normal case also */ - kmod_module_signature_info_free(&sig_info); + free(sig_info); if (ret < 0) { kmod_module_info_free_list(*list); diff --git a/libkmod/libkmod-signature.c b/libkmod/libkmod-signature.c index ff4c57a2..2af16ded 100644 --- a/libkmod/libkmod-signature.c +++ b/libkmod/libkmod-signature.c @@ -83,8 +83,12 @@ struct module_signature { static bool fill_default(const char *mem, off_t size, const struct module_signature *modsig, size_t sig_len, - struct kmod_signature_info *sig_info) + struct kmod_signature_info **out_sig_info) { + struct kmod_signature_info *sig_info = calloc(1, sizeof(*sig_info)); + if (sig_info == NULL) + return false; + size -= sig_len; sig_info->sig = mem + size; sig_info->sig_len = sig_len; @@ -99,31 +103,13 @@ static bool fill_default(const char *mem, off_t size, sig_info->hash_algo = pkey_hash_algo[modsig->hash]; + *out_sig_info = sig_info; + return true; } #if ENABLE_OPENSSL -struct pkcs7_private { - PKCS7 *pkcs7; - char *key_id; - BIGNUM *sno; - char *hash_algo; -}; - -static void pkcs7_free(void *s) -{ - struct kmod_signature_info *si = s; - struct pkcs7_private *pvt = si->private; - - PKCS7_free(pvt->pkcs7); - BN_free(pvt->sno); - free(pvt->key_id); - free(pvt->hash_algo); - free(pvt); - si->private = NULL; -} - static const char *x509_name_to_str(X509_NAME *name) { int i; @@ -150,8 +136,10 @@ static const char *x509_name_to_str(X509_NAME *name) } static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, - struct kmod_signature_info *sig_info) + struct kmod_signature_info **out_sig_info) { + struct kmod_signature_info *sig_info; + char *p; const char *pkcs7_raw; PKCS7 *pkcs7; STACK_OF(PKCS7_SIGNER_INFO) * sis; @@ -162,12 +150,9 @@ static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, X509_ALGOR *dig_alg; const ASN1_OBJECT *o; BIO *in; - int len; - char *key_id_str; - struct pkcs7_private *pvt; const char *issuer_str; - char *hash_algo; int hash_algo_len; + size_t total_len; size -= sig_len; pkcs7_raw = mem + size; @@ -175,13 +160,11 @@ static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, in = BIO_new_mem_buf(pkcs7_raw, sig_len); pkcs7 = d2i_PKCS7_bio(in, NULL); - if (pkcs7 == NULL) { - BIO_free(in); - return false; - } - BIO_free(in); + if (pkcs7 == NULL) + goto err; + sis = PKCS7_get_signer_info(pkcs7); if (sis == NULL) goto err; @@ -191,66 +174,86 @@ static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, goto err; is = si->issuer_and_serial; - sig = si->enc_digest; - PKCS7_SIGNER_INFO_get0_algs(si, NULL, &dig_alg, NULL); + /* Calculate the total length */ + total_len = sizeof(struct kmod_signature_info); - sig_info->sig = (const char *)ASN1_STRING_get0_data(sig); - sig_info->sig_len = ASN1_STRING_length(sig); + /* signer */ + issuer_str = x509_name_to_str(is->issuer); + if (issuer_str != NULL) + total_len += strlen(issuer_str); + /* key_id */ sno_bn = ASN1_INTEGER_to_BN(is->serial, NULL); if (sno_bn == NULL) goto err; - len = BN_num_bytes(sno_bn); - key_id_str = malloc(len); - if (key_id_str == NULL) - goto err2; - BN_bn2bin(sno_bn, (unsigned char *)key_id_str); + total_len += BN_num_bytes(sno_bn); - sig_info->key_id = key_id_str; - sig_info->key_id_len = len; + /* hash_algo */ + PKCS7_SIGNER_INFO_get0_algs(si, NULL, &dig_alg, NULL); + X509_ALGOR_get0(&o, NULL, NULL, dig_alg); + hash_algo_len = OBJ_obj2txt(NULL, 0, o, 0); + if (hash_algo_len < 0) + goto err1; - issuer_str = x509_name_to_str(is->issuer); + total_len += hash_algo_len + 1; + + /* sig */ + sig = si->enc_digest; + total_len += ASN1_STRING_length(sig); + + sig_info = calloc(1, total_len); + if (sig_info == NULL) + goto err1; + + p = (char *)sig_info; + + p += sizeof(struct kmod_signature_info); + + /* signer */ if (issuer_str != NULL) { - sig_info->signer = issuer_str; + sig_info->signer = p; sig_info->signer_len = strlen(issuer_str); + + memcpy(p, issuer_str, sig_info->signer_len); + p += sig_info->signer_len; } - X509_ALGOR_get0(&o, NULL, NULL, dig_alg); + /* key_id */ + sig_info->key_id = p; + sig_info->key_id_len = BN_num_bytes(sno_bn); - // Use OBJ_obj2txt to calculate string length - hash_algo_len = OBJ_obj2txt(NULL, 0, o, 0); - if (hash_algo_len < 0) - goto err3; - hash_algo = malloc(hash_algo_len + 1); - if (hash_algo == NULL) - goto err3; - hash_algo_len = OBJ_obj2txt(hash_algo, hash_algo_len + 1, o, 0); + BN_bn2bin(sno_bn, (unsigned char *)p); + p += sig_info->key_id_len; + + /* hash_algo */ + sig_info->hash_algo = p; + + hash_algo_len = OBJ_obj2txt(p, hash_algo_len + 1, o, 0); if (hash_algo_len < 0) - goto err4; + goto err2; + p += hash_algo_len; - // Assign libcrypto hash algo string or number - sig_info->hash_algo = hash_algo; + *p = '\0'; + p++; - pvt = malloc(sizeof(*pvt)); - if (pvt == NULL) - goto err4; + /* sig */ + sig_info->sig = p; + sig_info->sig_len = ASN1_STRING_length(sig); + + memcpy(p, ASN1_STRING_get0_data(sig), sig_info->sig_len); - pvt->pkcs7 = pkcs7; - pvt->key_id = key_id_str; - pvt->sno = sno_bn; - pvt->hash_algo = hash_algo; - sig_info->private = pvt; + BN_free(sno_bn); + PKCS7_free(pkcs7); - sig_info->free = pkcs7_free; + *out_sig_info = sig_info; return true; -err4: - free(hash_algo); -err3: - free(key_id_str); + err2: + free(sig_info); +err1: BN_free(sno_bn); err: PKCS7_free(pkcs7); @@ -260,9 +263,14 @@ err: #else static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, - struct kmod_signature_info *sig_info) + struct kmod_signature_info **out_sig_info) { + struct kmod_signature_info *sig_info = calloc(1, sizeof(*sig_info)); + if (sig_info == NULL) + return false; + sig_info->hash_algo = "unknown"; + *out_sig_info = sig_info; return true; } @@ -282,14 +290,14 @@ static bool fill_pkcs7(const char *mem, off_t size, size_t sig_len, */ bool kmod_module_signature_info(const struct kmod_file *file, - struct kmod_signature_info *sig_info) + struct kmod_signature_info **sig_info) { const char *mem; const void *contents; off_t size; struct module_signature modsig; size_t sig_len; - int ret; + bool ret; ret = kmod_file_get_contents(file, &contents, &size); if (ret) @@ -318,18 +326,17 @@ bool kmod_module_signature_info(const struct kmod_file *file, size < (int64_t)sig_len + modsig.signer_len + modsig.key_id_len) return false; - sig_info->id_type = pkey_id_type[modsig.id_type]; - switch (modsig.id_type) { case PKEY_ID_PKCS7: - return fill_pkcs7(mem, size, sig_len, sig_info); + ret = fill_pkcs7(mem, size, sig_len, sig_info); + break; default: - return fill_default(mem, size, &modsig, sig_len, sig_info); + ret = fill_default(mem, size, &modsig, sig_len, sig_info); + break; } -} -void kmod_module_signature_info_free(struct kmod_signature_info *sig_info) -{ - if (sig_info->free) - sig_info->free(sig_info); + if (ret) + (*sig_info)->id_type = pkey_id_type[modsig.id_type]; + + return ret; }