On Fri, 30 Nov 2018 at 09:12, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
>
> * Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>
> > From: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
> >
> > Since commit:
> >
> > ce2e6db554fa ("brcmfmac: Add support for getting nvram contents from
> > EFI variables")
>
> This commit ID is not upstream AFAICS. Which tree is it from? Mentioning
> non-upstream sha1's is discouraged in changelogs, as there's no guarantee
> that the sha1 will make it upstream.
>
This is a commit ID from Arend's own tree which is pulled into -next,
so I assumed that he'd only include commit IDs like this if they are
stable.
In any case, the fix itself is rather obvious, so much of the context
provided by the commit log could be summarized as '__efivars may be
NULL so check for that before you dereference it'
> > 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.
> >
> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Reported-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > Signed-off-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>
> Will that new commit be backported? If yes I suppose we could mark this
> fix -stable too? If not then it's fine for a v4.21 merge.
>
That commit is not -stable material at all, as far as I can tell.
