[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/arm/virt: Add DT property /secure-chosen/kaslr-seed
From: |
Peter Maydell |
Subject: |
Re: [PATCH] hw/arm/virt: Add DT property /secure-chosen/kaslr-seed |
Date: |
Fri, 17 Apr 2020 11:18:21 +0100 |
On Fri, 10 Apr 2020 at 18:02, Jerome Forissier <address@hidden> wrote:
>
> This commit generates a random seed to be used by the secure OS for
> ASLR when the machine is secure. The seed is a 64-bit random value
> exported via the DT in /secure-chosen/kaslr-seed. This interface is
> used by OP-TEE [1].
>
> [1] https://github.com/OP-TEE/optee_os/commit/ef262691fe0e
The kernel devicetree documentation documents this as a generic
property of /chosen -- should we be providing a /chosen/kaslr-seed
too ?
> Signed-off-by: Jerome Forissier <address@hidden>
> ---
> +static void create_secure_kaslr_seed(VirtMachineState *vms)
> +{
> + Error *err = NULL;
> + uint64_t seed;
> +
> + if (qcrypto_random_bytes(&seed, sizeof(seed), &err)) {
> + error_free(err);
> + return;
> + }
Since this is exposed to the guest I'm wondering if we should
use qemu_guest_getrandom() (which lets you make the randomness
deterministic for the benefit of record-and-replay). But I'm
not sure if that function is usable before the guest has even
started running. Pavel, could you answer that?
> + qemu_fdt_setprop_u64(vms->fdt, "/secure-chosen", "kaslr-seed", seed);
> +}
> +
> static void machvirt_init(MachineState *machine)
> {
> VirtMachineState *vms = VIRT_MACHINE(machine);
> @@ -1837,6 +1850,7 @@ static void machvirt_init(MachineState *machine)
> if (vms->secure) {
> create_secure_ram(vms, secure_sysmem);
> create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1));
> + create_secure_kaslr_seed(vms);
This is implicitly relying on create_uart() having created
the "/secure-chosen" node. I think it would be better now
that we have multiple things we might want to put there if we
just pulled the "create /secure-chosen" code out to the
create_fdt() function so we do it at the same place we
create "/chosen". (You can do that as a simple patch of its own
that comes before this one in the patchseries.)
> }
>
> vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
thanks
-- PMM