* [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images
@ 2018-06-10 13:12 Thomas Huth
2018-06-11 7:49 ` Christian Borntraeger
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2018-06-10 13:12 UTC (permalink / raw)
To: Christian Borntraeger, qemu-s390x; +Cc: Cornelia Huck, qemu-devel
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 <thuth@redhat.com>
---
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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images
2018-06-10 13:12 [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images Thomas Huth
@ 2018-06-11 7:49 ` Christian Borntraeger
2018-06-11 9:24 ` Cornelia Huck
0 siblings, 1 reply; 5+ messages in thread
From: Christian Borntraeger @ 2018-06-11 7:49 UTC (permalink / raw)
To: Thomas Huth, qemu-s390x; +Cc: Cornelia Huck, qemu-devel
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 <thuth@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
I think a similar problem exists for INITRD_PARM_START and INITRD_PARM_SIZE. No?
> ---
> 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);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images
2018-06-11 7:49 ` Christian Borntraeger
@ 2018-06-11 9:24 ` Cornelia Huck
2018-06-11 10:08 ` Thomas Huth
0 siblings, 1 reply; 5+ messages in thread
From: Cornelia Huck @ 2018-06-11 9:24 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: Thomas Huth, qemu-s390x, qemu-devel
On Mon, 11 Jun 2018 09:49:39 +0200
Christian Borntraeger <borntraeger@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 <thuth@redhat.com>
>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> I think a similar problem exists for INITRD_PARM_START and INITRD_PARM_SIZE. No?
I think so as well.
>
>
> > ---
> > 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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images
2018-06-11 9:24 ` Cornelia Huck
@ 2018-06-11 10:08 ` Thomas Huth
2018-06-11 10:53 ` Christian Borntraeger
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2018-06-11 10:08 UTC (permalink / raw)
To: Cornelia Huck, Christian Borntraeger; +Cc: qemu-s390x, qemu-devel
On 11.06.2018 11:24, Cornelia Huck wrote:
> On Mon, 11 Jun 2018 09:49:39 +0200
> Christian Borntraeger <borntraeger@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 <thuth@redhat.com>
>>
>> Reviewed-by: Christian Borntraeger <borntraeger@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images
2018-06-11 10:08 ` Thomas Huth
@ 2018-06-11 10:53 ` Christian Borntraeger
0 siblings, 0 replies; 5+ messages in thread
From: Christian Borntraeger @ 2018-06-11 10:53 UTC (permalink / raw)
To: Thomas Huth, Cornelia Huck; +Cc: qemu-s390x, qemu-devel
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 <borntraeger@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 <thuth@redhat.com>
>>>
>>> Reviewed-by: Christian Borntraeger <borntraeger@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?
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-11 10:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-10 13:12 [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash that occurs when -kernel is used with small images Thomas Huth
2018-06-11 7:49 ` Christian Borntraeger
2018-06-11 9:24 ` Cornelia Huck
2018-06-11 10:08 ` Thomas Huth
2018-06-11 10:53 ` Christian Borntraeger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.