Message ID | 20200924085926.21709-2-mhartmay@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | pc-bios: s390x: fix corner cases in booting from ECKD | expand |
Hi Marc, On 9/24/20 10:59 AM, Marc Hartmayer wrote: > This error takes effect when the magic value "zIPL" is located at the > end of a block. For example if s2_cur_blk = 0x7fe18000 and the magic > value "zIPL" is located at 0x7fe18ffc - 0x7fe18fff. > > Fixes: ba831b25262a ("s390-ccw: read stage2 boot loader data to find menu") > Reviewed-by: Collin Walling <walling@linux.ibm.com> > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> > --- > pc-bios/s390-ccw/bootmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 97205674e59a..0d29dceaa3cc 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -163,7 +163,7 @@ static bool find_zipl_boot_menu_banner(int *offset) > int i; > > /* Menu banner starts with "zIPL" */ > - for (i = 0; i < virtio_get_block_size() - 4; i++) { > + for (i = 0; i < virtio_get_block_size() - 3; i++) { Easier to review as: for (i = 0; i <= virtio_get_block_size() - 4; i++) { Even easier defining ZIPL_MAGIC_SIZE instead of the magic '4'. > if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) { > *offset = i; > return true; >
On 24/09/2020 10.59, Marc Hartmayer wrote: > This error takes effect when the magic value "zIPL" is located at the > end of a block. For example if s2_cur_blk = 0x7fe18000 and the magic > value "zIPL" is located at 0x7fe18ffc - 0x7fe18fff. > > Fixes: ba831b25262a ("s390-ccw: read stage2 boot loader data to find menu") > Reviewed-by: Collin Walling <walling@linux.ibm.com> > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> > --- > pc-bios/s390-ccw/bootmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 97205674e59a..0d29dceaa3cc 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -163,7 +163,7 @@ static bool find_zipl_boot_menu_banner(int *offset) > int i; > > /* Menu banner starts with "zIPL" */ > - for (i = 0; i < virtio_get_block_size() - 4; i++) { > + for (i = 0; i < virtio_get_block_size() - 3; i++) { > if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) { > *offset = i; > return true; I agree with Philippe, i <= virtio_get_block_size() - 4 would be a little bit nicer. Anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Thu, Sep 24, 2020 at 12:02 PM +0200, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Marc, > > On 9/24/20 10:59 AM, Marc Hartmayer wrote: >> This error takes effect when the magic value "zIPL" is located at the >> end of a block. For example if s2_cur_blk = 0x7fe18000 and the magic >> value "zIPL" is located at 0x7fe18ffc - 0x7fe18fff. >> >> Fixes: ba831b25262a ("s390-ccw: read stage2 boot loader data to find menu") >> Reviewed-by: Collin Walling <walling@linux.ibm.com> >> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> >> --- >> pc-bios/s390-ccw/bootmap.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c >> index 97205674e59a..0d29dceaa3cc 100644 >> --- a/pc-bios/s390-ccw/bootmap.c >> +++ b/pc-bios/s390-ccw/bootmap.c >> @@ -163,7 +163,7 @@ static bool find_zipl_boot_menu_banner(int *offset) >> int i; >> >> /* Menu banner starts with "zIPL" */ >> - for (i = 0; i < virtio_get_block_size() - 4; i++) { >> + for (i = 0; i < virtio_get_block_size() - 3; i++) { > > Easier to review as: > > for (i = 0; i <= virtio_get_block_size() - 4; i++) { Yep. > > Even easier defining ZIPL_MAGIC_SIZE instead of the magic '4'. I thought about adding such a macro as well. Makes even more sense with your proposed change. > >> if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) { >> *offset = i; >> return true; >> >
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 97205674e59a..0d29dceaa3cc 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -163,7 +163,7 @@ static bool find_zipl_boot_menu_banner(int *offset) int i; /* Menu banner starts with "zIPL" */ - for (i = 0; i < virtio_get_block_size() - 4; i++) { + for (i = 0; i < virtio_get_block_size() - 3; i++) { if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) { *offset = i; return true;