[PATCH] uefi: add a helper to check for address overflow

Colin King colin.king at canonical.com
Thu Jan 14 14:44:08 UTC 2021


From: Colin Ian King <colin.king at canonical.com>

Cppcheck found some code that indeed was being optimized out,
so add an inline helper function do to the overflow checking
in a way that actually works.

Fixes cppcheck warning:
src/uefi/securebootcert/securebootcert.c:274:49: warning: Invalid test
for overflow 'var_data_addr+siglist.SignatureListSize<var_data_addr';
pointer overflow is undefined behavior. Some mainstream compilers
remove such overflow tests when optimising the code and assume it's
always false. [invalidTestForOverflow]:

  if (var_data_addr + siglist.SignatureListSize < var_data_addr)

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 src/uefi/securebootcert/securebootcert.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/uefi/securebootcert/securebootcert.c b/src/uefi/securebootcert/securebootcert.c
index 57c81eb4..edb5ed8f 100644
--- a/src/uefi/securebootcert/securebootcert.c
+++ b/src/uefi/securebootcert/securebootcert.c
@@ -256,6 +256,13 @@ static void securebootcert_deployed_mode(fwts_framework *fw, fwts_uefi_var *var,
 	}
 }
 
+static inline bool check_addr_overflow(
+	const void *var_data_addr,
+	const size_t len)
+{
+        return (len > ~(uintptr_t)0 - (uintptr_t)var_data_addr);
+}
+
 static bool check_sigdb_presence(uint8_t *var_data, size_t datalen, uint8_t *key, uint32_t key_len)
 {
 	uint8_t *var_data_addr;
@@ -271,7 +278,7 @@ static bool check_sigdb_presence(uint8_t *var_data, size_t datalen, uint8_t *key
 		siglist = *((EFI_SIGNATURE_LIST *)var_data_addr);
 
 		/* check for potential overflow */
-		if (var_data_addr + siglist.SignatureListSize < var_data_addr)
+		if (check_addr_overflow(var_data_addr, siglist.SignatureListSize))
 			break;
 
 		if (var_data_addr + siglist.SignatureListSize > var_data + datalen)
-- 
2.29.2




More information about the fwts-devel mailing list