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.

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

detect: free all tenant detect engines #10219

Closed
wants to merge 4 commits into from

Conversation

jlucovsky
Copy link
Contributor

@jlucovsky jlucovsky commented Jan 22, 2024

Continuation of #10217

Free all tenants registered in the master.

(cherry picked from commit a4d80bc)

Link to redmine ticket: 6549

Describe changes:

  • Free all tenant detect engines (and indirect references)

Updates:

  • Added --debug-failed to suricata-verify tests.

Provide values to any of the below to override the defaults.

To use a pull request use a branch name like pr/N where N is the
pull request number.

Alternatively, SV_BRANCH may also be a link to an
OISF/suricata-verify pull-request.

SV_REPO=
SV_BRANCH=pr/1600
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_cfg.

Pipeline 17678

@catenacyber
Copy link
Contributor

Do you know why SV fails for multi-tenant test on MacOS ?

@jlucovsky
Copy link
Contributor Author

Do you know why SV fails for multi-tenant test on MacOS ?

I'm looking at that. I don't see anything that jumps out.

I don't have a mac build environment (yet) and the diag info in the log doesn't indicate the failure cause.

victorjulien and others added 3 commits January 23, 2024 10:05
Free all tenants registered in the master.

(cherry picked from commit a4d80bc)
Fixes file loading for rule files and Lua scripts.

Bug: OISF#6095.
(cherry picked from commit 04aee5f)
@catenacyber
Copy link
Contributor

I can confirm the failure

==33006==ERROR: AddressSanitizer: unknown-crash on address 0x70000d6efb80 at pc 0x000103b75f0e bp 0x70000d6efb50 sp 0x70000d6efb48
READ of size 8 at 0x70000d6efb80 thread T1

Not so much info...

@catenacyber
Copy link
Contributor

Fails the same way without this PR on master6

src/suricata.c Outdated Show resolved Hide resolved
@victorjulien
Copy link
Member

Another stack size issue?

Thread 2 "DL#01" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 31498]
memset () at src/string/aarch64/memset.S:94
warning: 94     src/string/aarch64/memset.S: No such file or directory
(gdb) bt
#0  memset () at src/string/aarch64/memset.S:94
#1  0x00000055949fa244 in SCACCreateFailureTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:510
#2  0x00000055949fad60 in SCACPrepareStateTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:726
#3  0x00000055949fb33c in SCACPreparePatterns (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:823
#4  0x00000055948df16c in DetectMpmPrepareAppMpms (de_ctx=0x7f9094e0e0) at detect-engine-mpm.c:277
#5  0x00000055948d6b90 in SigGroupBuild (de_ctx=0x7f9094e0e0) at detect-engine-build.c:1945
#6  0x00000055948dd954 in SigLoadSignatures (de_ctx=0x7f9094e0e0, sig_file=0x0, sig_file_exclusive=0) at detect-engine-loader.c:357
#7  0x00000055948c6220 in DetectEngineMultiTenantLoadTenant (tenant_id=1, filename=0x7f91c52a40 "a.yaml", loader_id=0) at detect-engine.c:3337
#8  0x00000055948c6524 in DetectLoaderFuncLoadTenant (vctx=0x7f917f3770, loader_id=0) at detect-engine.c:3418
#9  0x00000055948de1a8 in DetectLoader (th_v=0x7f917e77d0, thread_data=0x7f915257c0) at detect-engine-loader.c:572
#10 0x00000055949cb864 in TmThreadsManagement (td=0x7f917e77d0) at tm-threads.c:558
#11 0x0000007f91c0e118 in start (p=0x7f909ed720) at src/thread/pthread_create.c:207
#12 0x0000007f91c0c4c8 in __clone () at src/thread/aarch64/clone.s:28
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) f 1
#1  0x00000055949fa244 in SCACCreateFailureTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:510
510         memset(&q, 0, sizeof(StateQueue));
(gdb) p q
value of type `StateQueue' requires 262152 bytes, which is more than max-value-size

@victorjulien
Copy link
Member

Another stack size issue?

Thread 2 "DL#01" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 31498]
memset () at src/string/aarch64/memset.S:94
warning: 94     src/string/aarch64/memset.S: No such file or directory
(gdb) bt
#0  memset () at src/string/aarch64/memset.S:94
#1  0x00000055949fa244 in SCACCreateFailureTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:510
#2  0x00000055949fad60 in SCACPrepareStateTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:726
#3  0x00000055949fb33c in SCACPreparePatterns (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:823
#4  0x00000055948df16c in DetectMpmPrepareAppMpms (de_ctx=0x7f9094e0e0) at detect-engine-mpm.c:277
#5  0x00000055948d6b90 in SigGroupBuild (de_ctx=0x7f9094e0e0) at detect-engine-build.c:1945
#6  0x00000055948dd954 in SigLoadSignatures (de_ctx=0x7f9094e0e0, sig_file=0x0, sig_file_exclusive=0) at detect-engine-loader.c:357
#7  0x00000055948c6220 in DetectEngineMultiTenantLoadTenant (tenant_id=1, filename=0x7f91c52a40 "a.yaml", loader_id=0) at detect-engine.c:3337
#8  0x00000055948c6524 in DetectLoaderFuncLoadTenant (vctx=0x7f917f3770, loader_id=0) at detect-engine.c:3418
#9  0x00000055948de1a8 in DetectLoader (th_v=0x7f917e77d0, thread_data=0x7f915257c0) at detect-engine-loader.c:572
#10 0x00000055949cb864 in TmThreadsManagement (td=0x7f917e77d0) at tm-threads.c:558
#11 0x0000007f91c0e118 in start (p=0x7f909ed720) at src/thread/pthread_create.c:207
#12 0x0000007f91c0c4c8 in __clone () at src/thread/aarch64/clone.s:28
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) f 1
#1  0x00000055949fa244 in SCACCreateFailureTable (mpm_ctx=0x7f90a1a5a0) at util-mpm-ac.c:510
510         memset(&q, 0, sizeof(StateQueue));
(gdb) p q
value of type `StateQueue' requires 262152 bytes, which is more than max-value-size

Backporting 92fce2f fixes it.

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_cfg.

Pipeline 17724

So that we can have multi-tenant on MacOS without stack
overflows because of the size of the structure...

Ticket: OISF#6263.
(cherry picked from commit 92fce2f)
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_cfg.

Pipeline 17727

@jlucovsky jlucovsky marked this pull request as ready for review January 24, 2024 14:17
@jlucovsky jlucovsky requested a review from a team as a code owner January 24, 2024 14:17
@victorjulien
Copy link
Member

Why is there a QA failure?

@victorjulien
Copy link
Member

Also, force push, so please keep it draft.

@victorjulien victorjulien marked this pull request as draft January 24, 2024 18:01
@jlucovsky
Copy link
Contributor Author

Also, force push, so please keep it draft.

I'm marking this as 'ready for review' as all needed changes are present.

@jlucovsky jlucovsky marked this pull request as ready for review January 28, 2024 14:00
Copy link
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

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

please submit a clean PR w/o debug commits

@jlucovsky
Copy link
Contributor Author

please submit a clean PR w/o debug commits

The additional information generated on s-v failures has been helpful and matches the other branches. I'd like to retain the changes to builds.yml

@victorjulien
Copy link
Member

Please submit a clean PR with only the MT fixes. This can still make 6.0.16, it originally missed it due to not the PR not being clean.

@jlucovsky
Copy link
Contributor Author

Continued in #10311

@jlucovsky jlucovsky deleted the 6549/5 branch February 8, 2024 14:08
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.

4 participants