| Message ID | 20211006092631.20732-1-mhartmay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/ipl: check kernel command line size | expand |
Am 06.10.21 um 11:26 schrieb Marc Hartmayer: > Check if the provided kernel command line exceeds the maximum size of the s390x > Linux kernel command line size, which is 896 bytes. > > Reported-by: Sven Schnelle <svens@linux.ibm.com> > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390x/ipl.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 1821c6faeef3..a58ea58cc736 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -38,6 +38,7 @@ > #define KERN_IMAGE_START 0x010000UL > #define LINUX_MAGIC_ADDR 0x010008UL > #define KERN_PARM_AREA 0x010480UL > +#define KERN_PARM_AREA_SIZE 0x000380UL > #define INITRD_START 0x800000UL > #define INITRD_PARM_START 0x010408UL > #define PARMFILE_START 0x001000UL > @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > * loader) and it won't work. For this case we force it to 0x10000, too. > */ > if (pentry == KERN_IMAGE_START || pentry == 0x800) { > - char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1); > + size_t cmdline_size = strlen(ipl->cmdline) + 1; > + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); > + > ipl->start_addr = KERN_IMAGE_START; > /* Overwrite parameters in the kernel image, which are "rom" */ > if (parm_area) { > + if (cmdline_size > KERN_PARM_AREA_SIZE) { > + error_setg(errp, > + "kernel command line exceeds maximum size: %lu > %lu", > + cmdline_size, KERN_PARM_AREA_SIZE); > + return; > + } > + > strcpy(parm_area, ipl->cmdline); > } > } else { >
On 06/10/2021 11.26, Marc Hartmayer wrote: > Check if the provided kernel command line exceeds the maximum size of the s390x > Linux kernel command line size, which is 896 bytes. > > Reported-by: Sven Schnelle <svens@linux.ibm.com> > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> > --- > hw/s390x/ipl.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 1821c6faeef3..a58ea58cc736 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -38,6 +38,7 @@ > #define KERN_IMAGE_START 0x010000UL > #define LINUX_MAGIC_ADDR 0x010008UL > #define KERN_PARM_AREA 0x010480UL > +#define KERN_PARM_AREA_SIZE 0x000380UL > #define INITRD_START 0x800000UL > #define INITRD_PARM_START 0x010408UL > #define PARMFILE_START 0x001000UL > @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > * loader) and it won't work. For this case we force it to 0x10000, too. > */ > if (pentry == KERN_IMAGE_START || pentry == 0x800) { > - char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1); > + size_t cmdline_size = strlen(ipl->cmdline) + 1; > + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); > + > ipl->start_addr = KERN_IMAGE_START; > /* Overwrite parameters in the kernel image, which are "rom" */ > if (parm_area) { > + if (cmdline_size > KERN_PARM_AREA_SIZE) { > + error_setg(errp, > + "kernel command line exceeds maximum size: %lu > %lu", I think the first %lu should be %zd instead? Apart from that, the patch looks fine to me... so if you agree, I can fix that up when picking up the patch. Thomas > + cmdline_size, KERN_PARM_AREA_SIZE); > + return; > + } > + > strcpy(parm_area, ipl->cmdline); > } > } else { >
Thomas Huth <thuth@redhat.com> writes: > On 06/10/2021 11.26, Marc Hartmayer wrote: >> Check if the provided kernel command line exceeds the maximum size of the s390x >> Linux kernel command line size, which is 896 bytes. >> >> Reported-by: Sven Schnelle <svens@linux.ibm.com> >> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> >> --- >> hw/s390x/ipl.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c >> index 1821c6faeef3..a58ea58cc736 100644 >> --- a/hw/s390x/ipl.c >> +++ b/hw/s390x/ipl.c >> @@ -38,6 +38,7 @@ >> #define KERN_IMAGE_START 0x010000UL >> #define LINUX_MAGIC_ADDR 0x010008UL >> #define KERN_PARM_AREA 0x010480UL >> +#define KERN_PARM_AREA_SIZE 0x000380UL >> #define INITRD_START 0x800000UL >> #define INITRD_PARM_START 0x010408UL >> #define PARMFILE_START 0x001000UL >> @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) >> * loader) and it won't work. For this case we force it to 0x10000, too. >> */ >> if (pentry == KERN_IMAGE_START || pentry == 0x800) { >> - char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1); >> + size_t cmdline_size = strlen(ipl->cmdline) + 1; >> + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); >> + >> ipl->start_addr = KERN_IMAGE_START; >> /* Overwrite parameters in the kernel image, which are "rom" */ >> if (parm_area) { >> + if (cmdline_size > KERN_PARM_AREA_SIZE) { >> + error_setg(errp, >> + "kernel command line exceeds maximum size: %lu > %lu", > > I think the first %lu should be %zd instead? Yep, makes sense - thanks! > > Apart from that, the patch looks fine to me... so if you agree, I can fix > that up when picking up the patch. Thanks. > > Thomas > > >> + cmdline_size, KERN_PARM_AREA_SIZE); >> + return; >> + } >> + >> strcpy(parm_area, ipl->cmdline); >> } >> } else { >> >
On 10/11/21 15:38, Thomas Huth wrote: > On 06/10/2021 11.26, Marc Hartmayer wrote: >> Check if the provided kernel command line exceeds the maximum size of >> the s390x >> Linux kernel command line size, which is 896 bytes. >> >> Reported-by: Sven Schnelle <svens@linux.ibm.com> >> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> >> --- >> hw/s390x/ipl.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c >> index 1821c6faeef3..a58ea58cc736 100644 >> --- a/hw/s390x/ipl.c >> +++ b/hw/s390x/ipl.c >> @@ -38,6 +38,7 @@ >> #define KERN_IMAGE_START 0x010000UL >> #define LINUX_MAGIC_ADDR 0x010008UL >> #define KERN_PARM_AREA 0x010480UL >> +#define KERN_PARM_AREA_SIZE 0x000380UL >> #define INITRD_START 0x800000UL >> #define INITRD_PARM_START 0x010408UL >> #define PARMFILE_START 0x001000UL >> @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, >> Error **errp) >> * loader) and it won't work. For this case we force it to >> 0x10000, too. >> */ >> if (pentry == KERN_IMAGE_START || pentry == 0x800) { >> - char *parm_area = rom_ptr(KERN_PARM_AREA, >> strlen(ipl->cmdline) + 1); >> + size_t cmdline_size = strlen(ipl->cmdline) + 1; >> + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); >> + >> ipl->start_addr = KERN_IMAGE_START; >> /* Overwrite parameters in the kernel image, which are >> "rom" */ >> if (parm_area) { >> + if (cmdline_size > KERN_PARM_AREA_SIZE) { >> + error_setg(errp, >> + "kernel command line exceeds maximum >> size: %lu > %lu", > > I think the first %lu should be %zd instead? %zu ;) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Apart from that, the patch looks fine to me... so if you agree, I can > fix that up when picking up the patch. > > Thomas
On 11/10/2021 20.08, Philippe Mathieu-Daudé wrote: > On 10/11/21 15:38, Thomas Huth wrote: >> On 06/10/2021 11.26, Marc Hartmayer wrote: >>> Check if the provided kernel command line exceeds the maximum size of >>> the s390x >>> Linux kernel command line size, which is 896 bytes. >>> >>> Reported-by: Sven Schnelle <svens@linux.ibm.com> >>> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> >>> --- >>> hw/s390x/ipl.c | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c >>> index 1821c6faeef3..a58ea58cc736 100644 >>> --- a/hw/s390x/ipl.c >>> +++ b/hw/s390x/ipl.c >>> @@ -38,6 +38,7 @@ >>> #define KERN_IMAGE_START 0x010000UL >>> #define LINUX_MAGIC_ADDR 0x010008UL >>> #define KERN_PARM_AREA 0x010480UL >>> +#define KERN_PARM_AREA_SIZE 0x000380UL >>> #define INITRD_START 0x800000UL >>> #define INITRD_PARM_START 0x010408UL >>> #define PARMFILE_START 0x001000UL >>> @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, >>> Error **errp) >>> * loader) and it won't work. For this case we force it to >>> 0x10000, too. >>> */ >>> if (pentry == KERN_IMAGE_START || pentry == 0x800) { >>> - char *parm_area = rom_ptr(KERN_PARM_AREA, >>> strlen(ipl->cmdline) + 1); >>> + size_t cmdline_size = strlen(ipl->cmdline) + 1; >>> + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); >>> + >>> ipl->start_addr = KERN_IMAGE_START; >>> /* Overwrite parameters in the kernel image, which are >>> "rom" */ >>> if (parm_area) { >>> + if (cmdline_size > KERN_PARM_AREA_SIZE) { >>> + error_setg(errp, >>> + "kernel command line exceeds maximum >>> size: %lu > %lu", >> >> I think the first %lu should be %zd instead? > > %zu ;) All right, queued with %zu on my s390x-next branch: https://gitlab.com/thuth/qemu/-/commits/s390x-next/ Thomas
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 1821c6faeef3..a58ea58cc736 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -38,6 +38,7 @@ #define KERN_IMAGE_START 0x010000UL #define LINUX_MAGIC_ADDR 0x010008UL #define KERN_PARM_AREA 0x010480UL +#define KERN_PARM_AREA_SIZE 0x000380UL #define INITRD_START 0x800000UL #define INITRD_PARM_START 0x010408UL #define PARMFILE_START 0x001000UL @@ -190,10 +191,19 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) * loader) and it won't work. For this case we force it to 0x10000, too. */ if (pentry == KERN_IMAGE_START || pentry == 0x800) { - char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1); + size_t cmdline_size = strlen(ipl->cmdline) + 1; + char *parm_area = rom_ptr(KERN_PARM_AREA, cmdline_size); + ipl->start_addr = KERN_IMAGE_START; /* Overwrite parameters in the kernel image, which are "rom" */ if (parm_area) { + if (cmdline_size > KERN_PARM_AREA_SIZE) { + error_setg(errp, + "kernel command line exceeds maximum size: %lu > %lu", + cmdline_size, KERN_PARM_AREA_SIZE); + return; + } + strcpy(parm_area, ipl->cmdline); } } else {
Check if the provided kernel command line exceeds the maximum size of the s390x Linux kernel command line size, which is 896 bytes. Reported-by: Sven Schnelle <svens@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- hw/s390x/ipl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)