[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kerne
From: |
Christian Borntraeger |
Subject: |
Re: [qemu-s390x] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images |
Date: |
Mon, 11 Jun 2018 12:53:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 06/11/2018 12:08 PM, Thomas Huth wrote:
> On 11.06.2018 11:24, Cornelia Huck wrote:
>> On Mon, 11 Jun 2018 09:49:39 +0200
>> Christian Borntraeger <address@hidden> wrote:
>>
>>> On 06/10/2018 03:12 PM, Thomas Huth wrote:
>>>> Add a sanity check to fix the following crash:
>>>>
>>>> $ echo "Insane in the mainframe" > /tmp/test.txt
>>>> $ s390x-softmmu/qemu-system-s390x -nographic -kernel /tmp/test.txt
>>>> Segmentation fault (core dumped)
>>>>
>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>
>>> Reviewed-by: Christian Borntraeger <address@hidden>
>>>
>>> I think a similar problem exists for INITRD_PARM_START and
>>> INITRD_PARM_SIZE. No?
>>
>> I think so as well.
>
> You're right:
>
> $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt \
> -initrd /tmp/test.txt
> Segmentation fault (core dumped)
>
> Shall I sent a v2 of this patch, or do you prefer a separate patch for
> that issue?
Whatever is easier for you.
>
>>>> ---
>>>> hw/s390x/ipl.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>>>> index 04245b5..9bb9b50 100644
>>>> --- a/hw/s390x/ipl.c
>>>> +++ b/hw/s390x/ipl.c
>>>> @@ -168,7 +168,8 @@ static void s390_ipl_realize(DeviceState *dev, Error
>>>> **errp)
>>>> * we can not rely on the ELF entry point - it was 0x800 (the
>>>> SALIPL
>>>> * loader) and it won't work. For this case we force it to
>>>> 0x10000, too.
>>>> */
>>>> - if (pentry == KERN_IMAGE_START || pentry == 0x800) {
>>>> + if ((pentry == KERN_IMAGE_START || pentry == 0x800) &&
>>>> + kernel_size > KERN_PARM_AREA + strlen(ipl->cmdline)) {
>>>> ipl->start_addr = KERN_IMAGE_START;
>>>> /* Overwrite parameters in the kernel image, which are "rom"
>>>> */
>>>> strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
>>
>> The outcome of this is that we don't write into areas we must not write
>> into, but we still have a broken "kernel" and will simply fail if the
>> thing we're pointing to isn't a valid PSW. I guess that's what we want
>> ("crap in, crap out"), i.e. no fallback to the bios or something like
>> that?
>
> Yes, I think "crap in, crap out" is ok here. Theoretically, the user
> could also have a self-made micro-kernel that is just one byte smaller
> than KERN_PARM_AREA, and this would still work with this patch, so no
> need for an extra error message in that case.