From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42507) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRZW-0005Hs-VG for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:00:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTRZS-0003Bc-Ss for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:00:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36785) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTRZS-0003BR-LS for qemu-devel@nongnu.org; Wed, 10 Feb 2016 05:00:26 -0500 References: <1453978470-222624-4-git-send-email-imammedo@redhat.com> <20160128125842-mutt-send-email-mst@redhat.com> <20160128130316.11af4330@nial.brq.redhat.com> <20160128145348-mutt-send-email-mst@redhat.com> <20160129121359.17842fef@nial.brq.redhat.com> <20160131170118-mutt-send-email-mst@redhat.com> <20160202105953.476a05bd@nial.brq.redhat.com> <20160202123756-mutt-send-email-mst@redhat.com> <20160209114608.4f89b528@nial.brq.redhat.com> <20160210103628-mutt-send-email-mst@redhat.com> <20160210110920-mutt-send-email-mst@redhat.com> From: Laszlo Ersek Message-ID: <56BB0A36.7010304@redhat.com> Date: Wed, 10 Feb 2016 11:00:22 +0100 MIME-Version: 1.0 In-Reply-To: <20160210110920-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v19 3/9] pc: add a Virtual Machine Generation ID device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Igor Mammedov Cc: ghammer@redhat.com, lcapitulino@redhat.com, Xiao Guangrong , ehabkost@redhat.com, qemu-devel@nongnu.org On 02/10/16 10:28, Michael S. Tsirkin wrote: > On Wed, Feb 10, 2016 at 10:51:47AM +0200, Michael S. Tsirkin wrote: >> So maybe we should >> leave this alone, wait until we see an actual user - this way we can >> figure out the implementation constraints better. > > What I'm definitely interested in seeing is improving the > bios_linker_loader API within QEMU. > > Is the below going in the right direction, in your opinion? > > > diff --git a/include/hw/acpi/bios-linker-loader.h b/include/hw/acpi/bios-linker-loader.h > index 498c0af..78d9a16 100644 > --- a/include/hw/acpi/bios-linker-loader.h > +++ b/include/hw/acpi/bios-linker-loader.h > @@ -17,6 +17,17 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file, > void *start, unsigned size, > uint8_t *checksum); > > +/* > + * bios_linker_loader_add_pointer: ask guest to append address of source file > + * into destination file at the specified pointer. > + * > + * @linker: linker file array > + * @dest_file: destination file that must be changed > + * @src_file: source file whos address must be taken > + * @table: destination file array > + * @pointer: location of the pointer to be patched within destination file > + * @pointer_size: size of pointer to be patched, in bytes > + */ > void bios_linker_loader_add_pointer(GArray *linker, > const char *dest_file, > const char *src_file, > diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c > index e04d60a..84be25a 100644 > --- a/hw/acpi/bios-linker-loader.c > +++ b/hw/acpi/bios-linker-loader.c > @@ -142,7 +142,13 @@ void bios_linker_loader_add_pointer(GArray *linker, > uint8_t pointer_size) > { > BiosLinkerLoaderEntry entry; > - size_t offset = (gchar *)pointer - table->data; > + size_t offset; > + > + assert((gchar *)pointer >= table->data); > + > + offset = (gchar *)pointer - table->data; > + > + assert(offset + pointer_size < table->len); > > memset(&entry, 0, sizeof entry); > strncpy(entry.pointer.dest_file, dest_file, > I have two suggestions (independently of Igor's upcoming opinion): (1) I propose to do all this arithmetic in uintptr_t, not (char*). (2) In the last assertion, < should be <=. Both sides are exclusive, so equality is valid. (BTW the OVMF implementation of the linker-loader client is chock full of verifications like the above, done in UINT64, so I can only agree with the above safety measures, independently of vmgenid.) Thanks Laszlo