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