[PATCH u-boot] api: fix a potential serious bug caused by undef CONFIG_SYS_64BIT_LBA

The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA.
But api/api_storage.c includes this header before including part.h, causing the type of lbaint_t and subsequently the type signature of blk_dread() and blk_dwrite() functions to change from the rest of U-Boot (if CONFIG_SYS_64BIT_LBA is defined for the board).
This is of course wrong, because the call to blk_dread() / blk_dwrite() will recieve mangled arguments.
Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead make the immediate code do what it would do as if the macro was not defined.
Add a FIXME to whoever is maintaining this code.
CI managed to trigger this bug when compiling for lsxhl_defconfig, which has CONFIG_API selected. The compiler complained about blk_dwrite() and blk_dread() not matching original declarations:
include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match original declaration [-Wlto-type-mismatch] 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^ drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^
Signed-off-by: Marek Behún marek.behun@nic.cz --- include/api_public.h | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/api_public.h b/include/api_public.h index def103ce22..5a4465ea89 100644 --- a/include/api_public.h +++ b/include/api_public.h @@ -70,12 +70,25 @@ struct sys_info { int mr_no; /* number of memory regions */ };
-#undef CONFIG_SYS_64BIT_LBA -#ifdef CONFIG_SYS_64BIT_LBA -typedef u_int64_t lbasize_t; -#else +/* + * FIXME: Previously this code was: + * + * #undef CONFIG_SYS_64BIT_LBA + * #ifdef CONFIG_SYS_64BIT_LBA + * typedef u_int64_t lbasize_t; + * #else + * typedef unsigned long lbasize_t; + * #endif + * + * But we cannot just undefine CONFIG_SYS_64BIT_LBA, because then in + * api/api_storage.c the type signature of lbaint_t will be different if + * CONFIG_SYS_64BIT_LBA is enabled for the board, which can result in various + * bugs. + * So simply define lbasize_t as an unsigned long, since this was what was done + * anyway for at least 13 years, but don't undefine CONFIG_SYS_64BIT_LBA. + */ typedef unsigned long lbasize_t; -#endif + typedef unsigned long lbastart_t;
#define DEV_TYP_NONE 0x0000

On Sat, 6 Mar 2021 at 15:43, Marek Behún marek.behun@nic.cz wrote:
The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA.
But api/api_storage.c includes this header before including part.h, causing the type of lbaint_t and subsequently the type signature of blk_dread() and blk_dwrite() functions to change from the rest of U-Boot (if CONFIG_SYS_64BIT_LBA is defined for the board).
This is of course wrong, because the call to blk_dread() / blk_dwrite() will recieve mangled arguments.
Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead make the immediate code do what it would do as if the macro was not defined.
Add a FIXME to whoever is maintaining this code.
CI managed to trigger this bug when compiling for lsxhl_defconfig, which has CONFIG_API selected. The compiler complained about blk_dwrite() and blk_dread() not matching original declarations:
include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match original declaration [-Wlto-type-mismatch] 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^ drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^
Signed-off-by: Marek Behún marek.behun@nic.cz
include/api_public.h | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Mar 06, 2021 at 11:43:22PM +0100, Marek Behún wrote:
The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA.
But api/api_storage.c includes this header before including part.h, causing the type of lbaint_t and subsequently the type signature of blk_dread() and blk_dwrite() functions to change from the rest of U-Boot (if CONFIG_SYS_64BIT_LBA is defined for the board).
This is of course wrong, because the call to blk_dread() / blk_dwrite() will recieve mangled arguments.
Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead make the immediate code do what it would do as if the macro was not defined.
Add a FIXME to whoever is maintaining this code.
CI managed to trigger this bug when compiling for lsxhl_defconfig, which has CONFIG_API selected. The compiler complained about blk_dwrite() and blk_dread() not matching original declarations:
include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match original declaration [-Wlto-type-mismatch] 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^ drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st | ^
Signed-off-by: Marek Behún marek.behun@nic.cz Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Marek Behún
-
Simon Glass
-
Tom Rini