| From f94593e62066f3ea5cc6f1835f82da910a508f32 Mon Sep 17 00:00:00 2001 |
| From: Arend van Spriel <arend.vanspriel@broadcom.com> |
| Date: Thu, 29 Nov 2018 18:12:27 +0100 |
| Subject: firmware/efi: Add NULL pointer checks in efivars API functions |
| |
| [ 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 : [<c0c40718>] lr : [<bf2a3ef4>] 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 <jonathanh@nvidia.com> |
| Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> |
| Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> |
| Cc: Andy Lutomirski <luto@kernel.org> |
| Cc: Bhupesh Sharma <bhsharma@redhat.com> |
| Cc: Borislav Petkov <bp@alien8.de> |
| Cc: Dave Hansen <dave.hansen@intel.com> |
| Cc: Eric Snowberg <eric.snowberg@oracle.com> |
| Cc: Hans de Goede <hdegoede@redhat.com> |
| Cc: Joe Perches <joe@perches.com> |
| Cc: Julien Thierry <julien.thierry@arm.com> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Marc Zyngier <marc.zyngier@arm.com> |
| Cc: Matt Fleming <matt@codeblueprint.co.uk> |
| Cc: Nathan Chancellor <natechancellor@gmail.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com> |
| Cc: Sedat Dilek <sedat.dilek@gmail.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: YiFei Zhu <zhuyifei1999@gmail.com> |
| Cc: linux-efi@vger.kernel.org |
| Link: http://lkml.kernel.org/r/20181129171230.18699-9-ard.biesheuvel@linaro.org |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| 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 |
| |