MEDIUM: regex: modify regex_comp() to atomically allocate/free the my_regex struct

Now we atomically allocate the my_regex struct within function
regex_comp() and compile the regex or free both in case of failure. The
pointer to the allocated my_regex struct is returned directly. The
my_regex* argument to regex_comp() is removed.

Function regex_free() was modified so that it systematically frees the
my_regex entry. The function does nothing when called with a NULL as
argument (like free()). It will avoid existing risk of not properly
freeing the initialized area.

Other structures are also updated in order to be compatible (the ones
related to Lua and action rules).
This commit is contained in:
Dragan Dosen 2019-04-30 15:54:36 +02:00 committed by Willy Tarreau
parent 4d7894870a
commit 2674303912
14 changed files with 103 additions and 123 deletions

View File

@ -88,7 +88,7 @@ extern THREAD_LOCAL regmatch_t pmatch[MAX_MATCH];
*
* The function return 1 is succes case, else return 0 and err is filled.
*/
int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **err);
struct my_regex *regex_comp(const char *str, int cs, int cap, char **err);
int exp_replace(char *dst, unsigned int dst_size, char *src, const char *str, const regmatch_t *matches);
const char *check_replace_string(const char *str);
const char *chain_regex(struct hdr_exp **head, struct my_regex *preg,
@ -161,6 +161,8 @@ int regex_exec_match2(const struct my_regex *preg, char *subject, int length,
size_t nmatch, regmatch_t pmatch[], int flags);
static inline void regex_free(struct my_regex *preg) {
if (!preg)
return;
#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
pcre_free(preg->reg);
/* PCRE < 8.20 requires pcre_free() while >= 8.20 requires pcre_study_free(),
@ -176,6 +178,7 @@ static inline void regex_free(struct my_regex *preg) {
#else
regfree(&preg->regex);
#endif
free(preg);
}
#endif /* _COMMON_REGEX_H */

View File

@ -121,7 +121,7 @@ struct act_rule {
char *name; /* header name */
int name_len; /* header name's length */
struct list fmt; /* log-format compatible expression */
struct my_regex re; /* used by replace-header and replace-value */
struct my_regex *re; /* used by replace-header and replace-value */
} hdr_add; /* args used by "add-header" and "set-header" */
struct {
char *name; /* header name */

View File

@ -6,6 +6,7 @@
#include <lua.h>
#include <lauxlib.h>
#include <common/regex.h>
#include <common/xref.h>
#include <types/proto_http.h>

View File

@ -275,17 +275,10 @@ static int create_cond_regex_rule(const char *file, int line,
((px->cap & PR_CAP_BE) ? SMP_VAL_BE_HRS_HDR : SMP_VAL_FE_HRS_HDR),
file, line);
preg = calloc(1, sizeof(*preg));
if (!preg) {
ha_alert("parsing [%s:%d] : '%s' : not enough memory to build regex.\n", file, line, cmd);
ret_code = ERR_ALERT | ERR_FATAL;
goto err;
}
cs = !(flags & REG_ICASE);
cap = !(flags & REG_NOSUB);
error = NULL;
if (!regex_comp(reg, preg, cs, cap, &error)) {
if (!(preg = regex_comp(reg, cs, cap, &error))) {
ha_alert("parsing [%s:%d] : '%s' : regular expression '%s' : %s\n", file, line, cmd, reg, error);
free(error);
ret_code = ERR_ALERT | ERR_FATAL;
@ -309,7 +302,6 @@ static int create_cond_regex_rule(const char *file, int line,
err_free:
regex_free(preg);
err:
free(preg);
free(errmsg);
return ret_code;
}
@ -444,8 +436,14 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
curproxy->expect_str = strdup(defproxy.expect_str);
if (defproxy.expect_regex) {
/* note: this regex is known to be valid */
curproxy->expect_regex = calloc(1, sizeof(*curproxy->expect_regex));
regex_comp(defproxy.expect_str, curproxy->expect_regex, 1, 1, NULL);
error = NULL;
if (!(curproxy->expect_regex = regex_comp(defproxy.expect_str, 1, 1, &error))) {
ha_alert("parsing [%s:%d] : regular expression '%s' : %s\n", file, linenum,
defproxy.expect_str, error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
}
}
}
@ -631,11 +629,8 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
free(defproxy.server_id_hdr_name);
defproxy.server_id_hdr_len = 0;
free(defproxy.expect_str);
if (defproxy.expect_regex) {
regex_free(defproxy.expect_regex);
free(defproxy.expect_regex);
defproxy.expect_regex = NULL;
}
regex_free(defproxy.expect_regex);
defproxy.expect_regex = NULL;
if (defproxy.conf.logformat_string != default_http_log_format &&
defproxy.conf.logformat_string != default_tcp_log_format &&
@ -2927,16 +2922,11 @@ stats_error_parsing:
}
curproxy->options2 |= PR_O2_EXP_RSTS;
free(curproxy->expect_str);
if (curproxy->expect_regex) {
regex_free(curproxy->expect_regex);
free(curproxy->expect_regex);
curproxy->expect_regex = NULL;
}
regex_free(curproxy->expect_regex);
curproxy->expect_str = strdup(args[cur_arg + 1]);
curproxy->expect_regex = calloc(1, sizeof(*curproxy->expect_regex));
error = NULL;
if (!regex_comp(args[cur_arg + 1], curproxy->expect_regex, 1, 1, &error)) {
ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
if (!(curproxy->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
@ -2952,16 +2942,11 @@ stats_error_parsing:
}
curproxy->options2 |= PR_O2_EXP_RSTR;
free(curproxy->expect_str);
if (curproxy->expect_regex) {
regex_free(curproxy->expect_regex);
free(curproxy->expect_regex);
curproxy->expect_regex = NULL;
}
regex_free(curproxy->expect_regex);
curproxy->expect_str = strdup(args[cur_arg + 1]);
curproxy->expect_regex = calloc(1, sizeof(*curproxy->expect_regex));
error = NULL;
if (!regex_comp(args[cur_arg + 1], curproxy->expect_regex, 1, 1, &error)) {
ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
if (!(curproxy->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;
@ -3258,10 +3243,9 @@ stats_error_parsing:
tcpcheck->action = TCPCHK_ACT_EXPECT;
tcpcheck->string_len = 0;
tcpcheck->string = NULL;
tcpcheck->expect_regex = calloc(1, sizeof(*tcpcheck->expect_regex));
error = NULL;
if (!regex_comp(args[cur_arg + 1], tcpcheck->expect_regex, 1, 1, &error)) {
ha_alert("parsing [%s:%d] : '%s %s %s' : bad regular expression '%s': %s.\n",
if (!(tcpcheck->expect_regex = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s %s %s' : regular expression '%s': %s.\n",
file, linenum, args[0], args[1], ptr_arg, args[cur_arg + 1], error);
free(error);
err_code |= ERR_ALERT | ERR_FATAL;

View File

@ -3193,8 +3193,7 @@ void email_alert_free(struct email_alert *alert)
LIST_DEL(&rule->list);
free(rule->comment);
free(rule->string);
if (rule->expect_regex)
regex_free(rule->expect_regex);
regex_free(rule->expect_regex);
pool_free(pool_head_tcpcheck_rule, rule);
}
pool_free(pool_head_email_alert, alert);

View File

@ -2168,11 +2168,7 @@ void deinit(void)
}
for (exp = p->req_exp; exp != NULL; ) {
if (exp->preg) {
regex_free(exp->preg);
free(exp->preg);
}
regex_free(exp->preg);
free((char *)exp->replace);
expb = exp;
exp = exp->next;
@ -2180,11 +2176,7 @@ void deinit(void)
}
for (exp = p->rsp_exp; exp != NULL; ) {
if (exp->preg) {
regex_free(exp->preg);
free(exp->preg);
}
regex_free(exp->preg);
free((char *)exp->replace);
expb = exp;
exp = exp->next;

View File

@ -5366,13 +5366,13 @@ __LJMP static inline int hlua_http_rep_hdr(lua_State *L, struct hlua_txn *htxn,
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len));
const char *reg = MAY_LJMP(luaL_checkstring(L, 3));
const char *value = MAY_LJMP(luaL_checkstring(L, 4));
struct my_regex re;
struct my_regex *re;
if (!regex_comp(reg, &re, 1, 1, NULL))
if (!(re = regex_comp(reg, 1, 1, NULL)))
WILL_LJMP(luaL_argerror(L, 3, "invalid regex"));
http_transform_header_str(htxn->s, msg, name, name_len, value, &re, action);
regex_free(&re);
http_transform_header_str(htxn->s, msg, name, name_len, value, re, action);
regex_free(re);
return 0;
}

View File

@ -1537,14 +1537,14 @@ int hlua_match_addr(lua_State *L)
return 1;
}
static struct my_regex *hlua_check_regex(lua_State *L, int ud)
static struct my_regex **hlua_check_regex(lua_State *L, int ud)
{
return (hlua_checkudata(L, ud, class_regex_ref));
}
static int hlua_regex_comp(struct lua_State *L)
{
struct my_regex *regex;
struct my_regex **regex;
const char *str;
int cs;
char *err;
@ -1556,7 +1556,7 @@ static int hlua_regex_comp(struct lua_State *L)
regex = lua_newuserdata(L, sizeof(*regex));
err = NULL;
if (!regex_comp(str, regex, cs, 1, &err)) {
if (!(*regex = regex_comp(str, cs, 1, &err))) {
lua_pushboolean(L, 0); /* status error */
lua_pushstring(L, err); /* Reason */
free(err);
@ -1576,7 +1576,7 @@ static int hlua_regex_comp(struct lua_State *L)
static int hlua_regex_exec(struct lua_State *L)
{
struct my_regex *regex;
struct my_regex **regex;
const char *str;
size_t len;
struct buffer *tmp;
@ -1584,6 +1584,11 @@ static int hlua_regex_exec(struct lua_State *L)
regex = hlua_check_regex(L, 1);
str = luaL_checklstring(L, 2, &len);
if (!*regex) {
lua_pushboolean(L, 0);
return 1;
}
/* Copy the string because regex_exec2 require a 'char *'
* and not a 'const char *'.
*/
@ -1594,14 +1599,14 @@ static int hlua_regex_exec(struct lua_State *L)
}
memcpy(tmp->area, str, len);
lua_pushboolean(L, regex_exec2(regex, tmp->area, len));
lua_pushboolean(L, regex_exec2(*regex, tmp->area, len));
return 1;
}
static int hlua_regex_match(struct lua_State *L)
{
struct my_regex *regex;
struct my_regex **regex;
const char *str;
size_t len;
regmatch_t pmatch[20];
@ -1612,6 +1617,11 @@ static int hlua_regex_match(struct lua_State *L)
regex = hlua_check_regex(L, 1);
str = luaL_checklstring(L, 2, &len);
if (!*regex) {
lua_pushboolean(L, 0);
return 1;
}
/* Copy the string because regex_exec2 require a 'char *'
* and not a 'const char *'.
*/
@ -1622,7 +1632,7 @@ static int hlua_regex_match(struct lua_State *L)
}
memcpy(tmp->area, str, len);
ret = regex_exec_match2(regex, tmp->area, len, 20, pmatch, 0);
ret = regex_exec_match2(*regex, tmp->area, len, 20, pmatch, 0);
lua_pushboolean(L, ret);
lua_newtable(L);
if (ret) {
@ -1636,10 +1646,11 @@ static int hlua_regex_match(struct lua_State *L)
static int hlua_regex_free(struct lua_State *L)
{
struct my_regex *regex;
struct my_regex **regex;
regex = hlua_check_regex(L, 1);
regex_free(regex);
regex_free(*regex);
*regex = NULL;
return 0;
}

View File

@ -263,7 +263,7 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
LIST_INIT(&rule->arg.hdr_add.fmt);
error = NULL;
if (!regex_comp(args[cur_arg + 1], &rule->arg.hdr_add.re, 1, 1, &error)) {
if (!(rule->arg.hdr_add.re = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s' : %s.\n", file, linenum,
args[cur_arg + 1], error);
free(error);
@ -713,7 +713,7 @@ struct act_rule *parse_http_res_cond(const char **args, const char *file, int li
LIST_INIT(&rule->arg.hdr_add.fmt);
error = NULL;
if (!regex_comp(args[cur_arg + 1], &rule->arg.hdr_add.re, 1, 1, &error)) {
if (!(rule->arg.hdr_add.re = regex_comp(args[cur_arg + 1], 1, 1, &error))) {
ha_alert("parsing [%s:%d] : '%s' : %s.\n", file, linenum,
args[cur_arg + 1], error);
free(error);
@ -1191,7 +1191,7 @@ void free_http_res_rules(struct list *r)
list_for_each_entry_safe(pr, tr, r, list) {
LIST_DEL(&pr->list);
regex_free(&pr->arg.hdr_add.re);
regex_free(pr->arg.hdr_add.re);
free(pr);
}
}
@ -1205,7 +1205,7 @@ void free_http_req_rules(struct list *r)
if (pr->action == ACT_HTTP_REQ_AUTH)
free(pr->arg.auth.realm);
regex_free(&pr->arg.hdr_add.re);
regex_free(pr->arg.hdr_add.re);
free(pr);
}
}

View File

@ -1142,7 +1142,6 @@ void pat_prune_reg(struct pattern_expr *expr)
list_for_each_entry_safe(pat, tmp, &expr->patterns, list) {
regex_free(pat->pat.ptr.ptr);
free(pat->pat.ptr.ptr);
free(pat->pat.data);
free(pat);
}
@ -1253,18 +1252,9 @@ int pat_idx_list_reg_cap(struct pattern_expr *expr, struct pattern *pat, int cap
/* duplicate pattern */
memcpy(&patl->pat, pat, sizeof(*pat));
/* allocate regex */
patl->pat.ptr.reg = calloc(1, sizeof(*patl->pat.ptr.reg));
if (!patl->pat.ptr.reg) {
free(patl);
memprintf(err, "out of memory while indexing pattern");
return 0;
}
/* compile regex */
if (!regex_comp(pat->ptr.str, patl->pat.ptr.reg,
!(expr->mflags & PAT_MF_IGNORE_CASE), cap, err)) {
free(patl->pat.ptr.reg);
if (!(patl->pat.ptr.reg = regex_comp(pat->ptr.str, !(expr->mflags & PAT_MF_IGNORE_CASE),
cap, err))) {
free(patl);
return 0;
}
@ -1562,7 +1552,6 @@ void pat_del_list_reg(struct pattern_expr *expr, struct pat_ref_elt *ref)
/* Delete and free entry. */
LIST_DEL(&pat->list);
regex_free(pat->pat.ptr.ptr);
free(pat->pat.ptr.ptr);
free(pat->pat.data);
free(pat);
}

View File

@ -1581,7 +1581,7 @@ resume_execution:
if (http_transform_header(s, &txn->req, rule->arg.hdr_add.name,
rule->arg.hdr_add.name_len,
&rule->arg.hdr_add.fmt,
&rule->arg.hdr_add.re, rule->action)) {
rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
@ -1939,7 +1939,7 @@ resume_execution:
if (http_transform_header(s, &txn->rsp, rule->arg.hdr_add.name,
rule->arg.hdr_add.name_len,
&rule->arg.hdr_add.fmt,
&rule->arg.hdr_add.re, rule->action)) {
rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}

View File

@ -2901,7 +2901,7 @@ static enum rule_result htx_req_get_intercept_rule(struct proxy *px, struct list
if (htx_transform_header(s, &s->req, htx,
ist2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len),
&rule->arg.hdr_add.fmt,
&rule->arg.hdr_add.re, rule->action)) {
rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}
@ -3243,7 +3243,7 @@ resume_execution:
if (htx_transform_header(s, &s->res, htx,
ist2(rule->arg.hdr_add.name, rule->arg.hdr_add.name_len),
&rule->arg.hdr_add.fmt,
&rule->arg.hdr_add.re, rule->action)) {
rule->arg.hdr_add.re, rule->action)) {
rule_ret = HTTP_RULE_RES_BADREQ;
goto end;
}

View File

@ -337,30 +337,13 @@ int regex_exec_match2(const struct my_regex *preg, char *subject, int length,
#endif
}
int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **err)
struct my_regex *regex_comp(const char *str, int cs, int cap, char **err)
{
struct my_regex *regex = NULL;
#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
int flags = 0;
const char *error;
int erroffset;
if (!cs)
flags |= PCRE_CASELESS;
if (!cap)
flags |= PCRE_NO_AUTO_CAPTURE;
regex->reg = pcre_compile(str, flags, &error, &erroffset, NULL);
if (!regex->reg) {
memprintf(err, "regex '%s' is invalid (error=%s, erroffset=%d)", str, error, erroffset);
return 0;
}
regex->extra = pcre_study(regex->reg, PCRE_STUDY_JIT_COMPILE, &error);
if (!regex->extra && error != NULL) {
pcre_free(regex->reg);
memprintf(err, "failed to compile regex '%s' (error=%s)", str, error);
return 0;
}
#elif defined(USE_PCRE2) || defined(USE_PCRE2_JIT)
int flags = 0;
int errn;
@ -369,7 +352,35 @@ int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **
#endif
PCRE2_UCHAR error[256];
PCRE2_SIZE erroffset;
#else
int flags = REG_EXTENDED;
#endif
regex = calloc(1, sizeof(*regex));
if (!regex) {
memprintf(err, "not enough memory to build regex");
goto out_fail_alloc;
}
#if defined(USE_PCRE) || defined(USE_PCRE_JIT)
if (!cs)
flags |= PCRE_CASELESS;
if (!cap)
flags |= PCRE_NO_AUTO_CAPTURE;
regex->reg = pcre_compile(str, flags, &error, &erroffset, NULL);
if (!regex->reg) {
memprintf(err, "regex '%s' is invalid (error=%s, erroffset=%d)", str, error, erroffset);
goto out_fail_alloc;
}
regex->extra = pcre_study(regex->reg, PCRE_STUDY_JIT_COMPILE, &error);
if (!regex->extra && error != NULL) {
pcre_free(regex->reg);
memprintf(err, "failed to compile regex '%s' (error=%s)", str, error);
goto out_fail_alloc;
}
#elif defined(USE_PCRE2) || defined(USE_PCRE2_JIT)
if (!cs)
flags |= PCRE2_CASELESS;
if (!cap)
@ -379,7 +390,7 @@ int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **
if (!regex->reg) {
pcre2_get_error_message(errn, error, sizeof(error));
memprintf(err, "regex '%s' is invalid (error=%s, erroffset=%zu)", str, error, erroffset);
return 0;
goto out_fail_alloc;
}
#if defined(USE_PCRE2_JIT)
@ -391,13 +402,11 @@ int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **
if (jit < 0 && jit != PCRE2_ERROR_JIT_BADOPTION) {
pcre2_code_free(regex->reg);
memprintf(err, "regex '%s' jit compilation failed", str);
return 0;
goto out_fail_alloc;
}
#endif
#else
int flags = REG_EXTENDED;
if (!cs)
flags |= REG_ICASE;
if (!cap)
@ -405,10 +414,14 @@ int regex_comp(const char *str, struct my_regex *regex, int cs, int cap, char **
if (regcomp(&regex->regex, str, flags) != 0) {
memprintf(err, "regex '%s' is invalid", str);
return 0;
goto out_fail_alloc;
}
#endif
return 1;
return regex;
out_fail_alloc:
free(regex);
return NULL;
}
static void regex_register_build_options(void)

View File

@ -1328,20 +1328,11 @@ int smp_resolve_args(struct proxy *p)
continue;
}
reg = calloc(1, sizeof(*reg));
if (!reg) {
ha_alert("parsing [%s:%d] : not enough memory to build regex in arg %d of %s%s%s%s '%s' %s proxy '%s'.\n",
cur->file, cur->line,
cur->arg_pos + 1, conv_pre, conv_ctx, conv_pos, ctx, cur->kw, where, p->id);
cfgerr++;
continue;
}
rflags = 0;
rflags |= (arg->type_flags & ARGF_REG_ICASE) ? REG_ICASE : 0;
err = NULL;
if (!regex_comp(arg->data.str.area, reg, !(rflags & REG_ICASE), 1 /* capture substr */, &err)) {
if (!(reg = regex_comp(arg->data.str.area, !(rflags & REG_ICASE), 1 /* capture substr */, &err))) {
ha_alert("parsing [%s:%d] : error in regex '%s' in arg %d of %s%s%s%s '%s' %s proxy '%s' : %s.\n",
cur->file, cur->line,
arg->data.str.area,
@ -1425,11 +1416,8 @@ static void release_sample_arg(struct arg *p)
p->unresolved = 0;
}
else if (p->type == ARGT_REG) {
if (p->data.reg) {
regex_free(p->data.reg);
free(p->data.reg);
p->data.reg = NULL;
}
regex_free(p->data.reg);
p->data.reg = NULL;
}
p++;
}