[PATCH] linker_list: Fix ll_entry_get alignment

Sean Anderson seanga2 at gmail.com
Sat Sep 30 17:22:53 CEST 2023


On 9/30/23 10:36, Sean Anderson wrote:
> When ll_entry_get is used on a list entry ll_entry_declare'd in the same
> file, the lack of alignment on the access will override the
> ll_entry_declare alignment. This causes GCC to use the default section
> alignment of 32 bytes. As list entries are not necessarily 32-byte aligned,
> this will cause a gap in the linker list, corrupting further entries.
> 
> As a specific example, get_fs_loader uses DM_DRIVER_GET(fs_loader) in the
> same file where U_BOOT_DRIVER(fs_loader) is present. This causes a crash
> when walking the driver list.
> 
> Fix this by adding appropriate alignment to all accesses.
> 
> Fixes: 42ebaae3a33 ("common: Implement support for linker-generated arrays")
> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> ---
> 
>   include/linker_lists.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linker_lists.h b/include/linker_lists.h
> index f9a2ee0c762..e0c8a01b9ba 100644
> --- a/include/linker_lists.h
> +++ b/include/linker_lists.h
> @@ -209,7 +209,8 @@
>    */
>   #define ll_entry_get(_type, _name, _list)				\
>   	({								\
> -		extern _type _u_boot_list_2_##_list##_2_##_name;	\
> +		extern _type __aligned(4)				\
> +			_u_boot_list_2_##_list##_2_##_name;		\
>   		_type *_ll_result =					\
>   			&_u_boot_list_2_##_list##_2_##_name;		\
>   		_ll_result;						\
> @@ -229,7 +230,7 @@
>    * @_list: name of the list
>    */
>   #define ll_entry_ref(_type, _name, _list)				\
> -	((_type *)&_u_boot_list_2_##_list##_2_##_name)
> +	((_type __aligned(4) *)&_u_boot_list_2_##_list##_2_##_name)

OK, so this causes an error in clang. And it isn't really necessary
because the entry is already declared at this point.

So I guess the right fix is to replace DM_DRIVER_GET with DM_DRIVER_REF in
get_fs_loader. But this seems like a really big footgun. You can use the
wrong one and there are no errors except at runtime. I wonder if we can add
a warning of some kind?

--Sean

>   
>   /**
>    * ll_start() - Point to first entry of first linker-generated array



More information about the U-Boot mailing list