Open rkrishn7 opened 3 weeks ago
Hi @rkrishn7,
could you share your (minimal) config (at least to emulate this behavior)? And I assume you have your own application, so a minimal code example also would be fine.
Hey @airween,
Sure, I was able to create a minimal reproducible example in C (using libmodsecurity v3.0.12):
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <modsecurity/modsecurity.h>
#include <modsecurity/rules_set.h>
void *create_rules(void *arg)
{
const char *plain_rules = arg;
const char *err = NULL;
RulesSet *rules;
rules = msc_create_rules_set();
if (rules == NULL)
{
return NULL;
}
int retval = msc_rules_add(rules, plain_rules, &err);
if (retval < 0)
{
fprintf(stderr, "Error - msc_rules_add(): %s\n", err);
exit(EXIT_FAILURE);
}
return NULL;
}
int main()
{
pthread_t thread1, thread2;
int iret1, iret2;
const char *plain_rules_1 = "SecRuleEngine On\n";
const char *plain_rules_2 = "SecRuleEngine On\n";
iret1 = pthread_create(&thread1, NULL, create_rules, (void *)plain_rules_2);
if (iret1)
{
fprintf(stderr, "Error - pthread_create() return code: %d\n", iret1);
exit(EXIT_FAILURE);
}
iret2 = pthread_create(&thread2, NULL, create_rules, (void *)plain_rules_1);
if (iret2)
{
fprintf(stderr, "Error - pthread_create() return code: %d\n", iret2);
exit(EXIT_FAILURE);
}
// Wait till threads are complete before main continues.
pthread_join(thread1, NULL);
pthread_join(thread2, NULL);
return 0;
}
which yields the following output (of course the boundary errors are sporadic):
Error - msc_rules_add(): Rules error. File: <<reference missing or not informed>>. Line: 1. Column: 17. Invalid input:
fish: Job 1, './test' terminated by signal SIGSEGV (Address boundary error)
Hi @rkrishn7,
thanks for the example.
Perhaps you already know that libmodsecurity3 uses Bison as configuration parser (lexer and parser are in the source tree).
Unfortunately Bison generates a non-reetrant parser, therefore you can't use that in multi-threading environment in this state. As you can see there is a (theoretical?) solution, but parser is one of the most sensitive part of the library, so I wouldn't touch it.
But you can try to use mutex locks - eg. this worked for me (based on your given example above):
pthread_mutex_t lock;
void *create_rules(void *arg)
{
const char *plain_rules = arg;
const char *err = NULL;
RulesSet *rules;
rules = msc_create_rules_set();
if (rules == NULL)
{
return NULL;
}
pthread_mutex_lock(&lock);
int retval = msc_rules_add(rules, plain_rules, &err);
pthread_mutex_unlock(&lock);
if (retval < 0)
{
fprintf(stderr, "Error - msc_rules_add(): %s\n", err);
exit(EXIT_FAILURE);
}
else {
fprintf(stdout, "Done - msc_rules_add(): %s\n", err);
}
return NULL;
}
Ah thank you @airween for the context. Yes, I thought it was related to parsing per the backtrace.
In regards to serializing parsing via a Mutex - definitely agreed. I landed on this approach for my rust crate that provides an interface to libmodsecurity. See: https://github.com/rkrishn7/rust-modsecurity/blob/55cb5e65df0c7771f74524f7e4144046cb0d323b/src/rules.rs#L20
However, it is probably worth mentioning the thread-safety of certain APIs (e.g. ModSecurity
and RulesSet
) in the documentation somewhere.
Describe the bug
Hello!
I'm encountering an error when creating multiple rule sets and adding rules to each of them in a multi-threaded environment.
Logs and dumps
I've traced it back to
yylex
inyy::seclang_parser::parse()
:Is this a known issue? Is
Rules
meant to be initialized and filled only once and in a single-threaded context?