hw/s390x/ipl: Fix crashes that occurs when -kernel is used with small images
PATCH v2 1/1 hw/s390x/ipl: Fix crashes that occurs when -kernel is used with small images
Add a sanity checks to fix the following two crashes:
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)
---
v2: Fixed 2nd crash with -initrd
hw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
hw/s390x/ipl.c +6 -3
@@ -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);
@@ -195,8 +196,10 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)* we have to overwrite values in the kernel image, * which are "rom" */ -stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); -stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); +if (kernel_size >INITRD_PARM_SIZE +8 ) { +stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); +stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); + } } }/*
Signature · 1 line
1.8.3.1
Quoted 9 lines · Thomas Huth
Add a sanity checks to fix the following two crashes:
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)Signed-off-by Thomas Huth <[email protected]>
Quoted 34 lines
---
v2: Fixed 2nd crash with -initrdhw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)hw/s390x/ipl.c +6
-3@@ -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);@@ -195,8 +196,10 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)* we have to overwrite values in the kernel image, * which are "rom" */ -stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); -stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); +if (kernel_size >INITRD_PARM_SIZE +8 ) { +stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); +stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); + } } }/*
Cornelia Huck
AppliedOn Mon, 11 Jun 2018 19:26:47 +0200
Thomas Huth <[email protected]> wrote:
I've tweaked $SUBJECT to
hw/s390x/ipl: Fix crashes when using -kernel/-initrd with small images
Quoted 1 line
Add a sanity checks to fix the following two crashes:
Also, s/Add a sanity checks/Add sanity checks/
Quoted 7 lines
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)
Quoted 6 lines
Signed-off-by Thomas Huth <[email protected]>---
v2: Fixed 2nd crash with -initrdhw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Thanks, applied.
Quoted 32 lines · Thomas Huth
Add a sanity checks to fix the following two crashes:
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)Signed-off-by Thomas Huth <[email protected]>---
v2: Fixed 2nd crash with -initrdhw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)hw/s390x/ipl.c +2
-1@@ -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);@@ -195,8 +196,10 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)* we have to overwrite values in the kernel image, * which are "rom"
hw/s390x/ipl.c static void s390_ipl_realize(DeviceState *dev, Error **errp) · line 205
Quoted 10 lines
hw/s390x/ipl.c +4
-2*/ -stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); -stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); +if (kernel_size >INITRD_PARM_SIZE +8 ) { +stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); +stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); + } } }/*
I just had to discover that this breaks the argv handling in kvm-unit-tests. Cornelia, could you please unqueue this patch again? I'll try to come up with a better solution...
Thomas
Quoted 39 lines · On Thu, 14 Jun 2018 21:08:43 +0200 Thomas Huth <[email protected]> wrote:
Quoted 38 lines · Thomas Huth
Add a sanity checks to fix the following two crashes:
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)Signed-off-by Thomas Huth <[email protected]>---
v2: Fixed 2nd crash with -initrdhw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)hw/s390x/ipl.c +5
-3@@ -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);@@ -195,8 +196,10 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)* we have to overwrite values in the kernel image, * which are "rom" */ -stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); -stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); +if (kernel_size >INITRD_PARM_SIZE +8 ) { +stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); +stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size);
Quoted 8 lines
hw/s390x/ipl.c static void s390_ipl_realize(DeviceState *dev, Error **errp) · line 205
Quoted 4 lines
hw/s390x/ipl.c +1
-0+ } } }/* I just had to discover that this breaks the argv handling in kvm-unit-tests. Cornelia, could you please unqueue this patch again? I'll try to come up with a better solution...
Argh. I wonder why?
Unqueued.
Quoted 51 lines · Cornelia Huck
Quoted 39 lines · On Thu, 14 Jun 2018 21:08:43 +0200 Thomas Huth <[email protected]> wrote:
Quoted 38 lines · Thomas Huth
Add a sanity checks to fix the following two crashes:
$ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt Segmentation fault (core dumped)Signed-off-by Thomas Huth <[email protected]>---
v2: Fixed 2nd crash with -initrdhw/s390x/ipl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)hw/s390x/ipl.c +5
-3@@ -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);@@ -195,8 +196,10 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)* we have to overwrite values in the kernel image, * which are "rom" */ -stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); -stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size); +if (kernel_size >INITRD_PARM_SIZE +8 ) { +stq_p (rom_ptr (INITRD_PARM_START ), initrd_offset); +stq_p (rom_ptr (INITRD_PARM_SIZE ), initrd_size);Quoted 8 lines
hw/s390x/ipl.c static void s390_ipl_realize(DeviceState *dev, Error **errp) · line 205
Quoted 4 lines
hw/s390x/ipl.c +1
-0+ } } }/* I just had to discover that this breaks the argv handling in kvm-unit-tests. Cornelia, could you please unqueue this patch again? I'll try to come up with a better solution...
Quoted 1 line
Argh. I wonder why?
kvm-unit-tests are ELF files which are not loaded to address 0, but still use the same entry point as Linux (0x10000) to be able to use the "--append" option of QEMU. Thus the check "kernel_size > KERN_PARM_AREA
+ strlen(ipl->cmdline)" does not work here. We'd need something like "start_addr + kernel_size > KERN_PARM_AREA + strlen(ipl->cmdline)" instead - but this does not make too much sense with ELF files that have multiple segments either.
The correct fix is to check the size when calling rom_ptr() instead. I just sent a patch that does this.
Thomas