[U-Boot] [PATCH] disk: efi: Correct backing up the MBR boot code

[Sam Protsenko]

On 23 May 2018 at 12:51, Andy Shevchenko <andy.shevchenko at gmail.com> wrote:
> On Tue, May 22, 2018 at 2:04 AM, Sam Protsenko
> <semen.protsenko at linaro.org> wrote:
>> In commit e163a931af34 ("cmd: gpt: backup boot code before writing MBR")
>> there was added the procedure for storing old boot code when doing "gpt
>> write". But instead of storing just backup code, the whole MBR was
>> stored, and only specific fields were replaced further, keeping
>> everything else intact. That's obviously not what we want.
>>
>> Fix the code to actually store only old boot code and zero out
>> everything else. This fixes next testing case:
>>
>>     => mmc write $loadaddr 0x0 0x7b
>>     => gpt write mmc 1 $partitions
>>
>> In case when $loadaddr address and further memory contains 0xff, the
>> board was bricked (ROM-code probably didn't like partition entries that
>> were clobbered with 0xff). With this patch applied, commands above don't
>> brick the board.
>
> Tested-by: Andy Shevchenko <andy.shevchenko at gmail.com>
>
> At least it doesn't break Intel Edison.
>

Hi Tom,

Can we apply this one? It's kinda critical, as some boards were
bricked while performing tests.

Thanks.

>>
>> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
>> Cc: Alejandro Hernandez <ajhernandez at ti.com>
>> ---
>>  disk/part_efi.c    | 6 ++++--
>>  include/part_efi.h | 3 ++-
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index 72e3997b84..5c1039f013 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -350,8 +350,6 @@ static int set_protective_mbr(struct blk_desc *dev_desc)
>>  {
>>         /* Setup the Protective MBR */
>>         ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, p_mbr, 1, dev_desc->blksz);
>> -       memset(p_mbr, 0, sizeof(*p_mbr));
>> -
>>         if (p_mbr == NULL) {
>>                 printf("%s: calloc failed!\n", __func__);
>>                 return -1;
>> @@ -363,6 +361,10 @@ static int set_protective_mbr(struct blk_desc *dev_desc)
>>                 return -1;
>>         }
>>
>> +       /* Clear all data in MBR except of backed up boot code */
>> +       memset((char *)p_mbr + MSDOS_MBR_BOOT_CODE_SIZE, 0, sizeof(*p_mbr) -
>> +                       MSDOS_MBR_BOOT_CODE_SIZE);
>> +
>>         /* Append signature */
>>         p_mbr->signature = MSDOS_MBR_SIGNATURE;
>>         p_mbr->partition_record[0].sys_ind = EFI_PMBR_OSTYPE_EFI_GPT;
>> diff --git a/include/part_efi.h b/include/part_efi.h
>> index 6065c571f3..8525770445 100644
>> --- a/include/part_efi.h
>> +++ b/include/part_efi.h
>> @@ -20,6 +20,7 @@
>>  #include <efi.h>
>>
>>  #define MSDOS_MBR_SIGNATURE 0xAA55
>> +#define MSDOS_MBR_BOOT_CODE_SIZE 440
>>  #define EFI_PMBR_OSTYPE_EFI 0xEF
>>  #define EFI_PMBR_OSTYPE_EFI_GPT 0xEE
>>
>> @@ -111,7 +112,7 @@ typedef struct _gpt_entry {
>>  } __packed gpt_entry;
>>
>>  typedef struct _legacy_mbr {
>> -       u8 boot_code[440];
>> +       u8 boot_code[MSDOS_MBR_BOOT_CODE_SIZE];
>>         __le32 unique_mbr_signature;
>>         __le16 unknown;
>>         struct partition partition_record[4];
>> --
>> 2.17.0
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>
>
>
> --
> With Best Regards,
> Andy Shevchenko