Skip to content

Conversation

AirisX
Copy link
Contributor

@AirisX AirisX commented Dec 22, 2017

I did a little research and found out that the rules that are initialized in the pool by the msc_rules_add_file or msc_rules_add can not be freed by the msc_rules_cleanup because ngx_http_modsecurity_config_cleanup does not know anything about this pool (since it is NULL).

@AirisX
Copy link
Contributor Author

AirisX commented Dec 25, 2017

It seems that this PR is also reasonable for owasp-modsecurity/ModSecurity#1546

@zimmerle zimmerle requested review from zimmerle and defanator March 26, 2018 19:37
@zimmerle zimmerle self-assigned this Mar 26, 2018
dd("deleting a loc conf -- RuleSet is: \"%p\"", t->rules_set);

old_pool = ngx_http_modsecurity_pcre_malloc_init(NULL);
old_pool = ngx_http_modsecurity_pcre_malloc_init(t->pool);
Copy link
Contributor

Choose a reason for hiding this comment

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

Regardless the parameter here I guess the end result will be the same.

Copy link
Contributor

@zimmerle zimmerle Mar 26, 2018

Choose a reason for hiding this comment

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

oh wait, i think i've get your point. let me discuss with @defanator.

Copy link
Collaborator

@defanator defanator left a comment

Choose a reason for hiding this comment

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

@AirisX @zimmerle sorry for the delay with this one - I'm finally fine with merging it as is (note that the patch does not apply cleanly on the current master, though the fix is easy).

Thanks @AirisX!

@AirisX AirisX force-pushed the fix/conf_cleanup_pool branch from 7efd4f0 to c6ea19d Compare April 28, 2018 10:18
@AirisX
Copy link
Contributor Author

AirisX commented Apr 28, 2018

@defanator @zimmerle I did a rebase to apply patch more cleanly.

zimmerle pushed a commit that referenced this pull request May 3, 2018
@zimmerle
Copy link
Contributor

zimmerle commented May 3, 2018

Merged! Thanks!

@zimmerle zimmerle closed this May 3, 2018
pracj3am pushed a commit to cdn77/ModSecurity-nginx that referenced this pull request Nov 4, 2022
pracj3am added a commit to cdn77/ModSecurity-nginx that referenced this pull request Oct 10, 2023
pracj3am added a commit to cdn77/ModSecurity-nginx that referenced this pull request Oct 10, 2023
pracj3am added a commit to cdn77/ModSecurity-nginx that referenced this pull request Oct 10, 2023
pracj3am added a commit to cdn77/ModSecurity-nginx that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants