Re: [PATCH 4/6] coverity-model: clean up the models for array allocation functions

[Peter Maydell]

On Wed, 4 Aug 2021 at 09:35, Markus Armbruster <armbru@redhat.com> wrote:
>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On 02/08/21 14:36, Peter Maydell wrote:
> >> Reviewed-by: Peter Maydell<peter.maydell@linaro.org>
> >> The real g_malloc_n() returns failure if the multiplication
> >> would overflow;
>
> Really?  Its documentation:
>
>     This function is similar to g_malloc(), allocating (n_blocks *
>     n_block_bytes) bytes, but care is taken to detect possible overflow
>     during multiplication.
>
> There's also g_try_malloc_n():
>
>     This function is similar to g_try_malloc(), allocating (n_blocks *
>     n_block_bytes) bytes, but care is taken to detect possible overflow
>     during multiplication.
>
> I read this as g_malloc_n() can return null only for zero size, and
> crashes on error, just like g_malloc_n().  g_try_malloc_n() behaves like
> g_try_malloc_n(), i.e. it returns null on failure.

Yeah, I misspoke -- I just meant to say "treats multiply overflow
like an allocation failure", not that it returns NULL.

> >>                 I guess Coverity currently doesn't have any
> >> warnings it generates as a result of assuming overflow
> >> might happen?
> >
> > I couldn't find any Coverity-specific way to detect overflow, but
>
> Does it need one?
>
> Quick peek at Coverity 2012.12 Checker Reference:
>
>     4.188. INTEGER_OVERFLOW

> The example that follows shows a memory allocation where the size is
> computed like TAINTED * sizeof(MUMBLE), where TAINTED is a "tainted
> source", and the allocation's size is a "tainted sink".

My point was that we *don't* want Coverity to report INTEGER_OVERFLOW
errors based on the way our g_malloc_n() model is written like this:
  __coverity_alloc__(nmemb * size)
That expression in our model *can* overflow. We know the real g_malloc_n()
cannot overflow, but we don't do anything to tell Coverity that, so it's
possible that (maybe in future) it will report false positives to us
based on analysis that assumes possible overflows in the multiplication.

thanks
-- PMM