Received: by 2002:ac0:946b:0:0:0:0:0 with SMTP id j40csp2722733imj; Mon, 11 Feb 2019 07:31:22 -0800 (PST) X-Google-Smtp-Source: AHgI3Ia2LTX/PVdrucnykKmWsBrGPGpMfsdnLtcgA6wcZdmIYSu5LFCcFPsgobcIuxsy/okAJ+NZ X-Received: by 2002:a17:902:d01:: with SMTP id 1mr38526785plu.127.1549899081948; Mon, 11 Feb 2019 07:31:21 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1549899081; cv=none; d=google.com; s=arc-20160816; b=F4YJOZQnMdqLW0S1Pk/uop1HpRXjhglkG7dWc3K5CTxlkkPUBL7HUmCflHoZmtgDTu 8vcxDEG4OmyD98t7MnccbnwtC5A/jX9CLITw2EzZck3Aog4MqO3n7bJ4e63vIXseQjji 0iVvPTlMKesN8Qr3ctE6lGLF3TcXM3/WHL/dX/hb576CFeLBzqhKP/3BcdjCmQZaY9nw 24gmSx00r2jHlKLWWQovS3W2wa01P9vbEVkcNX4oAUHSdtz7/ZQbtFh5DdWM9Fna6Nrb +aPtv+w9zJYtqVC3/WHr4AwaeBTaLZRGU2XuQ12e5xN7RAH5O5A1KrQ04vgXTvX/+Bii I5iA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=ygx/Di+TqsCmD+VTIqM/Mw1VKF10ft2eKGktUS1mQcY=; b=IVAOyBGvAKGMHG62BBu1QTiFg2IoinIe2Y83a5gRycK3XgqoQPpbNdHDXoAbU8kzXL Oz9pPGAoQlktP8OVLYTcsxn8Bxj32776R4SFsaCfV114Fdg8dvNGZfn5cWL1RZxjHAXE HlpZ9BTGyRgQoi9GGB8+cib/2A+GVTTkoMvIimk6o2JfomBdnuHiIR9TZXNboHsboCA1 rUp6wVlix9lXuxP/JFrxEbd/aQfoBmASk2fwAs5N3ORFIJfFhdVp0q4V8IZM8RRo/aDA obtNp9rbDjQ7AM6WSuH3kA+cJtcJIb82b/35sbTjgUnc6BdZSS4ytrkn0VoFIHqoM21O +QPw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="Rp3I/KY5"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id i90si11030722pli.135.2019.02.11.07.31.05; Mon, 11 Feb 2019 07:31:21 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="Rp3I/KY5"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389467AbfBKO6E (ORCPT + 99 others); Mon, 11 Feb 2019 09:58:04 -0500 Received: from mail.kernel.org ([198.145.29.99]:45562 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389455AbfBKO6C (ORCPT ); Mon, 11 Feb 2019 09:58:02 -0500 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 76A93222A1; Mon, 11 Feb 2019 14:58:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1549897080; bh=HBm257pI5JqCPYUWCvnLgUulwsx7RUH9wJOOtfR5lTk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Rp3I/KY5ZihSFO2tuWEKqkbPYpHcQIPtoM+Luk4FxbqPSelolGdrmB5K1BcRwqeza mHQRHVrxrLQzb1XbClG3Q+2U8tHiXJUpE53vhrYabM13SigdBfSat6c/7QqVmXFYoH oDowQgO2C0p5l735JdOxc5grveVNmqzU7srS+HQM= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Jon Hunter , Arend van Spriel , Ard Biesheuvel , Andy Lutomirski , Bhupesh Sharma , Borislav Petkov , Dave Hansen , Eric Snowberg , Hans de Goede , Joe Perches , Julien Thierry , Linus Torvalds , Marc Zyngier , Matt Fleming , Nathan Chancellor , Peter Zijlstra , Sai Praneeth Prakhya , Sedat Dilek , Thomas Gleixner , YiFei Zhu , linux-efi@vger.kernel.org, Ingo Molnar , Sasha Levin Subject: [PATCH 4.14 054/205] firmware/efi: Add NULL pointer checks in efivars API functions Date: Mon, 11 Feb 2019 15:17:32 +0100 Message-Id: <20190211141832.364983050@linuxfoundation.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190211141827.214852402@linuxfoundation.org> References: <20190211141827.214852402@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.14-stable review patch. If anyone has any objections, please let me know. ------------------ [ Upstream commit ab2180a15ce54739fed381efb4cb12e78dfb1561 ] Since commit: ce2e6db554fa ("brcmfmac: Add support for getting nvram contents from EFI variables") we have a device driver accessing the efivars API. Several functions in the efivars API assume __efivars is set, i.e., that they will be accessed only after efivars_register() has been called. However, the following NULL pointer access was reported calling efivar_entry_size() from the brcmfmac device driver: Unable to handle kernel NULL pointer dereference at virtual address 00000008 pgd = 60bfa5f1 [00000008] *pgd=00000000 Internal error: Oops: 5 [#1] SMP ARM ... Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) Workqueue: events request_firmware_work_func PC is at efivar_entry_size+0x28/0x90 LR is at brcmf_fw_complete_request+0x3f8/0x8d4 [brcmfmac] pc : [] lr : [] psr: a00d0113 sp : ede7fe28 ip : ee983410 fp : c1787f30 r10: 00000000 r9 : 00000000 r8 : bf2b2258 r7 : ee983000 r6 : c1604c48 r5 : ede7fe88 r4 : edf337c0 r3 : 00000000 r2 : 00000000 r1 : ede7fe88 r0 : c17712c8 Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: ad16804a DAC: 00000051 Disassembly showed that the local static variable __efivars is NULL, which is not entirely unexpected given that it is a non-EFI platform. So add a NULL pointer check to efivar_entry_size(), and to related functions while at it. In efivars_register() a couple of sanity checks are added as well. Reported-by: Jon Hunter Signed-off-by: Arend van Spriel Signed-off-by: Ard Biesheuvel Cc: Andy Lutomirski Cc: Bhupesh Sharma Cc: Borislav Petkov Cc: Dave Hansen Cc: Eric Snowberg Cc: Hans de Goede Cc: Joe Perches Cc: Julien Thierry Cc: Linus Torvalds Cc: Marc Zyngier Cc: Matt Fleming Cc: Nathan Chancellor Cc: Peter Zijlstra Cc: Sai Praneeth Prakhya Cc: Sedat Dilek Cc: Thomas Gleixner Cc: YiFei Zhu Cc: linux-efi@vger.kernel.org Link: http://lkml.kernel.org/r/20181129171230.18699-9-ard.biesheuvel@linaro.org Signed-off-by: Ingo Molnar Signed-off-by: Sasha Levin --- drivers/firmware/efi/vars.c | 99 +++++++++++++++++++++++++++++-------- 1 file changed, 78 insertions(+), 21 deletions(-) diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index 9336ffdf6e2c..fceaafd67ec6 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -318,7 +318,12 @@ EXPORT_SYMBOL_GPL(efivar_variable_is_removable); static efi_status_t check_var_size(u32 attributes, unsigned long size) { - const struct efivar_operations *fops = __efivars->ops; + const struct efivar_operations *fops; + + if (!__efivars) + return EFI_UNSUPPORTED; + + fops = __efivars->ops; if (!fops->query_variable_store) return EFI_UNSUPPORTED; @@ -329,7 +334,12 @@ check_var_size(u32 attributes, unsigned long size) static efi_status_t check_var_size_nonblocking(u32 attributes, unsigned long size) { - const struct efivar_operations *fops = __efivars->ops; + const struct efivar_operations *fops; + + if (!__efivars) + return EFI_UNSUPPORTED; + + fops = __efivars->ops; if (!fops->query_variable_store) return EFI_UNSUPPORTED; @@ -429,13 +439,18 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid, int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *), void *data, bool duplicates, struct list_head *head) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; unsigned long variable_name_size = 1024; efi_char16_t *variable_name; efi_status_t status; efi_guid_t vendor_guid; int err = 0; + if (!__efivars) + return -EFAULT; + + ops = __efivars->ops; + variable_name = kzalloc(variable_name_size, GFP_KERNEL); if (!variable_name) { printk(KERN_ERR "efivars: Memory allocation failed.\n"); @@ -583,12 +598,14 @@ static void efivar_entry_list_del_unlock(struct efivar_entry *entry) */ int __efivar_entry_delete(struct efivar_entry *entry) { - const struct efivar_operations *ops = __efivars->ops; efi_status_t status; - status = ops->set_variable(entry->var.VariableName, - &entry->var.VendorGuid, - 0, 0, NULL); + if (!__efivars) + return -EINVAL; + + status = __efivars->ops->set_variable(entry->var.VariableName, + &entry->var.VendorGuid, + 0, 0, NULL); return efi_status_to_err(status); } @@ -607,12 +624,17 @@ EXPORT_SYMBOL_GPL(__efivar_entry_delete); */ int efivar_entry_delete(struct efivar_entry *entry) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_status_t status; if (down_interruptible(&efivars_lock)) return -EINTR; + if (!__efivars) { + up(&efivars_lock); + return -EINVAL; + } + ops = __efivars->ops; status = ops->set_variable(entry->var.VariableName, &entry->var.VendorGuid, 0, 0, NULL); @@ -650,13 +672,19 @@ EXPORT_SYMBOL_GPL(efivar_entry_delete); int efivar_entry_set(struct efivar_entry *entry, u32 attributes, unsigned long size, void *data, struct list_head *head) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_status_t status; efi_char16_t *name = entry->var.VariableName; efi_guid_t vendor = entry->var.VendorGuid; if (down_interruptible(&efivars_lock)) return -EINTR; + + if (!__efivars) { + up(&efivars_lock); + return -EINVAL; + } + ops = __efivars->ops; if (head && efivar_entry_find(name, vendor, head, false)) { up(&efivars_lock); return -EEXIST; @@ -687,12 +715,17 @@ static int efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor, u32 attributes, unsigned long size, void *data) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_status_t status; if (down_trylock(&efivars_lock)) return -EBUSY; + if (!__efivars) { + up(&efivars_lock); + return -EINVAL; + } + status = check_var_size_nonblocking(attributes, size + ucs2_strsize(name, 1024)); if (status != EFI_SUCCESS) { @@ -700,6 +733,7 @@ efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor, return -ENOSPC; } + ops = __efivars->ops; status = ops->set_variable_nonblocking(name, &vendor, attributes, size, data); @@ -727,9 +761,13 @@ efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor, int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes, bool block, unsigned long size, void *data) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_status_t status; + if (!__efivars) + return -EINVAL; + + ops = __efivars->ops; if (!ops->query_variable_store) return -ENOSYS; @@ -829,13 +867,18 @@ EXPORT_SYMBOL_GPL(efivar_entry_find); */ int efivar_entry_size(struct efivar_entry *entry, unsigned long *size) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_status_t status; *size = 0; if (down_interruptible(&efivars_lock)) return -EINTR; + if (!__efivars) { + up(&efivars_lock); + return -EINVAL; + } + ops = __efivars->ops; status = ops->get_variable(entry->var.VariableName, &entry->var.VendorGuid, NULL, size, NULL); up(&efivars_lock); @@ -861,12 +904,14 @@ EXPORT_SYMBOL_GPL(efivar_entry_size); int __efivar_entry_get(struct efivar_entry *entry, u32 *attributes, unsigned long *size, void *data) { - const struct efivar_operations *ops = __efivars->ops; efi_status_t status; - status = ops->get_variable(entry->var.VariableName, - &entry->var.VendorGuid, - attributes, size, data); + if (!__efivars) + return -EINVAL; + + status = __efivars->ops->get_variable(entry->var.VariableName, + &entry->var.VendorGuid, + attributes, size, data); return efi_status_to_err(status); } @@ -882,14 +927,19 @@ EXPORT_SYMBOL_GPL(__efivar_entry_get); int efivar_entry_get(struct efivar_entry *entry, u32 *attributes, unsigned long *size, void *data) { - const struct efivar_operations *ops = __efivars->ops; efi_status_t status; if (down_interruptible(&efivars_lock)) return -EINTR; - status = ops->get_variable(entry->var.VariableName, - &entry->var.VendorGuid, - attributes, size, data); + + if (!__efivars) { + up(&efivars_lock); + return -EINVAL; + } + + status = __efivars->ops->get_variable(entry->var.VariableName, + &entry->var.VendorGuid, + attributes, size, data); up(&efivars_lock); return efi_status_to_err(status); @@ -921,7 +971,7 @@ EXPORT_SYMBOL_GPL(efivar_entry_get); int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes, unsigned long *size, void *data, bool *set) { - const struct efivar_operations *ops = __efivars->ops; + const struct efivar_operations *ops; efi_char16_t *name = entry->var.VariableName; efi_guid_t *vendor = &entry->var.VendorGuid; efi_status_t status; @@ -940,6 +990,11 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes, if (down_interruptible(&efivars_lock)) return -EINTR; + if (!__efivars) { + err = -EINVAL; + goto out; + } + /* * Ensure that the available space hasn't shrunk below the safe level */ @@ -956,6 +1011,8 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes, } } + ops = __efivars->ops; + status = ops->set_variable(name, vendor, attributes, *size, data); if (status != EFI_SUCCESS) { err = efi_status_to_err(status); -- 2.19.1