queue-4.14/firmware-efi-add-null-pointer-checks-in-efivars-api-.patch

   1 From f94593e62066f3ea5cc6f1835f82da910a508f32 Mon Sep 17 00:00:00 2001
   2 From: Arend van Spriel <arend.vanspriel@broadcom.com>
   3 Date: Thu, 29 Nov 2018 18:12:27 +0100
   4 Subject: firmware/efi: Add NULL pointer checks in efivars API functions
   5
   6 [ Upstream commit ab2180a15ce54739fed381efb4cb12e78dfb1561 ]
   7
   8 Since commit:
   9
  10    ce2e6db554fa ("brcmfmac: Add support for getting nvram contents from EFI variables")
  11
  12 we have a device driver accessing the efivars API. Several functions in
  13 the efivars API assume __efivars is set, i.e., that they will be accessed
  14 only after efivars_register() has been called. However, the following NULL
  15 pointer access was reported calling efivar_entry_size() from the brcmfmac
  16 device driver:
  17
  18   Unable to handle kernel NULL pointer dereference at virtual address 00000008
  19   pgd = 60bfa5f1
  20   [00000008] *pgd=00000000
  21   Internal error: Oops: 5 [#1] SMP ARM
  22   ...
  23   Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
  24   Workqueue: events request_firmware_work_func
  25   PC is at efivar_entry_size+0x28/0x90
  26   LR is at brcmf_fw_complete_request+0x3f8/0x8d4 [brcmfmac]
  27   pc : [<c0c40718>]    lr : [<bf2a3ef4>]    psr: a00d0113
  28   sp : ede7fe28  ip : ee983410  fp : c1787f30
  29   r10: 00000000  r9 : 00000000  r8 : bf2b2258
  30   r7 : ee983000  r6 : c1604c48  r5 : ede7fe88  r4 : edf337c0
  31   r3 : 00000000  r2 : 00000000  r1 : ede7fe88  r0 : c17712c8
  32   Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
  33   Control: 10c5387d  Table: ad16804a  DAC: 00000051
  34
  35 Disassembly showed that the local static variable __efivars is NULL,
  36 which is not entirely unexpected given that it is a non-EFI platform.
  37
  38 So add a NULL pointer check to efivar_entry_size(), and to related
  39 functions while at it. In efivars_register() a couple of sanity checks
  40 are added as well.
  41
  42 Reported-by: Jon Hunter <jonathanh@nvidia.com>
  43 Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
  44 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
  45 Cc: Andy Lutomirski <luto@kernel.org>
  46 Cc: Bhupesh Sharma <bhsharma@redhat.com>
  47 Cc: Borislav Petkov <bp@alien8.de>
  48 Cc: Dave Hansen <dave.hansen@intel.com>
  49 Cc: Eric Snowberg <eric.snowberg@oracle.com>
  50 Cc: Hans de Goede <hdegoede@redhat.com>
  51 Cc: Joe Perches <joe@perches.com>
  52 Cc: Julien Thierry <julien.thierry@arm.com>
  53 Cc: Linus Torvalds <torvalds@linux-foundation.org>
  54 Cc: Marc Zyngier <marc.zyngier@arm.com>
  55 Cc: Matt Fleming <matt@codeblueprint.co.uk>
  56 Cc: Nathan Chancellor <natechancellor@gmail.com>
  57 Cc: Peter Zijlstra <peterz@infradead.org>
  58 Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
  59 Cc: Sedat Dilek <sedat.dilek@gmail.com>
  60 Cc: Thomas Gleixner <tglx@linutronix.de>
  61 Cc: YiFei Zhu <zhuyifei1999@gmail.com>
  62 Cc: linux-efi@vger.kernel.org
  63 Link: http://lkml.kernel.org/r/20181129171230.18699-9-ard.biesheuvel@linaro.org
  64 Signed-off-by: Ingo Molnar <mingo@kernel.org>
  65 Signed-off-by: Sasha Levin <sashal@kernel.org>
  66 ---
  67  drivers/firmware/efi/vars.c | 99 +++++++++++++++++++++++++++++--------
  68  1 file changed, 78 insertions(+), 21 deletions(-)
  69
  70 diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
  71 index 9336ffdf6e2c..fceaafd67ec6 100644
  72 --- a/drivers/firmware/efi/vars.c
  73 +++ b/drivers/firmware/efi/vars.c
  74 @@ -318,7 +318,12 @@ EXPORT_SYMBOL_GPL(efivar_variable_is_removable);
  75  static efi_status_t
  76  check_var_size(u32 attributes, unsigned long size)
  77  {
  78 -       const struct efivar_operations *fops = __efivars->ops;
  79 +       const struct efivar_operations *fops;
  80 +
  81 +       if (!__efivars)
  82 +               return EFI_UNSUPPORTED;
  83 +
  84 +       fops = __efivars->ops;
  85
  86         if (!fops->query_variable_store)
  87                 return EFI_UNSUPPORTED;
  88 @@ -329,7 +334,12 @@ check_var_size(u32 attributes, unsigned long size)
  89  static efi_status_t
  90  check_var_size_nonblocking(u32 attributes, unsigned long size)
  91  {
  92 -       const struct efivar_operations *fops = __efivars->ops;
  93 +       const struct efivar_operations *fops;
  94 +
  95 +       if (!__efivars)
  96 +               return EFI_UNSUPPORTED;
  97 +
  98 +       fops = __efivars->ops;
  99
 100         if (!fops->query_variable_store)
 101                 return EFI_UNSUPPORTED;
 102 @@ -429,13 +439,18 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid,
 103  int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
 104                 void *data, bool duplicates, struct list_head *head)
 105  {
 106 -       const struct efivar_operations *ops = __efivars->ops;
 107 +       const struct efivar_operations *ops;
 108         unsigned long variable_name_size = 1024;
 109         efi_char16_t *variable_name;
 110         efi_status_t status;
 111         efi_guid_t vendor_guid;
 112         int err = 0;
 113
 114 +       if (!__efivars)
 115 +               return -EFAULT;
 116 +
 117 +       ops = __efivars->ops;
 118 +
 119         variable_name = kzalloc(variable_name_size, GFP_KERNEL);
 120         if (!variable_name) {
 121                 printk(KERN_ERR "efivars: Memory allocation failed.\n");
 122 @@ -583,12 +598,14 @@ static void efivar_entry_list_del_unlock(struct efivar_entry *entry)
 123   */
 124  int __efivar_entry_delete(struct efivar_entry *entry)
 125  {
 126 -       const struct efivar_operations *ops = __efivars->ops;
 127         efi_status_t status;
 128
 129 -       status = ops->set_variable(entry->var.VariableName,
 130 -                                  &entry->var.VendorGuid,
 131 -                                  0, 0, NULL);
 132 +       if (!__efivars)
 133 +               return -EINVAL;
 134 +
 135 +       status = __efivars->ops->set_variable(entry->var.VariableName,
 136 +                                             &entry->var.VendorGuid,
 137 +                                             0, 0, NULL);
 138
 139         return efi_status_to_err(status);
 140  }
 141 @@ -607,12 +624,17 @@ EXPORT_SYMBOL_GPL(__efivar_entry_delete);
 142   */
 143  int efivar_entry_delete(struct efivar_entry *entry)
 144  {
 145 -       const struct efivar_operations *ops = __efivars->ops;
 146 +       const struct efivar_operations *ops;
 147         efi_status_t status;
 148
 149         if (down_interruptible(&efivars_lock))
 150                 return -EINTR;
 151
 152 +       if (!__efivars) {
 153 +               up(&efivars_lock);
 154 +               return -EINVAL;
 155 +       }
 156 +       ops = __efivars->ops;
 157         status = ops->set_variable(entry->var.VariableName,
 158                                    &entry->var.VendorGuid,
 159                                    0, 0, NULL);
 160 @@ -650,13 +672,19 @@ EXPORT_SYMBOL_GPL(efivar_entry_delete);
 161  int efivar_entry_set(struct efivar_entry *entry, u32 attributes,
 162                      unsigned long size, void *data, struct list_head *head)
 163  {
 164 -       const struct efivar_operations *ops = __efivars->ops;
 165 +       const struct efivar_operations *ops;
 166         efi_status_t status;
 167         efi_char16_t *name = entry->var.VariableName;
 168         efi_guid_t vendor = entry->var.VendorGuid;
 169
 170         if (down_interruptible(&efivars_lock))
 171                 return -EINTR;
 172 +
 173 +       if (!__efivars) {
 174 +               up(&efivars_lock);
 175 +               return -EINVAL;
 176 +       }
 177 +       ops = __efivars->ops;
 178         if (head && efivar_entry_find(name, vendor, head, false)) {
 179                 up(&efivars_lock);
 180                 return -EEXIST;
 181 @@ -687,12 +715,17 @@ static int
 182  efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor,
 183                              u32 attributes, unsigned long size, void *data)
 184  {
 185 -       const struct efivar_operations *ops = __efivars->ops;
 186 +       const struct efivar_operations *ops;
 187         efi_status_t status;
 188
 189         if (down_trylock(&efivars_lock))
 190                 return -EBUSY;
 191
 192 +       if (!__efivars) {
 193 +               up(&efivars_lock);
 194 +               return -EINVAL;
 195 +       }
 196 +
 197         status = check_var_size_nonblocking(attributes,
 198                                             size + ucs2_strsize(name, 1024));
 199         if (status != EFI_SUCCESS) {
 200 @@ -700,6 +733,7 @@ efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor,
 201                 return -ENOSPC;
 202         }
 203
 204 +       ops = __efivars->ops;
 205         status = ops->set_variable_nonblocking(name, &vendor, attributes,
 206                                                size, data);
 207
 208 @@ -727,9 +761,13 @@ efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor,
 209  int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
 210                           bool block, unsigned long size, void *data)
 211  {
 212 -       const struct efivar_operations *ops = __efivars->ops;
 213 +       const struct efivar_operations *ops;
 214         efi_status_t status;
 215
 216 +       if (!__efivars)
 217 +               return -EINVAL;
 218 +
 219 +       ops = __efivars->ops;
 220         if (!ops->query_variable_store)
 221                 return -ENOSYS;
 222
 223 @@ -829,13 +867,18 @@ EXPORT_SYMBOL_GPL(efivar_entry_find);
 224   */
 225  int efivar_entry_size(struct efivar_entry *entry, unsigned long *size)
 226  {
 227 -       const struct efivar_operations *ops = __efivars->ops;
 228 +       const struct efivar_operations *ops;
 229         efi_status_t status;
 230
 231         *size = 0;
 232
 233         if (down_interruptible(&efivars_lock))
 234                 return -EINTR;
 235 +       if (!__efivars) {
 236 +               up(&efivars_lock);
 237 +               return -EINVAL;
 238 +       }
 239 +       ops = __efivars->ops;
 240         status = ops->get_variable(entry->var.VariableName,
 241                                    &entry->var.VendorGuid, NULL, size, NULL);
 242         up(&efivars_lock);
 243 @@ -861,12 +904,14 @@ EXPORT_SYMBOL_GPL(efivar_entry_size);
 244  int __efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
 245                        unsigned long *size, void *data)
 246  {
 247 -       const struct efivar_operations *ops = __efivars->ops;
 248         efi_status_t status;
 249
 250 -       status = ops->get_variable(entry->var.VariableName,
 251 -                                  &entry->var.VendorGuid,
 252 -                                  attributes, size, data);
 253 +       if (!__efivars)
 254 +               return -EINVAL;
 255 +
 256 +       status = __efivars->ops->get_variable(entry->var.VariableName,
 257 +                                             &entry->var.VendorGuid,
 258 +                                             attributes, size, data);
 259
 260         return efi_status_to_err(status);
 261  }
 262 @@ -882,14 +927,19 @@ EXPORT_SYMBOL_GPL(__efivar_entry_get);
 263  int efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
 264                      unsigned long *size, void *data)
 265  {
 266 -       const struct efivar_operations *ops = __efivars->ops;
 267         efi_status_t status;
 268
 269         if (down_interruptible(&efivars_lock))
 270                 return -EINTR;
 271 -       status = ops->get_variable(entry->var.VariableName,
 272 -                                  &entry->var.VendorGuid,
 273 -                                  attributes, size, data);
 274 +
 275 +       if (!__efivars) {
 276 +               up(&efivars_lock);
 277 +               return -EINVAL;
 278 +       }
 279 +
 280 +       status = __efivars->ops->get_variable(entry->var.VariableName,
 281 +                                             &entry->var.VendorGuid,
 282 +                                             attributes, size, data);
 283         up(&efivars_lock);
 284
 285         return efi_status_to_err(status);
 286 @@ -921,7 +971,7 @@ EXPORT_SYMBOL_GPL(efivar_entry_get);
 287  int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
 288                               unsigned long *size, void *data, bool *set)
 289  {
 290 -       const struct efivar_operations *ops = __efivars->ops;
 291 +       const struct efivar_operations *ops;
 292         efi_char16_t *name = entry->var.VariableName;
 293         efi_guid_t *vendor = &entry->var.VendorGuid;
 294         efi_status_t status;
 295 @@ -940,6 +990,11 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
 296         if (down_interruptible(&efivars_lock))
 297                 return -EINTR;
 298
 299 +       if (!__efivars) {
 300 +               err = -EINVAL;
 301 +               goto out;
 302 +       }
 303 +
 304         /*
 305          * Ensure that the available space hasn't shrunk below the safe level
 306          */
 307 @@ -956,6 +1011,8 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
 308                 }
 309         }
 310
 311 +       ops = __efivars->ops;
 312 +
 313         status = ops->set_variable(name, vendor, attributes, *size, data);
 314         if (status != EFI_SUCCESS) {
 315                 err = efi_status_to_err(status);
 316 --
 317 2.19.1
 318