Re: [PATCH] device-tree/kernel: Use memcpy in output_length()

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 22/07/2025 06:50, Jason Andryuk wrote:
> On 2025-07-22 03:46, Michal Orzel wrote:
>> Trying to boot a compressed kernel with UBSAN enabled, results in the
>> following warning:
>> (XEN) UBSAN: Undefined behaviour in common/device-tree/kernel.c:21:12
>> (XEN) load of misaligned address 00000a0040f89867 for type 'uint32_t'
>> (XEN) which requires 4 byte alignment
>> ...
>> (XEN)    [<00000a0000529964>] kernel_decompress+0x2bc/0x5bc
>> (XEN)    [<00000a000052a354>] kernel_probe+0x6f0/0x734
>> (XEN)    [<00000a0000528714>] dom0less-build.c#construct_domU+0x188/0x9d8
>>
>> If &image[image_len - 4] is not aligned to 4B boundary it causes
>> unaligned access which is undefined behavior on Arm. Use memcpy instead
>> to be safe.
>>
>> Fixes: c1be0b102e0e ("xen/arm: support gzip compressed kernels")
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>> ---
>>   xen/common/device-tree/kernel.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/common/device-tree/kernel.c 
>> b/xen/common/device-tree/kernel.c
>> index ef393182b691..28096121a52d 100644
>> --- a/xen/common/device-tree/kernel.c
>> +++ b/xen/common/device-tree/kernel.c
>> @@ -18,7 +18,11 @@
>>   
>>   static uint32_t __init output_length(char *image, unsigned long image_len)
>>   {
>> -    return *(uint32_t *)&image[image_len - 4];
> 
> Maybe just:
>      return get_unaligned_le32(&image[image_len - 4]);
> 
> You'll also need:
> #include <xen/unaligned.h>
> 
> The gzip size is little endian, 
I thought about this solution but decided to use memcpy because one day we may
want to support other compression algorithms and forcing LE could cause issues.

I'm ok either way. Let other maintainers decide.

~Michal