]> git.dujemihanovic.xyz Git - u-boot.git/commitdiff
api: fix a potential serious bug caused by undef CONFIG_SYS_64BIT_LBA
authorMarek Behún <marek.behun@nic.cz>
Sat, 6 Mar 2021 22:43:22 +0000 (23:43 +0100)
committerTom Rini <trini@konsulko.com>
Fri, 19 Mar 2021 14:36:06 +0000 (10:36 -0400)
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 receive 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>
include/api_public.h

index def103ce22fd8f3cf2fde87e04252416db085e10..5a4465ea893d97aadafca12cf05f6a4cfe4800f8 100644 (file)
@@ -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