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

[Patrick Chen]

But you forgot that    (IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((a) +
(size_t)(m) * (size_t)(n))) : FAILURE_RESULT) is not the same equivalence
to    (IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((size_t)(a) +
(size_t)(m) * (size_t)(n))) : FAILURE_RESULT)
because of the ((func)((size_t)(a) + (size_t)(m) * (size_t)(n))

Le mar. 8 nov. 2022 à 14:59, Alexey Ivanov <aivanov at openjdk.org> a écrit :

> On Mon, 7 Nov 2022 22:04:55 GMT, Alexander Zuev <kizune at openjdk.org>
> wrote:
>
> > Removed the additional multiplication overflow detection.
> > Instead cast all the parameters to type_t just the way they are treated
> in the existing size check macro.
> > This way there is no possibility to accidentally provide parameters that
> will pass the size check macro while being cast to size_t there but then
> due to the missing cast cause the wrong size passed the actual allocation
> function.
> > Since this checking macro was used in couple of different places all of
> them needs to be updated in the similar way.
>
> Changes requested by aivanov (Reviewer).
>
> 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`.
>
> src/java.desktop/share/native/common/awt/utility/sizecalc.h line 115:
>
> > 113:  */
> > 114: #define SAFE_SIZE_STRUCT_ALLOC(func, a, m, n) \
> > 115:     (IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((a) + (size_t)(m)
> * (size_t)(n))) : FAILURE_RESULT)
>
> Suggestion:
>
>     (IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((size_t)(a) +
> (size_t)(m) * (size_t)(n))) : FAILURE_RESULT)
>
> To be safe, `a` should also be cast.
>
> And `IS_SAFE_STRUCT_SIZE` should also be updated to pass `(size_t)(m) *
> (size_t)(n)` to `IS_SAFE_SIZE_ADD` instead of `(m) * (n)`.
>
> -------------
>
> PR: https://git.openjdk.org/jdk/pull/11030
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/client-libs-dev/attachments/20221108/b0a36934/attachment.htm>