---
Hi Willy,
This is a fix of segmentation fault error (20260113):
The root cause was identified as a call to var_set() in the head of
action_store() function which passed an unintialized smp structure. It led to
var_set() accessing invalid mem addresses, resulting in a crash. I checked that
this var_set() line is unnessary since there was confusion during the initial
stages of code modification, and this confusion was not subsequently resolved.
Thanks for checking and giving me a hint.
Regards,
Hyeonggeun.
---
Currently, variable names are only used during parsing and are not
stored at runtime. This makes it impossible to iterate through
variables and retrieve their names.
This patch adds infrastructure to store variable names:
- Add 'name' and 'name_len' fields to var_desc structure
- Add 'name' field to var structure
- Add VDF_NAME_ALLOCATED flag to track memory ownership
- Store names in vars_fill_desc(), var_set(), vars_check_arg(),
and parse_store()
- Free names in var_clear() and release_store_rule()
This prepares the ground for implementing dump_all_vars() in the
next commit.
---
include/haproxy/vars-t.h | 4 ++++
src/vars.c | 47 ++++++++++++++++++++++++++++++++++++++--
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/include/haproxy/vars-t.h b/include/haproxy/vars-t.h
index 105506977..24f629743 100644
--- a/include/haproxy/vars-t.h
+++ b/include/haproxy/vars-t.h
@@ -57,12 +57,15 @@ struct vars {
};
#define VDF_PARENT_CTX 0x00000001 // Set if the variable is related to
the parent stream
+#define VDF_NAME_ALLOCATED 0x00000002 // Set if name was allocated and
must be freed
/* This struct describes a variable as found in an arg_data */
struct var_desc {
uint64_t name_hash;
enum vars_scope scope;
uint flags; /*VDF_* */
+ const char *name; /* variable name (not owned) */
+ size_t name_len; /* variable name length */
};
struct var {
@@ -70,6 +73,7 @@ struct var {
uint64_t name_hash; /* XXH3() of the variable's name, indexed by
<name_node> */
uint flags; // VF_*
/* 32-bit hole here */
+ char *name; /* variable name (allocated) */
struct sample_data data; /* data storage. */
};
diff --git a/src/vars.c b/src/vars.c
index 1755bf8ad..3e46340cd 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -178,6 +178,8 @@ unsigned int var_clear(struct vars *vars, struct var *var,
int force)
{
unsigned int size = 0;
+ ha_free(&var->name);
+
if (var->data.type == SMP_T_STR || var->data.type == SMP_T_BIN) {
ha_free(&var->data.u.str.area);
size += var->data.u.str.data;
@@ -322,6 +324,8 @@ static int vars_fill_desc(const char *name, int len, struct
var_desc *desc, char
}
desc->name_hash = XXH3(name, len, var_name_hash_seed);
+ desc->name = name;
+ desc->name_len = len;
return 1;
}
@@ -431,6 +435,16 @@ int var_set(const struct var_desc *desc, struct sample
*smp, uint flags)
goto unlock;
var->name_hash = desc->name_hash;
var->flags = flags & VF_PERMANENT;
+
+ /* Save variable name */
+ var->name = NULL;
+ if (desc->name && desc->name_len > 0) {
+ var->name = my_strndup(desc->name, desc->name_len);
+ if (!var->name) {
+ pool_free(var_pool, var);
+ goto unlock;
+ }
+ }
var->data.type = SMP_T_ANY;
cebu64_item_insert(&vars->name_root[var->name_hash %
VAR_NAME_ROOTS], name_node, name_hash, var);
}
@@ -623,6 +637,7 @@ int vars_check_arg(struct arg *arg, char **err)
{
struct sample empty_smp = { };
struct var_desc desc;
+ char *saved_name = NULL;
/* Check arg type. */
if (arg->type != ARGT_STR) {
@@ -634,15 +649,27 @@ int vars_check_arg(struct arg *arg, char **err)
if (!vars_fill_desc(arg->data.str.area, arg->data.str.data, &desc, err))
return 0;
- if (desc.scope == SCOPE_PROC && !var_set(&desc, &empty_smp,
VF_CREATEONLY|VF_PERMANENT))
+ /* Save variable name before destroying the chunk */
+ if (desc.name && desc.name_len > 0) {
+ saved_name = my_strndup(desc.name, desc.name_len);
+ if (!saved_name) {
+ memprintf(err, "out of memory");
+ return 0;
+ }
+ }
+
+ if (desc.scope == SCOPE_PROC && !var_set(&desc, &empty_smp,
VF_CREATEONLY|VF_PERMANENT)) {
+ if (desc.flags & VDF_NAME_ALLOCATED)
+ ha_free(&saved_name);
return 0;
+ }
/* properly destroy the chunk */
chunk_destroy(&arg->data.str);
/* Use the global variable name pointer. */
arg->type = ARGT_VAR;
- arg->data.var = desc;
+ arg->data.var = desc; /* desc.name already points to saved_name */
return 1;
}
@@ -868,6 +895,10 @@ static void release_store_rule(struct act_rule *rule)
lf_expr_deinit(&rule->arg.vars.fmt);
release_sample_expr(rule->arg.vars.expr);
+
+ /* Free variable name if allocated */
+ if (rule->arg.vars.desc.flags & VDF_NAME_ALLOCATED)
+ ha_free((char **)&rule->arg.vars.desc.name);
}
/* This two function checks the variable name and replace the
@@ -919,6 +950,7 @@ static enum act_parse_ret parse_store(const char **args,
int *arg, struct proxy
struct ist condition = IST_NULL;
struct ist var = IST_NULL;
struct ist varname_ist = IST_NULL;
+ char *saved_name = NULL;
if (strncmp(var_name, "set-var-fmt", 11) == 0) {
var_name += 11;
@@ -973,6 +1005,17 @@ static enum act_parse_ret parse_store(const char **args,
int *arg, struct proxy
if (!vars_fill_desc(var_name, var_len, &rule->arg.vars.desc, err))
return ACT_RET_PRS_ERR;
+ /* Save variable name for runtime use */
+ if (rule->arg.vars.desc.name && rule->arg.vars.desc.name_len > 0) {
+ saved_name = my_strndup(rule->arg.vars.desc.name,
rule->arg.vars.desc.name_len);
+ if (!saved_name) {
+ memprintf(err, "out of memory");
+ return ACT_RET_PRS_ERR;
+ }
+ rule->arg.vars.desc.name = saved_name;
+ rule->arg.vars.desc.flags |= VDF_NAME_ALLOCATED;
+ }
+
if (rule->arg.vars.desc.scope == SCOPE_PROC &&
!var_set(&rule->arg.vars.desc, &empty_smp,
VF_CREATEONLY|VF_PERMANENT))
return 0;
--
2.48.1
