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

[Alex Hung]

On 2021-01-14 7:44 a.m., Colin King wrote:
> 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)
> 


Acked-by: Alex Hung <alex.hung at canonical.com>