Re: [rtla 01/13] rtla: Check for memory allocation failures

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 17 Nov 2025 15:41:08 -0300
Wander Lairson Costa <wander@redhat.com> wrote:

> The actions_init() and actions_new() functions did not check the
> return value of calloc() and realloc() respectively. In a low
> memory situation, this could lead to a NULL pointer dereference.
> 
> Add checks for the return value of memory allocation functions
> and return an error in case of failure. Update the callers to
> handle the error properly.
> 
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
>  tools/tracing/rtla/src/actions.c       | 26 +++++++++++++++++++++++---
>  tools/tracing/rtla/src/actions.h       |  2 +-
>  tools/tracing/rtla/src/timerlat_hist.c |  7 +++++--
>  tools/tracing/rtla/src/timerlat_top.c  |  7 +++++--
>  4 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> index 8945aee58d511..01648a1425c10 100644
> --- a/tools/tracing/rtla/src/actions.c
> +++ b/tools/tracing/rtla/src/actions.c
> @@ -11,11 +11,13 @@
>  /*
>   * actions_init - initialize struct actions
>   */
> -void
> +int
>  actions_init(struct actions *self)
>  {
>  	self->size = action_default_size;
>  	self->list = calloc(self->size, sizeof(struct action));
> +	if (!self->list)
> +		return -1;

Can you return -ENOMEM?

>  	self->len = 0;
>  	self->continue_flag = false;
>  
> @@ -23,6 +25,7 @@ actions_init(struct actions *self)
>  
>  	/* This has to be set by the user */
>  	self->trace_output_inst = NULL;
> +	return 0;
>  }
>  
>  /*
> @@ -50,8 +53,13 @@ static struct action *
>  actions_new(struct actions *self)
>  {
>  	if (self->len >= self->size) {
> -		self->size *= 2;
> -		self->list = realloc(self->list, self->size * sizeof(struct action));
> +		const size_t new_size = self->size * 2;
> +		void *p = reallocarray(self->list, new_size, sizeof(struct action));
> +
> +		if (!p)
> +			return NULL;
> +		self->list = p;
> +		self->size = new_size;
>  	}
>  
>  	return &self->list[self->len++];
> @@ -65,6 +73,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;

I think !action should return -ENOMEM too.

> +
>  	self->present[ACTION_TRACE_OUTPUT] = true;
>  	action->type = ACTION_TRACE_OUTPUT;
>  	action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
> @@ -83,6 +94,9 @@ actions_add_signal(struct actions *self, int signal, int pid)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_SIGNAL] = true;
>  	action->type = ACTION_SIGNAL;
>  	action->signal = signal;
> @@ -99,6 +113,9 @@ actions_add_shell(struct actions *self, const char *command)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_SHELL] = true;
>  	action->type = ACTION_SHELL;
>  	action->command = calloc(strlen(command) + 1, sizeof(char));
> @@ -117,6 +134,9 @@ actions_add_continue(struct actions *self)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_CONTINUE] = true;
>  	action->type = ACTION_CONTINUE;
>  

The above same patterns too.

Thank you,



-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>