On 15/07/17 20:11, René Scharfe wrote:
> 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@xxxxxx>
> ---
> 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
