Message ID | 1399717549-10961-1-git-send-email-pl@kamp.de |
---|---|
State | New |
Headers | show |
> if a saved vm has unknown flags in the memory data qemu > currently simply ignores this flag and continues which > yields in an unpredictable result. > > this patch catches all unknown flags and > aborts the loading of the vm. > > CC: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > arch_init.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > Reviewed-by: ChenLiang <chenliang0016@icloud.com>
Peter Lieven <pl@kamp.de> wrote: > if a saved vm has unknown flags in the memory data qemu > currently simply ignores this flag and continues which > yields in an unpredictable result. > > this patch catches all unknown flags and > aborts the loading of the vm. > > CC: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> ..... Once here, shouldn't be better to do this as: change do {} while () for while (true) {} > > @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) > } > } else if (flags & RAM_SAVE_FLAG_HOOK) { > ram_control_load_hook(f, flags); > + } else if (!(flags & RAM_SAVE_FLAG_EOS)) { > + ret = -EINVAL; > + goto done; > } > error = qemu_file_get_error(f); > if (error) { } else if (flags & RAM_SAVE_FLAG_HOOK) { ram_control_load_hook(f, flags); + } else if (flags & RAM_SAVE_FLAG_EOS) { + break; + } else { + ret = -EINVAL; + goto done; } error = qemu_file_get_error(f); if (error) { } This way, we are checking RAM_SAVE_FLAG_EOS the same way than any other flag? And we don't have to duplicate the FLAG_NAME? Unrelated to this patch, all the flags are a bitmap, but really, the ones that can be together are RAM_SAVE_FLAG_CONTINUE and the rest, all the others need to be alone. I am telling this because we have used already 8 flags, and we are using the low bits of offset to save the flags, we have 10 flags? Perhaps changing the last flag to mean that the low bits pass to be a counter? PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works with all of this. Later, Juan.
Am 12.05.2014 12:19, schrieb Juan Quintela: > Peter Lieven <pl@kamp.de> wrote: >> if a saved vm has unknown flags in the memory data qemu >> currently simply ignores this flag and continues which >> yields in an unpredictable result. >> >> this patch catches all unknown flags and >> aborts the loading of the vm. >> >> CC: qemu-stable@nongnu.org >> Signed-off-by: Peter Lieven <pl@kamp.de> > ..... > > Once here, shouldn't be better to do this as: > > change do {} while () for while (true) {} > >> >> @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) >> } >> } else if (flags & RAM_SAVE_FLAG_HOOK) { >> ram_control_load_hook(f, flags); >> + } else if (!(flags & RAM_SAVE_FLAG_EOS)) { >> + ret = -EINVAL; >> + goto done; >> } >> error = qemu_file_get_error(f); >> if (error) { > > } else if (flags & RAM_SAVE_FLAG_HOOK) { > ram_control_load_hook(f, flags); > + } else if (flags & RAM_SAVE_FLAG_EOS) { > + break; > + } else { > + ret = -EINVAL; > + goto done; > } > error = qemu_file_get_error(f); > if (error) { > } > > > This way, we are checking RAM_SAVE_FLAG_EOS the same way than any other > flag? And we don't have to duplicate the FLAG_NAME? Ok, I will send a v2. > > Unrelated to this patch, all the flags are a bitmap, but really, the > ones that can be together are RAM_SAVE_FLAG_CONTINUE and the rest, all > the others need to be alone. I am telling this because we have used > already 8 flags, and we are using the low bits of offset to save the > flags, we have 10 flags? Perhaps changing the last flag to mean that > the low bits pass to be a counter? Some better encoding would indeed be useful. I already thought that we might run out of flags soon. We have 11 flags I think, but there is not much space left. Reserving the last flag to indicate that the lower 10 bits a are counter might be a good option. Peter > > PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works > with all of this. > > Later, Juan. >
Am 12.05.2014 12:25, schrieb Peter Lieven: > Am 12.05.2014 12:19, schrieb Juan Quintela: >> Peter Lieven <pl@kamp.de> wrote: >>> if a saved vm has unknown flags in the memory data qemu >>> currently simply ignores this flag and continues which >>> yields in an unpredictable result. >>> >>> this patch catches all unknown flags and >>> aborts the loading of the vm. >>> >>> CC: qemu-stable@nongnu.org >>> Signed-off-by: Peter Lieven <pl@kamp.de> >> ..... >> >> Once here, shouldn't be better to do this as: >> >> change do {} while () for while (true) {} >> >>> >>> @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) >>> } >>> } else if (flags & RAM_SAVE_FLAG_HOOK) { >>> ram_control_load_hook(f, flags); >>> + } else if (!(flags & RAM_SAVE_FLAG_EOS)) { >>> + ret = -EINVAL; >>> + goto done; >>> } >>> error = qemu_file_get_error(f); >>> if (error) { >> } else if (flags & RAM_SAVE_FLAG_HOOK) { >> ram_control_load_hook(f, flags); >> + } else if (flags & RAM_SAVE_FLAG_EOS) { >> + break; >> + } else { >> + ret = -EINVAL; >> + goto done; >> } >> error = qemu_file_get_error(f); >> if (error) { we can also drop the error variable I think and change the loop to while (!ret) {} >> } >> >> >> This way, we are checking RAM_SAVE_FLAG_EOS the same way than any other >> flag? And we don't have to duplicate the FLAG_NAME? > Ok, I will send a v2. > >> Unrelated to this patch, all the flags are a bitmap, but really, the >> ones that can be together are RAM_SAVE_FLAG_CONTINUE and the rest, all >> the others need to be alone. I am telling this because we have used >> already 8 flags, and we are using the low bits of offset to save the >> flags, we have 10 flags? Perhaps changing the last flag to mean that >> the low bits pass to be a counter? > Some better encoding would indeed be useful. I already thought > that we might run out of flags soon. We have 11 flags I think, > but there is not much space left. Reserving the last flag to indicate > that the lower 10 bits a are counter might be a good option. > > Peter > >> PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works >> with all of this. >> >> Later, Juan. >>
diff --git a/arch_init.c b/arch_init.c index 995f56d..582b716 100644 --- a/arch_init.c +++ b/arch_init.c @@ -1084,9 +1084,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) total_ram_bytes -= length; } - } - - if (flags & RAM_SAVE_FLAG_COMPRESS) { + } else if (flags & RAM_SAVE_FLAG_COMPRESS) { void *host; uint8_t ch; @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) } } else if (flags & RAM_SAVE_FLAG_HOOK) { ram_control_load_hook(f, flags); + } else if (!(flags & RAM_SAVE_FLAG_EOS)) { + ret = -EINVAL; + goto done; } error = qemu_file_get_error(f); if (error) {
if a saved vm has unknown flags in the memory data qemu currently simply ignores this flag and continues which yields in an unpredictable result. this patch catches all unknown flags and aborts the loading of the vm. CC: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> --- arch_init.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)