From patchwork Thu Mar 8 18:14:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Suchanek X-Patchwork-Id: 10268641 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 86436602BD for ; Thu, 8 Mar 2018 18:14:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 75C7A291B8 for ; Thu, 8 Mar 2018 18:14:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 69CB32928F; Thu, 8 Mar 2018 18:14:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9FB24291B8 for ; Thu, 8 Mar 2018 18:14:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754829AbeCHSOb (ORCPT ); Thu, 8 Mar 2018 13:14:31 -0500 Received: from mx2.suse.de ([195.135.220.15]:45952 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752023AbeCHSOa (ORCPT ); Thu, 8 Mar 2018 13:14:30 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 21E59AD03; Thu, 8 Mar 2018 18:14:29 +0000 (UTC) From: Michal Suchanek To: Yauheni Kaliuta Cc: Michal Suchanek , Lucas De Marchi , linux-modules , Ferry van Steen , David Howells Subject: [PATCH] libkmod-signature: Fix crash when module signature is not present. Date: Thu, 8 Mar 2018 19:14:26 +0100 Message-Id: <20180308181426.5617-1-msuchanek@suse.de> X-Mailer: git-send-email 2.13.6 In-Reply-To: <20180308135810.4309-2-yauheni.kaliuta@redhat.com> References: <20180308135810.4309-2-yauheni.kaliuta@redhat.com> Sender: owner-linux-modules@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP The mod_sig is allocated on stack and when no signature is present it is not initialized and contains garbage. Later when freeing mod_sig garbage pointer is dereferenced. Make mod_sig pointer initialized to NULL instead so it has meaningful value only when a signature was stored in it. This somewhat straightens the interface to kmod_module_signature_info as well. Signed-off-by: Michal Suchanek --- libkmod/libkmod-internal.h | 2 +- libkmod/libkmod-module.c | 21 +++++++++--------- libkmod/libkmod-signature.c | 53 +++++++++++++++++++++++++-------------------- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h index 2ad74c7a33a5..8bc9ecfb906e 100644 --- a/libkmod/libkmod-internal.h +++ b/libkmod/libkmod-internal.h @@ -192,5 +192,5 @@ struct kmod_signature_info { void (*free)(void *); void *private; }; -bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info) _must_check_ __attribute__((nonnull(1, 2))); +struct kmod_signature_info *kmod_module_signature_info(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1))); void kmod_module_signature_info_free(struct kmod_signature_info *sig_info) __attribute__((nonnull)); diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 7b00d526edec..cbe35819932e 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -2304,7 +2304,7 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ struct kmod_elf *elf; 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; @@ -2341,32 +2341,32 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ goto list_error; } - if (kmod_module_signature_info(mod->file, &sig_info)) { + if (sig_info = kmod_module_signature_info(mod->file)) { 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++; @@ -2377,8 +2377,8 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ */ 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; @@ -2389,7 +2389,8 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ list_error: /* aux structures freed in normal case also */ - kmod_module_signature_info_free(&sig_info); + if (sig_info) + kmod_module_signature_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 bdf84cb14718..fae074e6dd1d 100644 --- a/libkmod/libkmod-signature.c +++ b/libkmod/libkmod-signature.c @@ -93,13 +93,17 @@ struct module_signature { uint32_t sig_len; /* Length of signature data (big endian) */ }; -static bool +static struct kmod_signature_info * kmod_module_signature_info_default(const char *mem, off_t size, const struct module_signature *modsig, - size_t sig_len, - struct kmod_signature_info *sig_info) + size_t sig_len) { + struct kmod_signature_info *sig_info = malloc(sizeof *sig_info); + + if (!sig_info) + return NULL; + size -= sig_len; sig_info->sig = mem + size; sig_info->sig_len = sig_len; @@ -119,7 +123,7 @@ kmod_module_signature_info_default(const char *mem, sig_info->free = NULL; sig_info->private = NULL; - return true; + return sig_info; } static void kmod_module_signature_info_pkcs7_free(void *s) @@ -129,13 +133,13 @@ static void kmod_module_signature_info_pkcs7_free(void *s) pkcs7_free_cert(si->private); } -static bool +static struct kmod_signature_info * kmod_module_signature_info_pkcs7(const char *mem, off_t size, const struct module_signature *modsig, - size_t sig_len, - struct kmod_signature_info *sig_info) + size_t sig_len) { + struct kmod_signature_info *sig_info = NULL; const char *pkcs7_raw; struct pkcs7_cert *cert; @@ -144,7 +148,13 @@ kmod_module_signature_info_pkcs7(const char *mem, cert = pkcs7_parse_cert(pkcs7_raw, sig_len); if (cert == NULL) - return false; + return NULL; + + sig_info = malloc(sizeof *sig_info); + if (!sig_info) { + free(cert); + return NULL; + } sig_info->private = cert; sig_info->free = kmod_module_signature_info_pkcs7_free; @@ -162,7 +172,7 @@ kmod_module_signature_info_pkcs7(const char *mem, sig_info->hash_algo = cert->hash_algo; sig_info->id_type = pkey_id_type[modsig->id_type]; - return true; + return sig_info; } #define SIG_MAGIC "~Module signature appended~\n" @@ -178,48 +188,45 @@ kmod_module_signature_info_pkcs7(const char *mem, * [ SIG_MAGIC ] */ -bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signature_info *sig_info) +struct kmod_signature_info *kmod_module_signature_info(const struct kmod_file *file) { const char *mem; off_t size; const struct module_signature *modsig; size_t sig_len; - bool ret; size = kmod_file_get_size(file); mem = kmod_file_get_contents(file); if (size < (off_t)strlen(SIG_MAGIC)) - return false; + return NULL; size -= strlen(SIG_MAGIC); if (memcmp(SIG_MAGIC, mem + size, strlen(SIG_MAGIC)) != 0) - return false; + return NULL; if (size < (off_t)sizeof(struct module_signature)) - return false; + return NULL; size -= sizeof(struct module_signature); modsig = (struct module_signature *)(mem + size); if (modsig->algo >= PKEY_ALGO__LAST || modsig->hash >= PKEY_HASH__LAST || modsig->id_type >= PKEY_ID_TYPE__LAST) - return false; + return NULL; sig_len = be32toh(get_unaligned(&modsig->sig_len)); if (sig_len == 0 || size < (int64_t)(modsig->signer_len + modsig->key_id_len + sig_len)) - return false; + return NULL; if (modsig->id_type == PKEY_ID_PKCS7) - ret = kmod_module_signature_info_pkcs7(mem, size, - modsig, sig_len, - sig_info); + return kmod_module_signature_info_pkcs7(mem, size, + modsig, sig_len); else - ret = kmod_module_signature_info_default(mem, size, - modsig, sig_len, - sig_info); - return ret; + return kmod_module_signature_info_default(mem, size, + modsig, sig_len); } void kmod_module_signature_info_free(struct kmod_signature_info *sig_info) { if (sig_info->free) sig_info->free(sig_info); + free(sig_info); }