From: Laurent Vivier <laurent@vivier.eu>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>, qemu-devel@nongnu.org
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v4] m68k: write bootinfo as rom section and re-randomize on reboot
Date: Fri, 21 Oct 2022 21:25:18 +0200 [thread overview]
Message-ID: <e24ce881-2f0b-860d-05e5-a9a8a7dda3c8@vivier.eu> (raw)
In-Reply-To: <20221017202952.60762-1-Jason@zx2c4.com>
Le 17/10/2022 à 22:29, Jason A. Donenfeld a écrit :
> Rather than poking directly into RAM, add the bootinfo block as a proper
> ROM, so that it's restored when rebooting the system. This way, if the
> guest corrupts any of the bootinfo items, but then tries to reboot,
> it'll still be restored back to normal as expected.
>
> Then, since the RNG seed needs to be fresh on each boot, regenerate the
> RNG seed in the ROM when reseting the CPU.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> hw/m68k/bootinfo.h | 48 +++++++++++++++----------------
> hw/m68k/q800.c | 71 +++++++++++++++++++++++++++++++++-------------
> hw/m68k/virt.c | 51 +++++++++++++++++++++++----------
> 3 files changed, 111 insertions(+), 59 deletions(-)
Notes:
- don't send your patch as a reply to a previous version
- add an history:
v4: replace (void *)(((unsigned long)base + 3) & ~3) by
(void *)(((uintptr_t)base + 3) & ~3);
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Thanks,
Laurent
> diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
> index 897162b818..67e6b66b7d 100644
> --- a/hw/m68k/bootinfo.h
> +++ b/hw/m68k/bootinfo.h
> @@ -12,66 +12,66 @@
> #ifndef HW_M68K_BOOTINFO_H
> #define HW_M68K_BOOTINFO_H
>
> -#define BOOTINFO0(as, base, id) \
> +#define BOOTINFO0(base, id) \
> do { \
> - stw_phys(as, base, id); \
> + stw_p(base, id); \
> base += 2; \
> - stw_phys(as, base, sizeof(struct bi_record)); \
> + stw_p(base, sizeof(struct bi_record)); \
> base += 2; \
> } while (0)
>
> -#define BOOTINFO1(as, base, id, value) \
> +#define BOOTINFO1(base, id, value) \
> do { \
> - stw_phys(as, base, id); \
> + stw_p(base, id); \
> base += 2; \
> - stw_phys(as, base, sizeof(struct bi_record) + 4); \
> + stw_p(base, sizeof(struct bi_record) + 4); \
> base += 2; \
> - stl_phys(as, base, value); \
> + stl_p(base, value); \
> base += 4; \
> } while (0)
>
> -#define BOOTINFO2(as, base, id, value1, value2) \
> +#define BOOTINFO2(base, id, value1, value2) \
> do { \
> - stw_phys(as, base, id); \
> + stw_p(base, id); \
> base += 2; \
> - stw_phys(as, base, sizeof(struct bi_record) + 8); \
> + stw_p(base, sizeof(struct bi_record) + 8); \
> base += 2; \
> - stl_phys(as, base, value1); \
> + stl_p(base, value1); \
> base += 4; \
> - stl_phys(as, base, value2); \
> + stl_p(base, value2); \
> base += 4; \
> } while (0)
>
> -#define BOOTINFOSTR(as, base, id, string) \
> +#define BOOTINFOSTR(base, id, string) \
> do { \
> int i; \
> - stw_phys(as, base, id); \
> + stw_p(base, id); \
> base += 2; \
> - stw_phys(as, base, \
> + stw_p(base, \
> (sizeof(struct bi_record) + strlen(string) + \
> 1 /* null termination */ + 3 /* padding */) & ~3); \
> base += 2; \
> for (i = 0; string[i]; i++) { \
> - stb_phys(as, base++, string[i]); \
> + stb_p(base++, string[i]); \
> } \
> - stb_phys(as, base++, 0); \
> - base = (base + 3) & ~3; \
> + stb_p(base++, 0); \
> + base = (void *)(((uintptr_t)base + 3) & ~3); \
> } while (0)
>
> -#define BOOTINFODATA(as, base, id, data, len) \
> +#define BOOTINFODATA(base, id, data, len) \
> do { \
> int i; \
> - stw_phys(as, base, id); \
> + stw_p(base, id); \
> base += 2; \
> - stw_phys(as, base, \
> + stw_p(base, \
> (sizeof(struct bi_record) + len + \
> 2 /* length field */ + 3 /* padding */) & ~3); \
> base += 2; \
> - stw_phys(as, base, len); \
> + stw_p(base, len); \
> base += 2; \
> for (i = 0; i < len; ++i) { \
> - stb_phys(as, base++, data[i]); \
> + stb_p(base++, data[i]); \
> } \
> - base = (base + 3) & ~3; \
> + base = (void *)(((uintptr_t)base + 3) & ~3); \
> } while (0)
> #endif
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index a4590c2cb0..e09e244ddc 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -321,11 +321,22 @@ static const TypeInfo glue_info = {
> },
> };
>
> +typedef struct {
> + M68kCPU *cpu;
> + struct bi_record *rng_seed;
> +} ResetInfo;
> +
> static void main_cpu_reset(void *opaque)
> {
> - M68kCPU *cpu = opaque;
> + ResetInfo *reset_info = opaque;
> + M68kCPU *cpu = reset_info->cpu;
> CPUState *cs = CPU(cpu);
>
> + if (reset_info->rng_seed) {
> + qemu_guest_getrandom_nofail((void *)reset_info->rng_seed->data + 2,
> + be16_to_cpu(*(uint16_t *)reset_info->rng_seed->data));
> + }
> +
> cpu_reset(cs);
> cpu->env.aregs[7] = ldl_phys(cs->as, 0);
> cpu->env.pc = ldl_phys(cs->as, 4);
> @@ -386,6 +397,7 @@ static void q800_init(MachineState *machine)
> NubusBus *nubus;
> DeviceState *glue;
> DriveInfo *dinfo;
> + ResetInfo *reset_info;
> uint8_t rng_seed[32];
>
> linux_boot = (kernel_filename != NULL);
> @@ -396,9 +408,12 @@ static void q800_init(MachineState *machine)
> exit(1);
> }
>
> + reset_info = g_new0(ResetInfo, 1);
> +
> /* init CPUs */
> cpu = M68K_CPU(cpu_create(machine->cpu_type));
> - qemu_register_reset(main_cpu_reset, cpu);
> + reset_info->cpu = cpu;
> + qemu_register_reset(main_cpu_reset, reset_info);
>
> /* RAM */
> memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -598,6 +613,14 @@ static void q800_init(MachineState *machine)
> cs = CPU(cpu);
> if (linux_boot) {
> uint64_t high;
> + void *param_blob, *param_ptr, *param_rng_seed;
> +
> + if (kernel_cmdline) {
> + param_blob = g_malloc(strlen(kernel_cmdline) + 1024);
> + } else {
> + param_blob = g_malloc(1024);
> + }
> +
> kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
> &elf_entry, NULL, &high, NULL, 1,
> EM_68K, 0, 0);
> @@ -607,23 +630,24 @@ static void q800_init(MachineState *machine)
> }
> stl_phys(cs->as, 4, elf_entry); /* reset initial PC */
> parameters_base = (high + 1) & ~1;
> -
> - BOOTINFO1(cs->as, parameters_base, BI_MACHTYPE, MACH_MAC);
> - BOOTINFO1(cs->as, parameters_base, BI_FPUTYPE, FPU_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_MMUTYPE, MMU_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_CPUTYPE, CPU_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_CPUID, CPUB_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_MODEL, MAC_MODEL_Q800);
> - BOOTINFO1(cs->as, parameters_base,
> + param_ptr = param_blob;
> +
> + BOOTINFO1(param_ptr, BI_MACHTYPE, MACH_MAC);
> + BOOTINFO1(param_ptr, BI_FPUTYPE, FPU_68040);
> + BOOTINFO1(param_ptr, BI_MMUTYPE, MMU_68040);
> + BOOTINFO1(param_ptr, BI_CPUTYPE, CPU_68040);
> + BOOTINFO1(param_ptr, BI_MAC_CPUID, CPUB_68040);
> + BOOTINFO1(param_ptr, BI_MAC_MODEL, MAC_MODEL_Q800);
> + BOOTINFO1(param_ptr,
> BI_MAC_MEMSIZE, ram_size >> 20); /* in MB */
> - BOOTINFO2(cs->as, parameters_base, BI_MEMCHUNK, 0, ram_size);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_VADDR,
> + BOOTINFO2(param_ptr, BI_MEMCHUNK, 0, ram_size);
> + BOOTINFO1(param_ptr, BI_MAC_VADDR,
> VIDEO_BASE + macfb_mode->offset);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_VDEPTH, graphic_depth);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_VDIM,
> + BOOTINFO1(param_ptr, BI_MAC_VDEPTH, graphic_depth);
> + BOOTINFO1(param_ptr, BI_MAC_VDIM,
> (graphic_height << 16) | graphic_width);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_VROW, macfb_mode->stride);
> - BOOTINFO1(cs->as, parameters_base, BI_MAC_SCCBASE, SCC_BASE);
> + BOOTINFO1(param_ptr, BI_MAC_VROW, macfb_mode->stride);
> + BOOTINFO1(param_ptr, BI_MAC_SCCBASE, SCC_BASE);
>
> rom = g_malloc(sizeof(*rom));
> memory_region_init_ram_ptr(rom, NULL, "m68k_fake_mac.rom",
> @@ -632,13 +656,14 @@ static void q800_init(MachineState *machine)
> memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
>
> if (kernel_cmdline) {
> - BOOTINFOSTR(cs->as, parameters_base, BI_COMMAND_LINE,
> + BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
> kernel_cmdline);
> }
>
> /* Pass seed to RNG. */
> + param_rng_seed = param_ptr;
> qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> - BOOTINFODATA(cs->as, parameters_base, BI_RNG_SEED,
> + BOOTINFODATA(param_ptr, BI_RNG_SEED,
> rng_seed, sizeof(rng_seed));
>
> /* load initrd */
> @@ -653,13 +678,19 @@ static void q800_init(MachineState *machine)
> initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
> load_image_targphys(initrd_filename, initrd_base,
> ram_size - initrd_base);
> - BOOTINFO2(cs->as, parameters_base, BI_RAMDISK, initrd_base,
> + BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
> initrd_size);
> } else {
> initrd_base = 0;
> initrd_size = 0;
> }
> - BOOTINFO0(cs->as, parameters_base, BI_LAST);
> + BOOTINFO0(param_ptr, BI_LAST);
> + rom_add_blob_fixed_as("bootinfo", param_blob, param_ptr - param_blob,
> + parameters_base, cs->as);
> + reset_info->rng_seed = rom_ptr_for_as(cs->as, parameters_base,
> + param_ptr - param_blob) +
> + (param_rng_seed - param_blob);
> + g_free(param_blob);
> } else {
> uint8_t *ptr;
> /* allocate and load BIOS */
> diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
> index f7b903ea1b..89c4108eb5 100644
> --- a/hw/m68k/virt.c
> +++ b/hw/m68k/virt.c
> @@ -89,6 +89,7 @@ typedef struct {
> M68kCPU *cpu;
> hwaddr initial_pc;
> hwaddr initial_stack;
> + struct bi_record *rng_seed;
> } ResetInfo;
>
> static void main_cpu_reset(void *opaque)
> @@ -97,6 +98,11 @@ static void main_cpu_reset(void *opaque)
> M68kCPU *cpu = reset_info->cpu;
> CPUState *cs = CPU(cpu);
>
> + if (reset_info->rng_seed) {
> + qemu_guest_getrandom_nofail((void *)reset_info->rng_seed->data + 2,
> + be16_to_cpu(*(uint16_t *)reset_info->rng_seed->data));
> + }
> +
> cpu_reset(cs);
> cpu->env.aregs[7] = reset_info->initial_stack;
> cpu->env.pc = reset_info->initial_pc;
> @@ -212,6 +218,13 @@ static void virt_init(MachineState *machine)
> if (kernel_filename) {
> CPUState *cs = CPU(cpu);
> uint64_t high;
> + void *param_blob, *param_ptr, *param_rng_seed;
> +
> + if (kernel_cmdline) {
> + param_blob = g_malloc(strlen(kernel_cmdline) + 1024);
> + } else {
> + param_blob = g_malloc(1024);
> + }
>
> kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
> &elf_entry, NULL, &high, NULL, 1,
> @@ -222,35 +235,37 @@ static void virt_init(MachineState *machine)
> }
> reset_info->initial_pc = elf_entry;
> parameters_base = (high + 1) & ~1;
> + param_ptr = param_blob;
>
> - BOOTINFO1(cs->as, parameters_base, BI_MACHTYPE, MACH_VIRT);
> - BOOTINFO1(cs->as, parameters_base, BI_FPUTYPE, FPU_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_MMUTYPE, MMU_68040);
> - BOOTINFO1(cs->as, parameters_base, BI_CPUTYPE, CPU_68040);
> - BOOTINFO2(cs->as, parameters_base, BI_MEMCHUNK, 0, ram_size);
> + BOOTINFO1(param_ptr, BI_MACHTYPE, MACH_VIRT);
> + BOOTINFO1(param_ptr, BI_FPUTYPE, FPU_68040);
> + BOOTINFO1(param_ptr, BI_MMUTYPE, MMU_68040);
> + BOOTINFO1(param_ptr, BI_CPUTYPE, CPU_68040);
> + BOOTINFO2(param_ptr, BI_MEMCHUNK, 0, ram_size);
>
> - BOOTINFO1(cs->as, parameters_base, BI_VIRT_QEMU_VERSION,
> + BOOTINFO1(param_ptr, BI_VIRT_QEMU_VERSION,
> ((QEMU_VERSION_MAJOR << 24) | (QEMU_VERSION_MINOR << 16) |
> (QEMU_VERSION_MICRO << 8)));
> - BOOTINFO2(cs->as, parameters_base, BI_VIRT_GF_PIC_BASE,
> + BOOTINFO2(param_ptr, BI_VIRT_GF_PIC_BASE,
> VIRT_GF_PIC_MMIO_BASE, VIRT_GF_PIC_IRQ_BASE);
> - BOOTINFO2(cs->as, parameters_base, BI_VIRT_GF_RTC_BASE,
> + BOOTINFO2(param_ptr, BI_VIRT_GF_RTC_BASE,
> VIRT_GF_RTC_MMIO_BASE, VIRT_GF_RTC_IRQ_BASE);
> - BOOTINFO2(cs->as, parameters_base, BI_VIRT_GF_TTY_BASE,
> + BOOTINFO2(param_ptr, BI_VIRT_GF_TTY_BASE,
> VIRT_GF_TTY_MMIO_BASE, VIRT_GF_TTY_IRQ_BASE);
> - BOOTINFO2(cs->as, parameters_base, BI_VIRT_CTRL_BASE,
> + BOOTINFO2(param_ptr, BI_VIRT_CTRL_BASE,
> VIRT_CTRL_MMIO_BASE, VIRT_CTRL_IRQ_BASE);
> - BOOTINFO2(cs->as, parameters_base, BI_VIRT_VIRTIO_BASE,
> + BOOTINFO2(param_ptr, BI_VIRT_VIRTIO_BASE,
> VIRT_VIRTIO_MMIO_BASE, VIRT_VIRTIO_IRQ_BASE);
>
> if (kernel_cmdline) {
> - BOOTINFOSTR(cs->as, parameters_base, BI_COMMAND_LINE,
> + BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
> kernel_cmdline);
> }
>
> /* Pass seed to RNG. */
> + param_rng_seed = param_ptr;
> qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
> - BOOTINFODATA(cs->as, parameters_base, BI_RNG_SEED,
> + BOOTINFODATA(param_ptr, BI_RNG_SEED,
> rng_seed, sizeof(rng_seed));
>
> /* load initrd */
> @@ -265,13 +280,19 @@ static void virt_init(MachineState *machine)
> initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
> load_image_targphys(initrd_filename, initrd_base,
> ram_size - initrd_base);
> - BOOTINFO2(cs->as, parameters_base, BI_RAMDISK, initrd_base,
> + BOOTINFO2(param_ptr, BI_RAMDISK, initrd_base,
> initrd_size);
> } else {
> initrd_base = 0;
> initrd_size = 0;
> }
> - BOOTINFO0(cs->as, parameters_base, BI_LAST);
> + BOOTINFO0(param_ptr, BI_LAST);
> + rom_add_blob_fixed_as("bootinfo", param_blob, param_ptr - param_blob,
> + parameters_base, cs->as);
> + reset_info->rng_seed = rom_ptr_for_as(cs->as, parameters_base,
> + param_ptr - param_blob) +
> + (param_rng_seed - param_blob);
> + g_free(param_blob);
> }
> }
>
next prev parent reply other threads:[~2022-10-21 22:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-14 7:23 [PULL 0/2] M68k for 7.2 patches Laurent Vivier
2022-10-14 7:23 ` [PULL 1/2] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED Laurent Vivier
2022-10-14 7:23 ` [PULL 2/2] m68k: write bootinfo as rom section and re-randomize on reboot Laurent Vivier
2022-10-16 19:50 ` [PULL 0/2] M68k for 7.2 patches Stefan Hajnoczi
2022-10-17 3:56 ` Jason A. Donenfeld via
2022-10-17 7:25 ` Laurent Vivier
2022-10-17 20:29 ` [PATCH v4] m68k: write bootinfo as rom section and re-randomize on reboot Jason A. Donenfeld
2022-10-21 19:25 ` Laurent Vivier [this message]
2022-10-22 6:41 ` Richard Henderson
2022-10-23 19:09 ` Jason A. Donenfeld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e24ce881-2f0b-860d-05e5-a9a8a7dda3c8@vivier.eu \
--to=laurent@vivier.eu \
--cc=Jason@zx2c4.com \
--cc=geert@linux-m68k.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).