Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a3e776
Removed the additional multiplication overflow detection.
azuev-java Nov 7, 2022
ee223fc
Streamlining structure allocation
azuev-java Nov 8, 2022
9673bac
Fixed casting in SAFE_SIZE_NEW_ARRAY2
azuev-java Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/java.desktop/share/native/common/awt/utility/sizecalc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,9 @@

#define IS_SAFE_SIZE_T(x) ((x) >= 0 && (unsigned long long)(x) <= SIZE_MAX)

#define IS_MUL_OVERFLOW(m, n) \
((m) != 0 && (n) != 0 && (((size_t)((m)*(n))) != (((size_t)(m)) * ((size_t)(n)))))

#define IS_SAFE_SIZE_MUL(m, n) \
(IS_SAFE_SIZE_T(m) && IS_SAFE_SIZE_T(n) && \
((m) == 0 || (n) == 0 || (size_t)(n) <= (SIZE_MAX / (size_t)(m))) && \
!IS_MUL_OVERFLOW(m, n))
((m) == 0 || (n) == 0 || (size_t)(n) <= (SIZE_MAX / (size_t)(m))))

#define IS_SAFE_SIZE_ADD(a, b) \
(IS_SAFE_SIZE_T(a) && IS_SAFE_SIZE_T(b) && (size_t)(b) <= (SIZE_MAX - (size_t)(a)))
Expand All @@ -75,10 +71,10 @@
* // Use the allocated array...
*/
#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)

#define SAFE_SIZE_ARRAY_REALLOC(func, p, m, n) \
(IS_SAFE_SIZE_MUL((m), (n)) ? ((func)((p), (m) * (n))) : FAILURE_RESULT)
(IS_SAFE_SIZE_MUL((m), (n)) ? ((func)((p), (size_t)(m) * (size_t)(n))) : FAILURE_RESULT)

/*
* A helper macro to safely allocate an array of type 'type' with 'n' items
Expand All @@ -92,19 +88,19 @@
* IS_SAFE_... macros to check if the calculations are safe.
*/
#define SAFE_SIZE_NEW_ARRAY(type, n) \
(IS_SAFE_SIZE_MUL(sizeof(type), (n)) ? (new type[(n)]) : throw std::bad_alloc())
(IS_SAFE_SIZE_MUL(sizeof(type), (n)) ? (new type[(size_t)(n)]) : throw std::bad_alloc())

#define SAFE_SIZE_NEW_ARRAY2(type, n, m) \
(IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_MUL(sizeof(type), (n) * (m)) ? \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we do not cast it here: (n) * (m)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed https://bugs.openjdk.org/browse/JDK-8296957

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it must be cast here too, I have missed it.

Would you mind submitting a bug?

(new type[(n) * (m)]) : throw std::bad_alloc())
(new type[(size_t)(n) * (size_t)(m)]) : throw std::bad_alloc())

/*
* Checks if a data structure of size (a + m*n) can be safely allocated
* w/o producing an integer overflow when calculating its size.
*/
#define IS_SAFE_STRUCT_SIZE(a, m, n) \
( \
IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_ADD((m) * (n), (a)) \
IS_SAFE_SIZE_MUL((m), (n)) && IS_SAFE_SIZE_ADD((size_t)(m) * (size_t)(n), (a)) \
)

/*
Expand All @@ -116,7 +112,7 @@
* // Use the allocated memory...
*/
#define SAFE_SIZE_STRUCT_ALLOC(func, a, m, n) \
(IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((a) + (m) * (n))) : FAILURE_RESULT)
(IS_SAFE_STRUCT_SIZE((a), (m), (n)) ? ((func)((size_t)(a) + (size_t)(m) * (size_t)(n))) : FAILURE_RESULT)


#endif /* SIZECALC_H */
Expand Down