| Message ID | 20190508094857.21145-3-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,SUBSYSTEM,s390x,1/3] pc-bios/s390-ccw: Clean up harmless misuse of isdigit() | expand |
Shall we cc stable this? On 08.05.19 11:48, Thomas Huth wrote: > From: "Jason J. Herne" <jjherne@linux.ibm.com> > > Newer versions of zipl have the ability to write signature entries to the boot > script for secure boot. We don't yet support secure boot, but we need to skip > over signature entries while reading the boot script in order to maintain our > ability to boot guest operating systems that have a secure bootloader. > > Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com> > Reviewed-by: Farhan Ali <alifm@linux.ibm.com> > Message-Id: <1556543381-12671-1-git-send-email-jjherne@linux.ibm.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/bootmap.c | 19 +++++++++++++++++-- > pc-bios/s390-ccw/bootmap.h | 10 ++++++---- > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 7aef65ab67..d13b7cbd15 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -254,7 +254,14 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr, > memset(sec, FREE_SPACE_FILLER, sizeof(sec)); > read_block(block_nr, sec, "Cannot read Boot Map Script"); > > - for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) { > + for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD || > + bms->entry[i].type == BOOT_SCRIPT_SIGNATURE; i++) { > + > + /* We don't support secure boot yet, so we skip signature entries */ > + if (bms->entry[i].type == BOOT_SCRIPT_SIGNATURE) { > + continue; > + } > + > address = bms->entry[i].address.load_address; > block_nr = eckd_block_num(&bms->entry[i].blkptr.xeckd.bptr.chs); > > @@ -489,7 +496,15 @@ static void zipl_run(ScsiBlockPtr *pte) > > /* Load image(s) into RAM */ > entry = (ComponentEntry *)(&header[1]); > - while (entry->component_type == ZIPL_COMP_ENTRY_LOAD) { > + while (entry->component_type == ZIPL_COMP_ENTRY_LOAD || > + entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) { > + > + /* We don't support secure boot yet, so we skip signature entries */ > + if (entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) { > + entry++; > + continue; > + } > + > zipl_load_segment(entry); > > entry++; > diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h > index a085212077..94f53a5f1e 100644 > --- a/pc-bios/s390-ccw/bootmap.h > +++ b/pc-bios/s390-ccw/bootmap.h > @@ -98,8 +98,9 @@ typedef struct ScsiMbr { > #define ZIPL_COMP_HEADER_IPL 0x00 > #define ZIPL_COMP_HEADER_DUMP 0x01 > > -#define ZIPL_COMP_ENTRY_LOAD 0x02 > -#define ZIPL_COMP_ENTRY_EXEC 0x01 > +#define ZIPL_COMP_ENTRY_EXEC 0x01 > +#define ZIPL_COMP_ENTRY_LOAD 0x02 > +#define ZIPL_COMP_ENTRY_SIGNATURE 0x03 > > typedef struct XEckdMbr { > uint8_t magic[4]; /* == "xIPL" */ > @@ -117,8 +118,9 @@ typedef struct BootMapScriptEntry { > BootMapPointer blkptr; > uint8_t pad[7]; > uint8_t type; /* == BOOT_SCRIPT_* */ > -#define BOOT_SCRIPT_EXEC 0x01 > -#define BOOT_SCRIPT_LOAD 0x02 > +#define BOOT_SCRIPT_EXEC 0x01 > +#define BOOT_SCRIPT_LOAD 0x02 > +#define BOOT_SCRIPT_SIGNATURE 0x03 > union { > uint64_t load_address; > uint64_t load_psw; >
On 10/05/2019 15.59, Christian Borntraeger wrote:
> Shall we cc stable this?
I think I'd rather not do it unless someone really speaks up that they
urgently need it. If we could use the binary from the master branch, I'd
say go for it, but in this case we'd need to build a separate
s390-ccw.img for this (without the DASD passthrough patches), and since
the stable branch does not get that much testing attention from all the
s390x developers, you'd end up with a firmware binary in the stable
branch that is not very well tested... This does not sound very
appealing to me.
Thomas
On Sat, 11 May 2019 08:15:21 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 10/05/2019 15.59, Christian Borntraeger wrote: > > Shall we cc stable this? > > I think I'd rather not do it unless someone really speaks up that they > urgently need it. If we could use the binary from the master branch, I'd > say go for it, but in this case we'd need to build a separate > s390-ccw.img for this (without the DASD passthrough patches), and since > the stable branch does not get that much testing attention from all the > s390x developers, you'd end up with a firmware binary in the stable > branch that is not very well tested... This does not sound very > appealing to me. FWIW, I have rebuilt the bios for the stable tree in the past, when a bios patch had been picked. In this case, however, I would need to rely on someone else to sanity-check the binary. How likely are folks to run -stable QEMU with a bootmap containing signatures? It would be one more QEMU version with toleration for this, but I expect distros to pick up this one anyway?
On 13.05.19 09:42, Cornelia Huck wrote: > On Sat, 11 May 2019 08:15:21 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> On 10/05/2019 15.59, Christian Borntraeger wrote: >>> Shall we cc stable this? >> >> I think I'd rather not do it unless someone really speaks up that they >> urgently need it. If we could use the binary from the master branch, I'd >> say go for it, but in this case we'd need to build a separate >> s390-ccw.img for this (without the DASD passthrough patches), and since >> the stable branch does not get that much testing attention from all the >> s390x developers, you'd end up with a firmware binary in the stable >> branch that is not very well tested... This does not sound very >> appealing to me. > > FWIW, I have rebuilt the bios for the stable tree in the past, when a > bios patch had been picked. In this case, however, I would need to rely > on someone else to sanity-check the binary. > > How likely are folks to run -stable QEMU with a bootmap containing > signatures? It would be one more QEMU version with toleration for this, > but I expect distros to pick up this one anyway? Yes, I will try to push this into distros. I usually try to push things also to stable, but this might be more important for the kernel.
On Mon, 13 May 2019 09:44:37 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 13.05.19 09:42, Cornelia Huck wrote: > > On Sat, 11 May 2019 08:15:21 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > > > >> On 10/05/2019 15.59, Christian Borntraeger wrote: > >>> Shall we cc stable this? > >> > >> I think I'd rather not do it unless someone really speaks up that they > >> urgently need it. If we could use the binary from the master branch, I'd > >> say go for it, but in this case we'd need to build a separate > >> s390-ccw.img for this (without the DASD passthrough patches), and since > >> the stable branch does not get that much testing attention from all the > >> s390x developers, you'd end up with a firmware binary in the stable > >> branch that is not very well tested... This does not sound very > >> appealing to me. > > > > FWIW, I have rebuilt the bios for the stable tree in the past, when a > > bios patch had been picked. In this case, however, I would need to rely > > on someone else to sanity-check the binary. > > > > How likely are folks to run -stable QEMU with a bootmap containing > > signatures? It would be one more QEMU version with toleration for this, > > but I expect distros to pick up this one anyway? > > Yes, I will try to push this into distros. I usually try to push things also > to stable, but this might be more important for the kernel. > Yes, while the QEMU stable branch is useful, the kernel stable backports cover a lot more.
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 7aef65ab67..d13b7cbd15 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -254,7 +254,14 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr, memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(block_nr, sec, "Cannot read Boot Map Script"); - for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) { + for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD || + bms->entry[i].type == BOOT_SCRIPT_SIGNATURE; i++) { + + /* We don't support secure boot yet, so we skip signature entries */ + if (bms->entry[i].type == BOOT_SCRIPT_SIGNATURE) { + continue; + } + address = bms->entry[i].address.load_address; block_nr = eckd_block_num(&bms->entry[i].blkptr.xeckd.bptr.chs); @@ -489,7 +496,15 @@ static void zipl_run(ScsiBlockPtr *pte) /* Load image(s) into RAM */ entry = (ComponentEntry *)(&header[1]); - while (entry->component_type == ZIPL_COMP_ENTRY_LOAD) { + while (entry->component_type == ZIPL_COMP_ENTRY_LOAD || + entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) { + + /* We don't support secure boot yet, so we skip signature entries */ + if (entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) { + entry++; + continue; + } + zipl_load_segment(entry); entry++; diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index a085212077..94f53a5f1e 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -98,8 +98,9 @@ typedef struct ScsiMbr { #define ZIPL_COMP_HEADER_IPL 0x00 #define ZIPL_COMP_HEADER_DUMP 0x01 -#define ZIPL_COMP_ENTRY_LOAD 0x02 -#define ZIPL_COMP_ENTRY_EXEC 0x01 +#define ZIPL_COMP_ENTRY_EXEC 0x01 +#define ZIPL_COMP_ENTRY_LOAD 0x02 +#define ZIPL_COMP_ENTRY_SIGNATURE 0x03 typedef struct XEckdMbr { uint8_t magic[4]; /* == "xIPL" */ @@ -117,8 +118,9 @@ typedef struct BootMapScriptEntry { BootMapPointer blkptr; uint8_t pad[7]; uint8_t type; /* == BOOT_SCRIPT_* */ -#define BOOT_SCRIPT_EXEC 0x01 -#define BOOT_SCRIPT_LOAD 0x02 +#define BOOT_SCRIPT_EXEC 0x01 +#define BOOT_SCRIPT_LOAD 0x02 +#define BOOT_SCRIPT_SIGNATURE 0x03 union { uint64_t load_address; uint64_t load_psw;