Paolo Bonzini <pbonz...@redhat.com> writes: > On 02/08/21 14:36, Peter Maydell wrote: >> Reviewed-by: Peter Maydell<peter.mayd...@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. >> 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 [...] 4.188.1. Overview Supported Languages: C, C++, Objective-C, Objective-C++ INTEGER_OVERFLOW finds many cases of integer overflows and truncations resulting from arithmetic operations. Some forms of integer overflow can cause security vulnerabilities, for example, when the overflowed value is used as the argument to an allocation function. [...] [...] Disabled by default: INTEGER_OVERFLOW is disabled by default. To enable it, you can use the --enable option to the cov-analyze command. 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". Our model for g_malloc_n() uses __coverity_alloc(), which should provide "tainted sink". > making nmemb a tainted sink could be an interesting way to ensure that > untrusted data does not end up causing such a failure. > > Likewise, we should try making __bufwrite taint the buffer it is > writing to; there's already a TODO for that but I never followed up on > it. __bufwrite() tells Coverity that the buf[len] bytes are overwritten with unspecified data. I'd expect that to taint the buffer already.