diff mbox series

[1/3] pc-bios/s390-ccw: fix off-by-one error

Message ID 20200924085926.21709-2-mhartmay@linux.ibm.com
State New
Headers show
Series pc-bios: s390x: fix corner cases in booting from ECKD | expand

Commit Message

Marc Hartmayer Sept. 24, 2020, 8:59 a.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Sept. 24, 2020, 10:02 a.m. UTC | #1
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;

>
Thomas Huth Sept. 24, 2020, 11:43 a.m. UTC | #2
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>
Marc Hartmayer Sept. 24, 2020, 2:09 p.m. UTC | #3
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;

>> 

>

-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
diff mbox series

Patch

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;