qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] m68k: write bootinfo as rom section and re-randomize on reboot
@ 2022-09-29 18:39 Jason A. Donenfeld
  2022-10-02 10:37 ` [PATCH v2] " Jason A. Donenfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Jason A. Donenfeld @ 2022-09-29 18:39 UTC (permalink / raw)
  To: laurent, qemu-devel; +Cc: Jason A. Donenfeld, Geert Uytterhoeven

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. This assumes the
bootinfo block won't exceed 1k.

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     | 64 +++++++++++++++++++++++++++++++---------------
 hw/m68k/virt.c     | 44 ++++++++++++++++++++-----------
 3 files changed, 97 insertions(+), 59 deletions(-)

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index 897162b818..eb92937cf6 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 *)(((unsigned long)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 *)(((unsigned long)base + 3) & ~3); \
     } while (0)
 #endif
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index a4590c2cb0..1856ea7c2f 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,8 @@ static void q800_init(MachineState *machine)
     cs = CPU(cpu);
     if (linux_boot) {
         uint64_t high;
+        void *param_blob = g_malloc(1024), *param_ptr, *param_rng_seed;
+
         kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
                                &elf_entry, NULL, &high, NULL, 1,
                                EM_68K, 0, 0);
@@ -607,23 +624,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 +650,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 +672,18 @@ 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(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..655b0d50ab 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,7 @@ static void virt_init(MachineState *machine)
     if (kernel_filename) {
         CPUState *cs = CPU(cpu);
         uint64_t high;
+        void *param_blob = g_malloc(1024), *param_ptr, *param_rng_seed;
 
         kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
                                &elf_entry, NULL, &high, NULL, 1,
@@ -222,35 +229,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 +274,18 @@ 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(parameters_base, param_ptr - param_blob) +
+                               (param_rng_seed - param_blob);
+        g_free(param_blob);
     }
 }
 
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-09-29 18:39 [PATCH] m68k: write bootinfo as rom section and re-randomize on reboot Jason A. Donenfeld
@ 2022-10-02 10:37 ` Jason A. Donenfeld
  2022-10-03 11:02   ` [PATCH v3] " Jason A. Donenfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Jason A. Donenfeld @ 2022-10-02 10:37 UTC (permalink / raw)
  To: laurent, qemu-devel; +Cc: Jason A. Donenfeld, Geert Uytterhoeven

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. This assumes the
bootinfo block won't exceed 1k.

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     | 65 ++++++++++++++++++++++++++++++++--------------
 hw/m68k/virt.c     | 45 +++++++++++++++++++++-----------
 3 files changed, 99 insertions(+), 59 deletions(-)

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index 897162b818..eb92937cf6 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 *)(((unsigned long)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 *)(((unsigned long)base + 3) & ~3); \
     } while (0)
 #endif
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index a4590c2cb0..d94da38cde 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,8 @@ static void q800_init(MachineState *machine)
     cs = CPU(cpu);
     if (linux_boot) {
         uint64_t high;
+        void *param_blob = g_malloc(1024), *param_ptr, *param_rng_seed;
+
         kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
                                &elf_entry, NULL, &high, NULL, 1,
                                EM_68K, 0, 0);
@@ -607,23 +624,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 +650,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 +672,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..1b9771442d 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,7 @@ static void virt_init(MachineState *machine)
     if (kernel_filename) {
         CPUState *cs = CPU(cpu);
         uint64_t high;
+        void *param_blob = g_malloc(1024), *param_ptr, *param_rng_seed;
 
         kernel_size = load_elf(kernel_filename, NULL, NULL, NULL,
                                &elf_entry, NULL, &high, NULL, 1,
@@ -222,35 +229,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 +274,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);
     }
 }
 
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-10-02 10:37 ` [PATCH v2] " Jason A. Donenfeld
@ 2022-10-03 11:02   ` Jason A. Donenfeld
  2022-10-11  8:22     ` Laurent Vivier
  0 siblings, 1 reply; 7+ messages in thread
From: Jason A. Donenfeld @ 2022-10-03 11:02 UTC (permalink / raw)
  To: laurent, qemu-devel; +Cc: Jason A. Donenfeld, Geert Uytterhoeven

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(-)

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index 897162b818..eb92937cf6 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 *)(((unsigned long)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 *)(((unsigned long)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);
     }
 }
 
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-10-03 11:02   ` [PATCH v3] " Jason A. Donenfeld
@ 2022-10-11  8:22     ` Laurent Vivier
  2022-10-11  9:29       ` Peter Maydell
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2022-10-11  8:22 UTC (permalink / raw)
  To: Jason A. Donenfeld, qemu-devel; +Cc: Geert Uytterhoeven

Le 03/10/2022 à 13:02, 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.

As it's needed to be refreshed, I think it would better not to use a ROM and to regenerate all the 
bootinfo data on the reset. This will also avoid the conditional g_malloc().

Thanks,
Laurent

> 
> 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(-)
> 
> diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
> index 897162b818..eb92937cf6 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 *)(((unsigned long)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 *)(((unsigned long)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);
>       }
>   }
>   



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-10-11  8:22     ` Laurent Vivier
@ 2022-10-11  9:29       ` Peter Maydell
  2022-10-11 14:56         ` Jason A. Donenfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2022-10-11  9:29 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Jason A. Donenfeld, qemu-devel, Geert Uytterhoeven

On Tue, 11 Oct 2022 at 09:41, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 03/10/2022 à 13:02, 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.
>
> As it's needed to be refreshed, I think it would better not to use a ROM and to regenerate all the
> bootinfo data on the reset.

I quite liked the use of a rom blob in this patch -- it gets rid
of a lot of direct stl_phys() calls (which is a semi-deprecated
API because it ignores the possibility of failure).

-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-10-11  9:29       ` Peter Maydell
@ 2022-10-11 14:56         ` Jason A. Donenfeld
  2022-10-11 21:03           ` Laurent Vivier
  0 siblings, 1 reply; 7+ messages in thread
From: Jason A. Donenfeld @ 2022-10-11 14:56 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Laurent Vivier, qemu-devel, Geert Uytterhoeven

On Tue, Oct 11, 2022 at 10:29:45AM +0100, Peter Maydell wrote:
> On Tue, 11 Oct 2022 at 09:41, Laurent Vivier <laurent@vivier.eu> wrote:
> >
> > Le 03/10/2022 à 13:02, 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.
> >
> > As it's needed to be refreshed, I think it would better not to use a ROM and to regenerate all the
> > bootinfo data on the reset.
> 
> I quite liked the use of a rom blob in this patch -- it gets rid
> of a lot of direct stl_phys() calls (which is a semi-deprecated
> API because it ignores the possibility of failure).

A ROM is also how other archs do it. I'm good either way though.
Laurent/Peter - can you guys decide something and let me know if I need
a v+1 that avoids the ROM, or if you'll go with this v3 that uses the
ROM? Just make a decision, and I'll follow it.

Jason


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] m68k: write bootinfo as rom section and re-randomize on reboot
  2022-10-11 14:56         ` Jason A. Donenfeld
@ 2022-10-11 21:03           ` Laurent Vivier
  0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2022-10-11 21:03 UTC (permalink / raw)
  To: Jason A. Donenfeld, Peter Maydell; +Cc: qemu-devel, Geert Uytterhoeven

Le 11/10/2022 à 16:56, Jason A. Donenfeld a écrit :
> On Tue, Oct 11, 2022 at 10:29:45AM +0100, Peter Maydell wrote:
>> On Tue, 11 Oct 2022 at 09:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>>
>>> Le 03/10/2022 à 13:02, 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.
>>>
>>> As it's needed to be refreshed, I think it would better not to use a ROM and to regenerate all the
>>> bootinfo data on the reset.
>>
>> I quite liked the use of a rom blob in this patch -- it gets rid
>> of a lot of direct stl_phys() calls (which is a semi-deprecated
>> API because it ignores the possibility of failure).
> 
> A ROM is also how other archs do it. I'm good either way though.
> Laurent/Peter - can you guys decide something and let me know if I need
> a v+1 that avoids the ROM, or if you'll go with this v3 that uses the
> ROM? Just make a decision, and I'll follow it.
> 

If Peter likes it, it's ok.

Applied to my m68k-for-7.2 branch

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-10-11 21:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 18:39 [PATCH] m68k: write bootinfo as rom section and re-randomize on reboot Jason A. Donenfeld
2022-10-02 10:37 ` [PATCH v2] " Jason A. Donenfeld
2022-10-03 11:02   ` [PATCH v3] " Jason A. Donenfeld
2022-10-11  8:22     ` Laurent Vivier
2022-10-11  9:29       ` Peter Maydell
2022-10-11 14:56         ` Jason A. Donenfeld
2022-10-11 21:03           ` Laurent Vivier

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).