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

Tom Rini
2014-02-19T15:23:10Z

-----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.

- --
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-----