From patchwork Wed Jun 3 12:05:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juan Quintela X-Patchwork-Id: 479917 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 514AE140213 for ; Wed, 3 Jun 2015 22:14:27 +1000 (AEST) Received: from localhost ([::1]:35126 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07Yv-0003kV-C5 for incoming@patchwork.ozlabs.org; Wed, 03 Jun 2015 08:14:25 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07RI-0007K4-Qv for qemu-devel@nongnu.org; Wed, 03 Jun 2015 08:06:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z07RH-0004Do-BI for qemu-devel@nongnu.org; Wed, 03 Jun 2015 08:06:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07RH-0004CP-4K for qemu-devel@nongnu.org; Wed, 03 Jun 2015 08:06:31 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id D51A9347A7D for ; Wed, 3 Jun 2015 12:06:30 +0000 (UTC) Received: from trasno.mitica (ovpn-116-79.ams2.redhat.com [10.36.116.79]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t53C5w7M004305; Wed, 3 Jun 2015 08:06:29 -0400 From: Juan Quintela To: qemu-devel@nongnu.org Date: Wed, 3 Jun 2015 14:05:54 +0200 Message-Id: <1433333157-9939-19-git-send-email-quintela@redhat.com> In-Reply-To: <1433333157-9939-1-git-send-email-quintela@redhat.com> References: <1433333157-9939-1-git-send-email-quintela@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: amit.shah@redhat.com, "Dr. David Alan Gilbert" Subject: [Qemu-devel] [PULL 18/21] Add a protective section footer X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: "Dr. David Alan Gilbert" Badly formatted migration streams can go undetected or produce misleading errors due to a lock of checking at the end of sections. In particular a section that adds an extra 0x00 at the end causes what looks like a normal end of stream and thus doesn't produce any errors, and something that ends in a 0x01..0x04 kind of look like real section headers and then fail when the section parser tries to figure out which section they are. This is made worse by the choice of 0x00..0x04 being small numbers that are particularly common in normal section data. This patch adds a section footer consisting of a marker (0x7e - ~) followed by the section-id that was also sent in the header. If they mismatch then it throws an error explaining which section was being loaded. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- include/migration/migration.h | 1 + migration/savevm.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/include/migration/migration.h b/include/migration/migration.h index 7bdaf55..9387c8c 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -34,6 +34,7 @@ #define QEMU_VM_SECTION_FULL 0x04 #define QEMU_VM_SUBSECTION 0x05 #define QEMU_VM_VMDESCRIPTION 0x06 +#define QEMU_VM_SECTION_FOOTER 0x7e struct MigrationParams { bool blk; diff --git a/migration/savevm.c b/migration/savevm.c index 80c4389..2091882 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -639,6 +639,53 @@ static void save_section_header(QEMUFile *f, SaveStateEntry *se, } } +/* + * Write a footer onto device sections that catches cases misformatted device + * sections. + */ +static void save_section_footer(QEMUFile *f, SaveStateEntry *se) +{ + if (!skip_section_footers) { + qemu_put_byte(f, QEMU_VM_SECTION_FOOTER); + qemu_put_be32(f, se->section_id); + } +} + +/* + * Read a footer off the wire and check that it matches the expected section + * + * Returns: true if the footer was good + * false if there is a problem (and calls error_report to say why) + */ +static bool check_section_footer(QEMUFile *f, SaveStateEntry *se) +{ + uint8_t read_mark; + uint32_t read_section_id; + + if (skip_section_footers) { + /* No footer to check */ + return true; + } + + read_mark = qemu_get_byte(f); + + if (read_mark != QEMU_VM_SECTION_FOOTER) { + error_report("Missing section footer for %s", se->idstr); + return false; + } + + read_section_id = qemu_get_be32(f); + if (read_section_id != se->section_id) { + error_report("Mismatched section id in footer for %s -" + " read 0x%x expected 0x%x", + se->idstr, read_section_id, se->section_id); + return false; + } + + /* All good */ + return true; +} + bool qemu_savevm_state_blocked(Error **errp) { SaveStateEntry *se; @@ -686,6 +733,7 @@ void qemu_savevm_state_begin(QEMUFile *f, save_section_header(f, se, QEMU_VM_SECTION_START); ret = se->ops->save_live_setup(f, se->opaque); + save_section_footer(f, se); if (ret < 0) { qemu_file_set_error(f, ret); break; @@ -723,6 +771,7 @@ int qemu_savevm_state_iterate(QEMUFile *f) ret = se->ops->save_live_iterate(f, se->opaque); trace_savevm_section_end(se->idstr, se->section_id, ret); + save_section_footer(f, se); if (ret < 0) { qemu_file_set_error(f, ret); @@ -770,6 +819,7 @@ void qemu_savevm_state_complete(QEMUFile *f) ret = se->ops->save_live_complete(f, se->opaque); trace_savevm_section_end(se->idstr, se->section_id, ret); + save_section_footer(f, se); if (ret < 0) { qemu_file_set_error(f, ret); return; @@ -796,6 +846,7 @@ void qemu_savevm_state_complete(QEMUFile *f) json_end_object(vmdesc); trace_savevm_section_end(se->idstr, se->section_id, 0); + save_section_footer(f, se); } qemu_put_byte(f, QEMU_VM_EOF); @@ -900,6 +951,8 @@ static int qemu_save_device_state(QEMUFile *f) save_section_header(f, se, QEMU_VM_SECTION_FULL); vmstate_save(f, se, NULL); + + save_section_footer(f, se); } qemu_put_byte(f, QEMU_VM_EOF); @@ -1027,6 +1080,10 @@ int qemu_loadvm_state(QEMUFile *f) " device '%s'", instance_id, idstr); goto out; } + if (!check_section_footer(f, le->se)) { + ret = -EINVAL; + goto out; + } break; case QEMU_VM_SECTION_PART: case QEMU_VM_SECTION_END: @@ -1050,6 +1107,10 @@ int qemu_loadvm_state(QEMUFile *f) section_id, le->se->idstr); goto out; } + if (!check_section_footer(f, le->se)) { + ret = -EINVAL; + goto out; + } break; default: error_report("Unknown savevm section type %d", section_type);