Il lun 10 mar 2025, 23:18 Alistair Francis <[email protected]> ha scritto:
> On Tue, Mar 11, 2025 at 3:34 AM Paolo Bonzini <[email protected]> wrote: > > > > On Fri, Mar 7, 2025 at 1:45 AM Alistair Francis <[email protected]> > wrote: > > > I'm not convinced that this is the thing that we should be checking > > > for. If someone can corrupt the migration data for an attack there are > > > better things to change then the MXL > > > > In principle you could have code that uses 2 << MXL to compute the > > size of a memcpy, or something like that... never say never. :) > > But couldn't they also change the PC at that point and execute > malicious code? Or MTVEC or anything else? > The point is exploiting the host, not the guest. It just seems like this check doesn't actually provide any security. In the future we will want to merge misa_mxl and misa_mxl_max and this > patch just makes that a little bit harder, for no real gains that I > can see. > > > Do you prefer turning all the priv_ver, vext_ver, misa_mxl, > > misa_ext_mask fields into VMSTATE_UNUSED? That would also be okay. > > I think we do want to migrate them, they contain important information. > They are constant though, aren't they? Properties aren't migrated, they are set on the command line of the destination. I will drop the patch, but I think there's some work to do on the migration of RISC-V CPU state. Paolo > Alistair > > > > > I would also add a check for misa_ext against misa_ext_mask and > > riscv_cpu_validate_set_extensions(). > > > > Paolo > > > > > Alistair > > > > > > > > > > > Paolo > > > > > > > > > Alistair > > > > > > > > > >> would have a snowball effect on, for example, the valid VM modes. > > > > >> > > > > >> Signed-off-by: Paolo Bonzini <[email protected]> > > > > >> --- > > > > >> target/riscv/machine.c | 13 +++++++++++++ > > > > >> 1 file changed, 13 insertions(+) > > > > >> > > > > >> diff --git a/target/riscv/machine.c b/target/riscv/machine.c > > > > >> index d8445244ab2..c3d8e7c4005 100644 > > > > >> --- a/target/riscv/machine.c > > > > >> +++ b/target/riscv/machine.c > > > > >> @@ -375,6 +375,18 @@ static const VMStateDescription vmstate_ssp > = { > > > > >> } > > > > >> }; > > > > >> > > > > >> +static bool riscv_validate_misa_mxl(void *opaque, int version_id) > > > > >> +{ > > > > >> + RISCVCPU *cpu = RISCV_CPU(opaque); > > > > >> + CPURISCVState *env = &cpu->env; > > > > >> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu); > > > > >> + uint32_t misa_mxl_saved = env->misa_mxl; > > > > >> + > > > > >> + /* Preserve misa_mxl even if the migration stream corrupted > it */ > > > > >> + env->misa_mxl = mcc->misa_mxl_max; > > > > >> + return misa_mxl_saved == mcc->misa_mxl_max; > > > > >> +} > > > > >> + > > > > >> const VMStateDescription vmstate_riscv_cpu = { > > > > >> .name = "cpu", > > > > >> .version_id = 10, > > > > >> @@ -394,6 +406,7 @@ const VMStateDescription vmstate_riscv_cpu = { > > > > >> VMSTATE_UINTTL(env.priv_ver, RISCVCPU), > > > > >> VMSTATE_UINTTL(env.vext_ver, RISCVCPU), > > > > >> VMSTATE_UINT32(env.misa_mxl, RISCVCPU), > > > > >> + VMSTATE_VALIDATE("MXL must match", > riscv_validate_misa_mxl), > > > > >> VMSTATE_UINT32(env.misa_ext, RISCVCPU), > > > > >> VMSTATE_UNUSED(4), > > > > >> VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU), > > > > >> -- > > > > >> 2.48.1 > > > > >> > > > > >> > > > > > > > > > > > > > > > > > > > > >
