Skip to content

Stack 8001 v13#14850

Closed
catenacyber wants to merge 4 commits intoOISF:mainfrom
catenacyber:stack-8001-v13
Closed

Stack 8001 v13#14850
catenacyber wants to merge 4 commits intoOISF:mainfrom
catenacyber:stack-8001-v13

Conversation

@catenacyber
Copy link
Copy Markdown
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/8001

Describe changes: improve stack allocations

#14821 next batch

Still todo after :

  • handle other cases of git grep '\];' src/*.c | grep -v = | grep -v '[0-9]\];' | grep -v '[A-Z]\];' | grep -v return | grep -v g_alproto_max | grep -v '\[\];' | grep -v sizeof > src/detect.c
  • Allocate arrays on the heap instead of the stack when the size comes from buffer_type_id which is a legit u32

Before stack allocation

Ticket: 8001
Before doing stack allocation

Ticket: 8001
Before stack allocation

Ticket: 8001
To avoid running a big (when many signatures) stack allocation
on each detection loop with postmatches

Ticket: 8001
Comment thread src/detect-parse.c
if (strlen(parser->opts) > 0) {
size_t buffer_size = strlen(parser->opts) + 1;
if (buffer_size > UINT16_MAX) {
SCLogError("Too long options for signature : %zu>%d", buffer_size, UINT16_MAX);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is %zu portable? We normally use the PRI macros

also, "too long options" doesn't sound great

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What is the PRI macro for size_t ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the pattern is PRIuMAX .. (uintmax_t)var

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replacing with a debug validation as we have DETECT_MAX_RULE_SIZE as a global bound

Comment thread src/detect-parse.c
/* we can have no options, so make sure we have them */
if (strlen(parser->opts) > 0) {
size_t buffer_size = strlen(parser->opts) + 1;
if (buffer_size > UINT16_MAX) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a rule has a max len of 8k? So this would be unreachable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah I see DETECT_MAX_RULE_SIZE, will look again at this

Comment thread src/detect-metadata.c
return NULL;

// a rule can not have that many metadata
DEBUG_VALIDATE_BUG_ON(cnt > UINT16_MAX);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems impossible, as long as the rule len max is 8k.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will look again with DETECT_MAX_RULE_SIZE

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 29763

@catenacyber catenacyber marked this pull request as draft February 20, 2026 10:49
@catenacyber
Copy link
Copy Markdown
Contributor Author

Draft : see inline comments

@catenacyber catenacyber mentioned this pull request Mar 4, 2026
@catenacyber
Copy link
Copy Markdown
Contributor Author

Replaced by #14939

@catenacyber catenacyber closed this Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants