On Tue, 17 May 2022 17:57:23 +0800 Yuanjun Gong ruc_gongyuanjun@163.com wrote:
From: Gong Yuanjun ruc_gongyuanjun@163.com
In hist_trigger_elt_data_alloc(), elt_data is freed by hist_elt_data_free() if kcalloc fails.
static int hist_trigger_elt_data_alloc(struct tracing_map_elt *elt) { ... elt_data->field_var_str = kcalloc(n_str, sizeof(char *), GFP_KERNEL); if (!elt_data->field_var_str) {
But at this position elt_data->n_field_var_str is zero.
hist_elt_data_free(elt_data); return -EINVAL; }...}
In hist_elt_data_free() the elt_data->field_var_str field should be checked before dereference.
Signed-off-by: Gong Yuanjun ruc_gongyuanjun@163.com
kernel/trace/trace_events_hist.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 44db5ba9cabb..73177c9f94b2 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -1576,11 +1576,11 @@ static void hist_elt_data_free(struct hist_elt_data *elt_data) { unsigned int i;
- for (i = 0; i < elt_data->n_field_var_str; i++)
This loop will not execute because n_field_var_str is zero, and 0 < 0 is false.
kfree(elt_data->field_var_str[i]);- kfree(elt_data->field_var_str);
freeing a NULL pointer is OK to do.
There is nothing wrong with the current code. NACK.
-- Steve
- if (elt_data->field_var_str) {
for (i = 0; i < elt_data->n_field_var_str; i++)kfree(elt_data->field_var_str[i]);kfree(elt_data->field_var_str);- } kfree(elt_data->comm); kfree(elt_data);
}