Re: [PATCH 5/5] commands: fix unaligned accesses of aliased commands on amd64

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 8/23/19 9:36 AM, Sascha Hauer wrote:
> On Thu, Aug 22, 2019 at 08:19:41AM +0200, Ahmad Fatoum wrote:
>> If a command has aliases (e.g. edit and sedit), register_command
>> allocates a new struct command for the alias. As struct command has a
>> alignment of 64 specified on __x86_64__, this new allocation needs to
>> observe the alignment lest unaligned access could occur. I don't think
>> it's likely that GCC would generate SIMD code here that expects a 64 byte
>> alignment, but heed UBSan's advice and use xmemalign with the appropriate
>> alignment.
>>
>> Fixes: 8c14b97758 ("svn_rev_477")
>> Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
>> ---
>> I am not sure about this, because I don't understand why there was a 64
>> bit alignment in the x86_64 linker script in the first place.
>> The fix is trivial though, so even if it's only a theoretical, lets have
>> it?
> 
> Have you tried removing the alignment? AFAIK without it we are not able
> to iterate over the commands array generated by the linker. I don't know
> if this issue is still present. There is no requirement to align a
> struct command to 64bit otherwise.

Ah, I see it now. The issue isn't the first element's alignment, but the
follow up ones. sizeof(struct command) is 88 on Linux amd64, but the linker
without being told anything decided to leave 128 between the start of each
element, while barebox assumes a stride of sizeof(struct command) and thus
it trips over while parsing the second command.

Hmm, please drop this patch here for now.

Thanks
Ahmad


> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox