BUG/MINOR: cfgcond: revisit the condition freeing mechanism to avoid a leak
The cfg_free_cond_{term,and,expr}() functions used to take a pointer to
the pointer to be freed in order to replace it with a NULL once done.
But this doesn't cope well with freeing lists as it would require
recursion which the current code tried to avoid.
Let's just change the API to free the area and let the caller set the NULL.
This leak was reported by oss-fuzz (issue 36265).
diff --git a/include/haproxy/cfgcond.h b/include/haproxy/cfgcond.h
index 6af88a2..a1f86e6 100644
--- a/include/haproxy/cfgcond.h
+++ b/include/haproxy/cfgcond.h
@@ -28,15 +28,15 @@
const struct cond_pred_kw *cfg_lookup_cond_pred(const char *str);
int cfg_parse_cond_term(const char **text, struct cfg_cond_term **term, char **err, const char **errptr);
int cfg_eval_cond_term(const struct cfg_cond_term *term, char **err);
-void cfg_free_cond_term(struct cfg_cond_term **term);
+void cfg_free_cond_term(struct cfg_cond_term *term);
int cfg_parse_cond_and(const char **text, struct cfg_cond_and **expr, char **err, const char **errptr);
int cfg_eval_cond_and(struct cfg_cond_and *expr, char **err);
-void cfg_free_cond_and(struct cfg_cond_and **expr);
+void cfg_free_cond_and(struct cfg_cond_and *expr);
int cfg_parse_cond_expr(const char **text, struct cfg_cond_expr **expr, char **err, const char **errptr);
int cfg_eval_cond_expr(struct cfg_cond_expr *expr, char **err);
-void cfg_free_cond_expr(struct cfg_cond_expr **expr);
+void cfg_free_cond_expr(struct cfg_cond_expr *expr);
int cfg_eval_condition(char **args, char **err, const char **errptr);
diff --git a/src/cfgcond.c b/src/cfgcond.c
index fd03196..c1259c0 100644
--- a/src/cfgcond.c
+++ b/src/cfgcond.c
@@ -46,17 +46,19 @@
}
/* Frees <term> and its args. NULL is supported and does nothing. */
-void cfg_free_cond_term(struct cfg_cond_term **term)
+void cfg_free_cond_term(struct cfg_cond_term *term)
{
- if (!term || !*term)
+ if (!term)
return;
- if ((*term)->type == CCTT_PAREN)
- cfg_free_cond_expr(&(*term)->expr);
+ if (term->type == CCTT_PAREN) {
+ cfg_free_cond_expr(term->expr);
+ term->expr = NULL;
+ }
- free_args((*term)->args);
- free((*term)->args);
- ha_free(term);
+ free_args(term->args);
+ free(term->args);
+ free(term);
}
/* Parse an indirect input text as a possible config condition term.
@@ -158,7 +160,8 @@
if (errptr)
*errptr = *text;
fail2:
- cfg_free_cond_term(term);
+ cfg_free_cond_term(*term);
+ *term = NULL;
return -1;
}
@@ -240,20 +243,28 @@
/* Frees <expr> and its terms and args. NULL is supported and does nothing. */
-void cfg_free_cond_and(struct cfg_cond_and **expr)
+void cfg_free_cond_and(struct cfg_cond_and *expr)
{
- while (expr && *expr) {
- cfg_free_cond_term(&(*expr)->left);
- expr = &(*expr)->right;
+ struct cfg_cond_and *prev;
+
+ while (expr) {
+ cfg_free_cond_term(expr->left);
+ prev = expr;
+ expr = expr->right;
+ free(prev);
}
}
/* Frees <expr> and its terms and args. NULL is supported and does nothing. */
-void cfg_free_cond_expr(struct cfg_cond_expr **expr)
+void cfg_free_cond_expr(struct cfg_cond_expr *expr)
{
- while (expr && *expr) {
- cfg_free_cond_and(&(*expr)->left);
- expr = &(*expr)->right;
+ struct cfg_cond_expr *prev;
+
+ while (expr) {
+ cfg_free_cond_and(expr->left);
+ prev = expr;
+ expr = expr->right;
+ free(prev);
}
}
@@ -313,8 +324,10 @@
if (ret > 0)
*text = in;
done:
- if (ret < 0)
- cfg_free_cond_and(expr);
+ if (ret < 0) {
+ cfg_free_cond_and(*expr);
+ *expr = NULL;
+ }
return ret;
}
@@ -374,8 +387,10 @@
if (ret > 0)
*text = in;
done:
- if (ret < 0)
- cfg_free_cond_expr(expr);
+ if (ret < 0) {
+ cfg_free_cond_expr(*expr);
+ *expr = NULL;
+ }
return ret;
}
@@ -450,6 +465,6 @@
if (errptr)
*errptr = text;
done:
- cfg_free_cond_expr(&expr);
+ cfg_free_cond_expr(expr);
return ret;
}