[Qemu-devel] [PATCH] pc_init: Fail on bad kernel

[Qemu-devel] [PATCH] pc_init: Fail on bad kernel

[Sasha Levin]

When providing QEMU with a bad '-kernel' parameter, such as a file which
is not really a kernel, QEMU will attempt to allocate a huge amount of
memory and fail either with "Failed to allocate memory: Cannot allocate
memory" or a GLib error: "GLib-ERROR **: gmem.c:170: failed to allocate
18446744073709529965 bytes"

This patch handles the case where the magic sig wasn't located in the
provided kernel, and loading it as multiboot failed as well.

Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 hw/pc.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 6b3662e..428440b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -691,8 +691,14 @@ static void load_linux(void *fw_cfg,
 	/* This looks like a multiboot kernel. If it is, let's stop
 	   treating it like a Linux kernel. */
         if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
-                           kernel_cmdline, kernel_size, header))
+                           kernel_cmdline, kernel_size, header)) {
             return;
+        } else {
+            fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
+		    kernel_filename, strerror(errno));
+	    exit(1);
+        }
+	    
 	protocol = 0;
     }
 
-- 
1.7.6.1

Re: [Qemu-devel] [PATCH] pc_init: Fail on bad kernel

[Anthony Liguori]

On 09/03/2011 02:35 PM, Sasha Levin wrote:
> When providing QEMU with a bad '-kernel' parameter, such as a file which
> is not really a kernel, QEMU will attempt to allocate a huge amount of
> memory and fail either with "Failed to allocate memory: Cannot allocate
> memory" or a GLib error: "GLib-ERROR **: gmem.c:170: failed to allocate
> 18446744073709529965 bytes"
>
> This patch handles the case where the magic sig wasn't located in the
> provided kernel, and loading it as multiboot failed as well.
>
> Cc: Anthony Liguori<aliguori@us.ibm.com>
> Signed-off-by: Sasha Levin<levinsasha928@gmail.com>
> ---
>   hw/pc.c |    8 +++++++-
>   1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 6b3662e..428440b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -691,8 +691,14 @@ static void load_linux(void *fw_cfg,
>   	/* This looks like a multiboot kernel. If it is, let's stop
>   	   treating it like a Linux kernel. */
>           if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
> -                           kernel_cmdline, kernel_size, header))
> +                           kernel_cmdline, kernel_size, header)) {
>               return;
> +        } else {
> +            fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
> +		    kernel_filename, strerror(errno));
> +	    exit(1);
> +        }
> +	

There's trailing whitespace on this line.

But I also don't think this is the right fix.  This change makes the 
line below unreachable.  There is still code in this path attempting to 
handle protocols < 2.00.  Admittedly, these would be ancient kernels 
that I doubt anyone would really use but the code is there to support it 
nonetheless.

I think a better fix would be to positively identify kernels that are 
older than this.

Perhaps hpa knows how we could positively identify a kernel that's older 
than protocol 200?

Regards,

Anthony Liguori

>   	protocol = 0;
>       }
>

Re: [Qemu-devel] [PATCH] pc_init: Fail on bad kernel

[Sasha Levin]

Ping?

On Sat, 2011-09-03 at 22:35 +0300, Sasha Levin wrote:
> When providing QEMU with a bad '-kernel' parameter, such as a file which
> is not really a kernel, QEMU will attempt to allocate a huge amount of
> memory and fail either with "Failed to allocate memory: Cannot allocate
> memory" or a GLib error: "GLib-ERROR **: gmem.c:170: failed to allocate
> 18446744073709529965 bytes"
> 
> This patch handles the case where the magic sig wasn't located in the
> provided kernel, and loading it as multiboot failed as well.
> 
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  hw/pc.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 6b3662e..428440b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -691,8 +691,14 @@ static void load_linux(void *fw_cfg,
>  	/* This looks like a multiboot kernel. If it is, let's stop
>  	   treating it like a Linux kernel. */
>          if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
> -                           kernel_cmdline, kernel_size, header))
> +                           kernel_cmdline, kernel_size, header)) {
>              return;
> +        } else {
> +            fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
> +		    kernel_filename, strerror(errno));
> +	    exit(1);
> +        }
> +	    
>  	protocol = 0;
>      }
>  

-- 

Sasha.

Re: [Qemu-devel] [PATCH] pc_init: Fail on bad kernel

[Anthony Liguori]

On 09/14/2011 02:45 AM, Sasha Levin wrote:
> Ping?

I replied on 9/9 with comments.

Regards,

Anthony Liguori

>
> On Sat, 2011-09-03 at 22:35 +0300, Sasha Levin wrote:
>> When providing QEMU with a bad '-kernel' parameter, such as a file which
>> is not really a kernel, QEMU will attempt to allocate a huge amount of
>> memory and fail either with "Failed to allocate memory: Cannot allocate
>> memory" or a GLib error: "GLib-ERROR **: gmem.c:170: failed to allocate
>> 18446744073709529965 bytes"
>>
>> This patch handles the case where the magic sig wasn't located in the
>> provided kernel, and loading it as multiboot failed as well.
>>
>> Cc: Anthony Liguori<aliguori@us.ibm.com>
>> Signed-off-by: Sasha Levin<levinsasha928@gmail.com>
>> ---
>>   hw/pc.c |    8 +++++++-
>>   1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 6b3662e..428440b 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -691,8 +691,14 @@ static void load_linux(void *fw_cfg,
>>   	/* This looks like a multiboot kernel. If it is, let's stop
>>   	   treating it like a Linux kernel. */
>>           if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
>> -                           kernel_cmdline, kernel_size, header))
>> +                           kernel_cmdline, kernel_size, header)) {
>>               return;
>> +        } else {
>> +            fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
>> +		    kernel_filename, strerror(errno));
>> +	    exit(1);
>> +        }
>> +	
>>   	protocol = 0;
>>       }
>>
>