git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] bswap: convert to unsigned before shifting in get_be32

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
Jul 16, 2017, 00:23 UTC
Message-ID
<5fd84b40-9dde-0fd2-694c-49a073e87495@ramsayjones.plus.com>
In-Reply-To
<56f1263c-a0b4-260b-7e23-881e21119041@web.de>
On 15/07/17 20:11, René Scharfe wrote:
Show 33 quoted lines
> The pointer p is dereferenced and we get an unsigned char.  Before
> shifting it's automatically promoted to int.  Left-shifting a signed
> 32-bit value bigger than 127 by 24 places is undefined.  Explicitly
> convert to a 32-bit unsigned type to avoid undefined behaviour if
> the highest bit is set.
> 
> Found with Clang's UBSan.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  compat/bswap.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/compat/bswap.h b/compat/bswap.h
> index d47c003544..4582c1107a 100644
> --- a/compat/bswap.h
> +++ b/compat/bswap.h
> @@ -166,10 +166,10 @@ static inline uint64_t git_bswap64(uint64_t x)
>  	(*((unsigned char *)(p) + 0) << 8) | \
>  	(*((unsigned char *)(p) + 1) << 0) )
>  #define get_be32(p)	( \
> -	(*((unsigned char *)(p) + 0) << 24) | \
> -	(*((unsigned char *)(p) + 1) << 16) | \
> -	(*((unsigned char *)(p) + 2) <<  8) | \
> -	(*((unsigned char *)(p) + 3) <<  0) )
> +	((uint32_t)*((unsigned char *)(p) + 0) << 24) | \
> +	((uint32_t)*((unsigned char *)(p) + 1) << 16) | \
> +	((uint32_t)*((unsigned char *)(p) + 2) <<  8) | \
> +	((uint32_t)*((unsigned char *)(p) + 3) <<  0) )
>  #define put_be32(p, v)	do { \
>  	unsigned int __v = (v); \
>  	*((unsigned char *)(p) + 0) = __v >> 24; \
> 

Heh, I have a patch that is pretty much identical. I suspect you can guess why. ;-)

ATB, Ramsay Jones

Previous: Jeff King
Message 5 of 5 in “bswap: convert to unsigned before shifting in get_be32”
  1. bswap: convert to unsigned before shifting in get_be32René Scharfe, Jul 15, 2017
  2. 2/1 bswap: convert get_be16, get_be32 and put_be32 to inline functionsRené Scharfe, Jul 15, 2017
  3. Jeff KingJul 16, 2017
  4. Jeff KingJul 16, 2017
  5. Ramsay JonesJul 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.