From: Jeff King <peff@peff.net>
Date: Sat, 03 Dec 2016 05:17:21 GMT
Subject: Re: [PATCH 2/4] shallow.c: avoid theoretical pointer wrap-around
Message-ID: <20161203051720.z5elgoaapulqxlw5@sigill.intra.peff.net>
In-Reply-To: <1480710664-26290-2-git-send-email-rv@rasmusvillemoes.dk>

On Fri, Dec 02, 2016 at 09:31:02PM +0100, Rasmus Villemoes wrote:

> The expression info->free+size is technically undefined behaviour in
> exactly the case we want to test for. Moreover, the compiler is likely
> to translate the expression to
> 
>   (unsigned long)info->free + size > (unsigned long)info->end
> 
> where there's at least a theoretical chance that the LHS could wrap
> around 0, giving a false negative.
> 
> This might as well be written using pointer subtraction avoiding these
> issues.
> [...]
>
> -	if (!info->slab_count || info->free + size > info->end) {
> +	if (!info->slab_count || size > info->end - info->free) {

Yeah, I agree the correct way to write this is to compare the sizes
directly. That is how overflow checks _must_ be written. This one is
less likely to overflow, but even computing the value more than one past
the end of the array is technically undefined.

-Peff