RFR: 8296496: Overzealous check in sizecalc.h prevents large memory allocation

Alexey Ivanov aivanov at openjdk.org
Tue Nov 8 22:39:22 UTC 2022


On Tue, 8 Nov 2022 21:38:54 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

>> src/java.desktop/share/native/common/awt/utility/sizecalc.h line 95:
>> 
>>> 93: #define SAFE_SIZE_NEW_ARRAY2(type, n, m) \
>>> 94:     (IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_MUL(sizeof(type), (n) * (m)) ? \
>>> 95:      (new type[(size_t)((n) * (m))]) : throw std::bad_alloc())
>> 
>> Suggestion:
>> 
>>      (new type[(size_t)(n) * (size_t)(m)]) : throw std::bad_alloc())
>> 
>> Each parameter must be cast as in `SAFE_SIZE_ARRAY_ALLOC`.
>
>> Each parameter must be cast as in SAFE_SIZE_ARRAY_ALLOC.
> 
> If we do that then logic might be broken since we checking for the limits against the (size_t)(m * n) but performing call with ((size_t)(m) * (size_t)(n)) which might add potential point of failure. I prefer to do the conversions in the same way we do them in checks.

It's [what you just did](https://github.com/openjdk/jdk/pull/11030/files#diff-f4ac4382758a5b83444f9483cbc756ca6572fddf4c3811f2f5d86002a91599c2L77-R74) for `SAFE_SIZE_ARRAY_ALLOC`:

 #define SAFE_SIZE_ARRAY_ALLOC(func, m, n) \
-    (IS_SAFE_SIZE_MUL((m), (n)) ? ((func)((m) * (n))) : FAILURE_RESULT)
+    (IS_SAFE_SIZE_MUL((m), (n)) ? ((func)((size_t)(m) * (size_t)(n))) : FAILURE_RESULT)

You cast both `m` and `n`.

This code basically does the same. If you don't cast each parameter, the result may be different, since there are cases where `((size_t)(m) * (size_t)(n))` is not equal to `(size_t)((m) * (n))`.

-------------

PR: https://git.openjdk.org/jdk/pull/11030



More information about the client-libs-dev mailing list