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
I found
the existing code fails to load unusual small kernels, less than
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é Rebe <address@hidden>
--- hw/pc.c (revision 6501)
+++ hw/pc.c (working copy)
@@ -554,7 +989,7 @@
/* load the kernel header */
f = fopen(kernel_filename, "rb");
if (!f || !(kernel_size = get_file_size(f)) ||
- fread(header, 1, 1024, f) != 1024) {
+ fread(header, 1, MIN(8192, kernel_size), f) != 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
series
which increases the header read to 8192 bytes.
Regardless, this code should not hardcode the size like this. It
should
use sizeof(header) instead of 1024 or 8192, thus avoiding the
potential
bug.
You don't really know sizeof(header), do you? Header could be the
Linux header or the Multiboot header which is by definition allowed to
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