On 11.06.2018 11:24, Cornelia Huck wrote:
> On Mon, 11 Jun 2018 09:49:39 +0200
> Christian Borntraeger <borntrae...@de.ibm.com> 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 <th...@redhat.com>
>>
>> Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
>>
>> 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?
>>> ---
>>> 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.
Thomas
