Skip to content
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

avoid setting LIMIT_HEAP incorrectly since d90fb23 (included in PCRE2 10.41) #184

Merged
merged 1 commit into from
Jan 17, 2023
Merged

avoid setting LIMIT_HEAP incorrectly since d90fb23 (included in PCRE2 10.41) #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/pcre2_intmodedep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ doing traditional NFA matching (pcre2_match() and friends). */

typedef struct match_block {
pcre2_memctl memctl; /* For general use */
PCRE2_SIZE heap_limit; /* As it says */
uint32_t heap_limit; /* As it says */
Copy link
Collaborator

Choose a reason for hiding this comment

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

PCRE2_SIZE is 64 bit long on 64 bit systems, isn't it?

Copy link
Contributor Author

@carenas carenas Dec 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

correct, but the heap_limit value can only be lower than UINT32_MAX even if it can represent memory sizes larger than 4GB in 64-bit systems because it uses KB for units.

the main point of this change (other than making it consistent with the other references and saving a little bit of memory) is to try to get the compiler to help us in the future to detect places where similar truncations could be introduced.

uint32_t match_limit; /* As it says */
uint32_t match_limit_depth; /* As it says */
uint32_t match_call_count; /* Number of times a new frame is created */
Expand Down Expand Up @@ -911,7 +911,7 @@ typedef struct dfa_match_block {
PCRE2_SPTR last_used_ptr; /* Latest consulted character */
const uint8_t *tables; /* Character tables */
PCRE2_SIZE start_offset; /* The start offset value */
PCRE2_SIZE heap_limit; /* As it says */
uint32_t heap_limit; /* As it says */
PCRE2_SIZE heap_used; /* As it says */
uint32_t match_limit; /* As it says */
uint32_t match_limit_depth; /* As it says */
Expand Down
38 changes: 27 additions & 11 deletions src/pcre2_match.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,13 +665,28 @@ N = (heapframe *)((char *)F + frame_size);
if (N >= frames_top)
{
heapframe *new;
PCRE2_SIZE newsize = match_data->heapframes_size * 2;
PCRE2_SIZE newsize;

if (newsize > mb->heap_limit)
if (match_data->heapframes_size >= PCRE2_SIZE_MAX / 2)
{
PCRE2_SIZE maxsize = (mb->heap_limit/frame_size) * frame_size;
if (match_data->heapframes_size >= maxsize) return PCRE2_ERROR_HEAPLIMIT;
newsize = maxsize;
if (match_data->heapframes_size == PCRE2_SIZE_MAX - 1)
return PCRE2_ERROR_NOMEMORY;
newsize = PCRE2_SIZE_MAX - 1;
}
else
newsize = match_data->heapframes_size * 2;

if (newsize / 1024 >= mb->heap_limit)
{
PCRE2_SIZE old_size = match_data->heapframes_size / 1024;
if (mb->heap_limit <= old_size) return PCRE2_ERROR_HEAPLIMIT;
else
{
PCRE2_SIZE max_delta = 1024 * (mb->heap_limit - old_size);
int over_bytes = match_data->heapframes_size % 1024;
if (over_bytes) max_delta -= (1024 - over_bytes);
newsize = match_data->heapframes_size + max_delta;
}
}

new = match_data->memctl.malloc(newsize, match_data->memctl.memory_data);
Expand Down Expand Up @@ -6801,7 +6816,7 @@ the pattern. It is not used at all if there are no capturing parentheses.

frame_size is the total size of each frame
match_data->heapframes is the pointer to the frames vector
match_data->heapframes_size is the total size of the vector
match_data->heapframes_size is the allocated size of the vector

We must pad the frame_size for alignment to ensure subsequent frames are as
aligned as heapframe. Whilst ovector is word-aligned due to being a PCRE2_SIZE
Expand All @@ -6816,7 +6831,7 @@ frame_size = (offsetof(heapframe, ovector) +
smaller. */

mb->heap_limit = ((mcontext->heap_limit < re->limit_heap)?
mcontext->heap_limit : re->limit_heap) * 1024;
mcontext->heap_limit : re->limit_heap);

mb->match_limit = (mcontext->match_limit < re->limit_match)?
mcontext->match_limit : re->limit_match;
Expand All @@ -6832,14 +6847,15 @@ the size to a multiple of the frame size. */

heapframes_size = frame_size * 10;
if (heapframes_size < START_FRAMES_SIZE) heapframes_size = START_FRAMES_SIZE;
if (heapframes_size > mb->heap_limit)
if (heapframes_size / 1024 > mb->heap_limit)
{
if (frame_size > mb->heap_limit ) return PCRE2_ERROR_HEAPLIMIT;
heapframes_size = mb->heap_limit;
PCRE2_SIZE max_size = 1024 * mb->heap_limit;
if (max_size < frame_size) return PCRE2_ERROR_HEAPLIMIT;
heapframes_size = max_size;
}

/* If an existing frame vector in the match_data block is large enough, we can
use it.Otherwise, free any pre-existing vector and get a new one. */
use it. Otherwise, free any pre-existing vector and get a new one. */

if (match_data->heapframes_size < heapframes_size)
{
Expand Down