[U-Boot] [PATCH V2] disk:efi: avoid unaligned access on efi partition

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Tom,

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/19/2014 10:03 AM, Tom Rini wrote:
> > On 02/19/2014 09:56 AM, Hector Palacios wrote:
> >> On 01/28/2014 01:46 PM, Piotr Wilczek wrote:
> >>> This patch fixes part_efi code to avoid unaligned access exception
> >>> on some ARM platforms.
> >>>
> >>> Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>> CC: Tom Rini <trini@ti.com>
> >>> CC: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >>> ---
> >>> Chnages for V2:
> >>>   - used put_unaligned to copy value;
> >>>   - use __aligned to align local array;
> >>>
> >>>   disk/part_efi.c |    7 ++++---
> >>>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/disk/part_efi.c b/disk/part_efi.c
> >>> index 9c33ae7..eb2cd57 100644
> >>> --- a/disk/part_efi.c
> >>> +++ b/disk/part_efi.c
> >>> @@ -225,7 +225,7 @@ static int set_protective_mbr(block_dev_desc_t
> >>> *dev_desc)
> >>>       p_mbr->signature = MSDOS_MBR_SIGNATURE;
> >>>       p_mbr->partition_record[0].sys_ind =
> >>> EFI_PMBR_OSTYPE_EFI_GPT; p_mbr->partition_record[0].start_sect =
> >>> 1;
> >>> -    p_mbr->partition_record[0].nr_sects = (u32) dev_desc->lba;
> >>> +    put_unaligned(dev_desc->lba,
> >>> &p_mbr->partition_record[0].nr_sects);
> >>>
> >>>       /* Write MBR sector to the MMC device */
> >>>       if (dev_desc->block_write(dev_desc->dev, 0, 1, p_mbr) != 1)
> >>> { @@ -361,6 +361,8 @@ int gpt_fill_pte(gpt_header *gpt_h,
> >>> gpt_entry *gpt_e, #ifdef CONFIG_PARTITION_UUIDS
> >>>       char *str_uuid;
> >>>   #endif
> >>> +    static efi_guid_t basic_guid __aligned(0x04) =
> >>> +        PARTITION_BASIC_DATA_GUID;
> >>>
> >>>       for (i = 0; i < parts; i++) {
> >>>           /* partition starting lba */
> >>> @@ -388,8 +390,7 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry
> >>> *gpt_e, gpt_e[i].ending_lba = cpu_to_le64(offset - 1);
> >>>
> >>>           /* partition type GUID */
> >>> -        memcpy(gpt_e[i].partition_type_guid.b,
> >>> -            &PARTITION_BASIC_DATA_GUID, 16);
> >>> +        gpt_e[i].partition_type_guid = basic_guid;
> >>>
> >>>   #ifdef CONFIG_PARTITION_UUIDS
> >>>           str_uuid = partitions[i].uuid;
> >>>
> > 
> >> Sorry, I sent my tested-by on a different thread:
> >> http://patchwork.ozlabs.org/patch/319649/
> > 
> >> Tested on i.MX6 (armv7) custom board and it is working fine without
> >> the -mno-unaligned-access flag.
> > 
> >> Tested-by: Hector Palacios <hector.palacios@digi.com>
> > 
> >> Replying to Tom's question in that other thread, regarding the
> >> issue:
> > 
> >>> Can you give me some steps on how to hit this bug?  I believe
> >>> it's a bug and I believe we need to fix it, I just want to
> >>> investigate a few things while we've got a trigger case right
> >>> now.  Thanks!
> > 
> >> GPT partition table needs to write a protective MBR to sector 0.
> >> The MBR structure has four partition entries (each occupying 16
> >> bytes) at unaligned offsets +1BEh, +1CEh, +1DEh, +1EEh (see [1]).
> >> The structure of each partition entry is defined at
> >> include/part_efi.h
> > 
> >> struct partition {
> >>     u8 boot_ind;        /* 0x80 - active */
> >>     u8 head;        /* starting head */
> >>     u8 sector;        /* starting sector */
> >>     u8 cyl;            /* starting cylinder */
> >>     u8 sys_ind;        /* What partition type */
> >>     u8 end_head;        /* end head */
> >>     u8 end_sector;        /* end sector */
> >>     u8 end_cyl;        /* end cylinder */
> >>     __le32 start_sect;    /* starting sector counting from 0 */
> >>     __le32 nr_sects;    /* nr of sectors in partition */
> >> } __packed;
> > 
> >> showing eight 1-byte fields and two 4-byte fields. Since the
> >> offsets for each partition entry are unaligned, the last two
> >> fields (which are 32bit) are unaligned as well. But it's not an
> >> error, it's just the specification of the MBR requires these
> >> fields to be at those exact offsets. So the usage of unaligned
> >> macros for accessing those fields is justified.
> > 
> > Right.  I would have sworn I used the GPT commands since we've
> > dropped -mno-unaligned-access but I'll just go re-test locally now
> > then, thanks.
> 
> Indeed I hadn't re-tested recently enough, thanks.

Have you managed to reproduce the problem on your setup?

I've reproduced it on Trats/Trats2 with ELDK 5.4 (gcc 4.7.2 armv7a
toolchain).

This patch fixes the problem.

> 
> - -- 
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
> 
> iQIcBAEBAgAGBQJTBMxeAAoJENk4IS6UOR1WmKAP/05tEc0Ix4vrbI+WEYv4s26v
> Up3Uj1yMHBgHhCmdUops6C6eNjoSX2Z2TzfFx8jQ1YCTrGXO229CSi0U/oXkTWCB
> lbdMxQ6IyKh1v+Z4MdEJBq43XxcGMVE8g75Z8Eb/7B74kHRvsLeLGIDMg4A3z1yB
> b95HOCaifhSykBELsrqWXpMhJr4KMkn+GT9lNA5umLJdHT/1A2pEglbkZWAu4tAt
> ybDpdpXI3Ai8eVg9NuMJXEAcYEGezlg55esFv57gJfLfBPM/e0WLF4MtZYyFJmC/
> 0SLe46OG19E6JzHMKrHngZbyVSC+Iwzh5Mw6vY40IyVxA6fpXZEZ119AyxLtP4m8
> BiWjjRAO65KC6qzRJJYzXKoXSD8Ky+VJ3ATWzUhVSeLQRHeE2am8JsT8ENj5dngC
> f93pzqTmOp9LPqOB81dlIbu+R8QoruX0mOQxygNy+7tpTTijLn+UUu23z165j42b
> qsBzrAddgFZzUxqfyJLHIr/bvrVqBpEP6BGv2i+5eA6Q/dPHMvVLV3hTZjnxtK52
> I6RVfK2R4uTX8mK+yN2HzBdqZhCJqDgxbWYUMuDIkVq7QeY5rtPBaFlOguLPqwx2
> +i38rtGj5abZcqr9ASDcmrfIoe1mIAj2NPuJejNLzbwyipyqRVO0CT+GG/FwXLLG
> LFWWTaNc/X4FMjGctcyr
> =1et2
> -----END PGP SIGNATURE-----



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group