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;
MIME-Version: 1.0
References: <20191212103158.4958-1-hdegoede@redhat.com> <20191212103158.4958-2-hdegoede@redhat.com>
In-Reply-To: <20191212103158.4958-2-hdegoede@redhat.com>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Thu, 12 Dec 2019 10:37:58 +0000
Message-ID: <CAKv+Gu9Fa=ccEyGW=hyZya0y-ujL27twKVX5JwcqWRMQ=yfQzg@mail.gmail.com>
Subject: Re: [PATCH 5.5 regression fix 1/2] efi/libstub/random: Initialize
 pointer variables to zero for mixed mode
To:     Hans de Goede <hdegoede@redhat.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Dominik Brodowski <linux@dominikbrodowski.net>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        linux-efi <linux-efi@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 12 Dec 2019 at 11:32, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Commit 0d95981438c3 ("x86: efi/random: Invoke EFI_RNG_PROTOCOL to seed the
> UEFI RNG table"), causes the drivers/efi/libstub/random.c code to get used
> on x86 for the first time.
>
> But this code was not written with EFI mixed mode in mind (running a 64
> bit kernel on 32 bit EFI firmware), this causes the kernel to crash during
> early boot when running in mixed mode.
>
> The problem is that in mixed mode pointers are 64 bit, but when running on
> a 32 bit firmware, EFI calls which return a pointer value by reference only
> fill the lower 32 bits of the passed pointer, leaving the upper 32 bits
> uninitialized which leads to crashes.
>
> This commit fixes this by initializing pointers which are passed by
> reference to EFI calls to NULL before passing them, so that the upper 32
> bits are initialized to 0.
>
> Fixes: 0d95981438c3 ("x86: efi/random: Invoke EFI_RNG_PROTOCOL to seed the UEFI RNG table")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks Hans.

I'm a bit annoyed with myself since I should have been able to catch
this in my QEMU tests and I didn't

I'll queue this (and the next patch) as a fix


> ---
>  drivers/firmware/efi/libstub/random.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
> index 35edd7cfb6a1..97378cf96a2e 100644
> --- a/drivers/firmware/efi/libstub/random.c
> +++ b/drivers/firmware/efi/libstub/random.c
> @@ -33,7 +33,7 @@ efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table_arg,
>  {
>         efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
>         efi_status_t status;
> -       struct efi_rng_protocol *rng;
> +       struct efi_rng_protocol *rng = NULL;
>
>         status = efi_call_early(locate_protocol, &rng_proto, NULL,
>                                 (void **)&rng);
> @@ -162,8 +162,8 @@ efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg)
>         efi_guid_t rng_proto = EFI_RNG_PROTOCOL_GUID;
>         efi_guid_t rng_algo_raw = EFI_RNG_ALGORITHM_RAW;
>         efi_guid_t rng_table_guid = LINUX_EFI_RANDOM_SEED_TABLE_GUID;
> -       struct efi_rng_protocol *rng;
> -       struct linux_efi_random_seed *seed;
> +       struct efi_rng_protocol *rng = NULL;
> +       struct linux_efi_random_seed *seed = NULL;
>         efi_status_t status;
>
>         status = efi_call_early(locate_protocol, &rng_proto, NULL,
> --
> 2.23.0
>