From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42366)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WjnKN-0007Gt-Nz
	for qemu-devel@nongnu.org; Mon, 12 May 2014 06:19:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WjnKI-0007lW-UN
	for qemu-devel@nongnu.org; Mon, 12 May 2014 06:19:23 -0400
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1399717549-10961-1-git-send-email-pl@kamp.de> (Peter Lieven's
	message of "Sat, 10 May 2014 12:25:49 +0200")
References: <1399717549-10961-1-git-send-email-pl@kamp.de>
Date: Mon, 12 May 2014 12:19:11 +0200
Message-ID: <87lhu78fkw.fsf@elfo.mitica>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH] migration: catch unknown flags in ram_load
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Lieven <pl@kamp.de>
Cc: pbonzini@redhat.com, qemu-stable@nongnu.org, qemu-devel@nongnu.org, dgilbert@redhat.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.