From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LULry-0000ro-4L for qemu-devel@nongnu.org; Tue, 03 Feb 2009 09:03:18 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LULrv-0000rP-OX for qemu-devel@nongnu.org; Tue, 03 Feb 2009 09:03:17 -0500 Received: from [199.232.76.173] (port=50594 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LULrt-0000rD-5J for qemu-devel@nongnu.org; Tue, 03 Feb 2009 09:03:13 -0500 Received: from mr01.hansenet.de ([213.191.74.10]:35754) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LULrs-0002f4-9E for qemu-devel@nongnu.org; Tue, 03 Feb 2009 09:03:12 -0500 Message-ID: <49884E8B.6010106@exactcode.de> Date: Tue, 03 Feb 2009 15:02:51 +0100 From: Rene Rebe MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] fix loading tiny kernels References: <49880773.5000203@exactcode.de> <49880902.20700@exactcode.de> <20090203103013.GC8886@redhat.com> <20090203133025.GC15613@redhat.com> In-Reply-To: <20090203133025.GC15613@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: Quoted-Printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Alexander Graf Daniel P. Berrange wrote: > On Tue, Feb 03, 2009 at 12:09:42PM +0100, Alexander Graf wrote: >> On 03.02.2009, at 11:30, Daniel P. Berrange wrote: >> >>> 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. >> 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. >=20 > I meant in terms of making sure we didn't overflow the header variable > which is allocated on the stack. So instead of >=20 > uint8_t header[1024]; > ... > fread(header, 1, 1024, f); >=20 > You'd have >=20 > uint8_t header[1024]; > ... > fread(header, 1, sizeof(header), f); >=20 > Daniel Just preventing this in the case it's changed in the future and one place is forgotten. I already changed the code to use the ARRAY_SIZE macro in my working copy: http://svn.exactcode.de/t2/trunk/package/emulators/kvm/09-qemu-multibo= ot.patch --=20 Ren=E9 Rebe - ExactCODE GmbH - Europe, Germany, Berlin http://exactcode.de | http://t2-project.org | http://rene.rebe.name