From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LULMG-0007C7-9M for qemu-devel@nongnu.org; Tue, 03 Feb 2009 08:30:32 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LULMD-0007Az-8e for qemu-devel@nongnu.org; Tue, 03 Feb 2009 08:30:31 -0500 Received: from [199.232.76.173] (port=34337 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LULMD-0007Aq-28 for qemu-devel@nongnu.org; Tue, 03 Feb 2009 08:30:29 -0500 Received: from mx1.redhat.com ([66.187.233.31]:52700) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LULMC-00075D-M6 for qemu-devel@nongnu.org; Tue, 03 Feb 2009 08:30:28 -0500 Date: Tue, 3 Feb 2009 13:30:25 +0000 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] [PATCH] fix loading tiny kernels Message-ID: <20090203133025.GC15613@redhat.com> References: <49880773.5000203@exactcode.de> <49880902.20700@exactcode.de> <20090203103013.GC8886@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Reply-To: "Daniel P. Berrange" , qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-devel@nongnu.org On Tue, Feb 03, 2009 at 12:09:42PM +0100, Alexander Graf wrote: >=20 > On 03.02.2009, at 11:30, Daniel P. Berrange wrote: >=20 > >On Tue, Feb 03, 2009 at 10:06:10AM +0100, Ren? Rebe wrote: > >>I babbled: > >>>Further testing / polishing the multi-boot kernel loading support =20 > >>>I found > >>>the existing code fails to load unusual small kernels, less than =20 > >>>8192 > >>>bytes - > >>>for example the example multi-boot kernel shipped within GRUB that > >>>compiles to just 7121 bytes on my system. > >>> > >>>Signed-off-by: Ren=E9 Rebe > >>> > >>>--- hw/pc.c (revision 6501) > >>>+++ hw/pc.c (working copy) > >>>@@ -554,7 +989,7 @@ > >>> /* load the kernel header */ > >>> f =3D fopen(kernel_filename, "rb"); > >>> if (!f || !(kernel_size =3D get_file_size(f)) || > >>>- fread(header, 1, 1024, f) !=3D 1024) { > >>>+ fread(header, 1, MIN(8192, kernel_size), f) !=3D MIN(8192, > >>>kernel_size)) { > >>> fprintf(stderr, "qemu: could not load kernel '%s'\n", > >>> kernel_filename); > >>> exit(1); > >>> > >>Ah, sorry - mix in the series. This only applies to the multi-boot =20 > >>series > >>which increases the header read to 8192 bytes. > > > >Regardless, this code should not hardcode the size like this. It =20 > >should > >use sizeof(header) instead of 1024 or 8192, thus avoiding the =20 > >potential > >bug. >=20 > You don't really know sizeof(header), do you? Header could be the =20 > Linux header or the Multiboot header which is by definition allowed to = =20 > sit somewhere within the first 8192 bytes. I meant in terms of making sure we didn't overflow the header variable which is allocated on the stack. So instead of uint8_t header[1024]; ... fread(header, 1, 1024, f); You'd have uint8_t header[1024]; ... fread(header, 1, sizeof(header), f); Daniel --=20 |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange= / :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 950= 5 :|