mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@barebox.org>
To: barebox@lists.infradead.org
Cc: Ahmad Fatoum <a.fatoum@barebox.org>
Subject: [PATCH RFC 3/3] hush: fix memory leaks
Date: Mon, 27 Oct 2025 08:54:34 +0100	[thread overview]
Message-ID: <20251027075438.2480311-3-a.fatoum@barebox.org> (raw)
In-Reply-To: <20251027075438.2480311-1-a.fatoum@barebox.org>

The memory leaks in hush have been bothered me for years and every time
I look into it, I give up after a while, because the code is convoluted.

Talloc shines here as we can just allocate a "zero-size" talloc object
as parent for a context and associate all allocations from that context
with it.

Then when that context is free'd all children will be freed as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 common/hush.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/common/hush.c b/common/hush.c
index ec3c0cd91320..2e0cc4229d35 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -97,6 +97,7 @@
 
 #define pr_fmt(fmt) "hush: " fmt
 
+#include <talloc.h>
 #include <malloc.h>         /* malloc, free, realloc*/
 #include <xfuncs.h>
 #include <linux/ctype.h>    /* isalpha, isdigit */
@@ -180,6 +181,7 @@ struct option {
 
 /* This holds pointers to the various results of parsing */
 struct p_context {
+	const void *scope;
 	struct child_prog *child;
 	struct pipe *list_head;
 	struct pipe *pipe;
@@ -308,7 +310,7 @@ static int run_pipe_real(struct p_context *ctx, struct pipe *pi);
 /*   variable assignment: */
 static int is_assignment(const char *s);
 /*   data structure manipulation: */
-static void initialize_context(struct p_context *ctx);
+static void initialize_context(struct p_context *ctx, bool newscope);
 static void release_context(struct p_context *ctx);
 static int done_word(o_string *dest, struct p_context *ctx);
 static int done_command(struct p_context *ctx);
@@ -435,7 +437,7 @@ static char *getprompt(void)
 	if (prompt_command) {
 		unsigned int lr = last_return_code;
 
-		initialize_context(&ctx);
+		initialize_context(&ctx, false);
 		parse_string_outer(&ctx, prompt_command, FLAG_PARSE_SEMICOLON);
 		release_context(&ctx);
 
@@ -837,7 +839,7 @@ static int run_pipe_real(struct p_context *ctx, struct pipe *pi)
 		char * str = NULL;
 		struct p_context ctx1;
 
-		initialize_context(&ctx1);
+		initialize_context(&ctx1, true);
 
 		str = make_string((child->argv + i));
 		rcode = parse_string_outer(&ctx1, str, FLAG_EXIT_FROM_LOOP | FLAG_REPARSING);
@@ -1062,7 +1064,8 @@ static int free_pipe(struct pipe *pi, int indent)
 		}
 	}
 
-	free(pi->progs);   /* children are an array, they get freed all at once */
+	/* children are an array, they get freed all at once */
+	talloc_free(pi->progs);
 	pi->progs = NULL;
 
 	return ret_code;
@@ -1079,7 +1082,7 @@ static int free_pipe_list(struct pipe *head, int indent)
 		final_printf("%s pipe followup code %d\n", indenter(indent), pi->followup);
 		next = pi->next;
 		pi->next = NULL;
-		free(pi);
+		talloc_free(pi);
 	}
 	return rcode;
 }
@@ -1182,16 +1185,18 @@ static int is_assignment(const char *s)
 }
 
 
-static struct pipe *new_pipe(void)
+static struct pipe *new_pipe(struct p_context *ctx)
 {
-	return xzalloc(sizeof(struct pipe));
+	return xtalloc_zero_size(ctx->scope, sizeof(struct pipe));
 }
 
-static void initialize_context(struct p_context *ctx)
+static void initialize_context(struct p_context *ctx, bool newscope)
 {
+	if (newscope)
+		ctx->scope = talloc_new(NULL);
 	ctx->pipe = NULL;
 	ctx->child = NULL;
-	ctx->list_head = new_pipe();
+	ctx->list_head = new_pipe(ctx);
 	ctx->pipe = ctx->list_head;
 	ctx->w = RES_NONE;
 	ctx->stack = NULL;
@@ -1211,6 +1216,7 @@ static void release_context(struct p_context *ctx)
 		free(opt);
 	}
 #endif
+	talloc_free((void *)ctx->scope);
 }
 
 /* normal return is 0
@@ -1269,7 +1275,7 @@ static int reserved_word(o_string *dest, struct p_context *ctx)
 				return 1;
 			}
 			*new = *ctx;   /* physical copy */
-			initialize_context(ctx);
+			initialize_context(ctx, false);
 			ctx->stack = new;
 		} else if (ctx->w == RES_NONE || !(ctx->old_flag & (1 << r->code))) {
 			syntax_unexpected_token(r->literal);
@@ -1366,7 +1372,10 @@ static int done_command(struct p_context *ctx)
 	} else {
 		hush_debug("%s: initializing\n", __func__);
 	}
-	pi->progs = xrealloc(pi->progs, sizeof(*pi->progs) * (pi->num_progs + 1));
+	pi->progs = talloc_realloc_size(ctx->scope, pi->progs,
+					sizeof(*pi->progs) * (pi->num_progs + 1));
+	if (!pi->progs)
+		panic("enomem");
 
 	prog = pi->progs + pi->num_progs;
 	prog->glob_result.gl_pathv = NULL;
@@ -1392,7 +1401,7 @@ static int done_pipe(struct p_context *ctx, pipe_style type)
 	ctx->pipe->followup = type;
 	ctx->pipe->r_mode = ctx->w;
 
-	new_p = new_pipe();
+	new_p = new_pipe(ctx);
 
 	ctx->pipe->next = new_p;
 	ctx->pipe = new_p;
@@ -1705,7 +1714,7 @@ char *shell_expand(char *str)
 
 	o.quote = 1;
 
-	initialize_context(&ctx);
+	initialize_context(&ctx, false);
 
 	parse_string(&o, &ctx, str);
 
@@ -1732,7 +1741,7 @@ static int parse_stream_outer(struct p_context *ctx, struct in_str *inp, int fla
 
 	do {
 		ctx->type = flag;
-		initialize_context(ctx);
+		initialize_context(ctx, false);
 		update_ifs_map();
 
 		if (!(flag & FLAG_PARSE_SEMICOLON) || (flag & FLAG_REPARSING))
@@ -1921,7 +1930,7 @@ int run_command(const char *cmd)
 	if (!IS_ALLOWED(SCONFIG_SHELL))
 		return -EPERM;
 
-	initialize_context(&ctx);
+	initialize_context(&ctx, true);
 
 	ret = parse_string_outer(&ctx, cmd, FLAG_PARSE_SEMICOLON);
 	release_context(&ctx);
@@ -1949,7 +1958,7 @@ static int source_script(const char *path, int argc, char *argv[])
 	char *script;
 	int ret;
 
-	initialize_context(&ctx);
+	initialize_context(&ctx, true);
 
 	ctx.global_argc = argc;
 	ctx.global_argv = argv;
-- 
2.47.3




  parent reply	other threads:[~2025-10-27  7:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27  7:54 [PATCH RFC 1/3] lib: add talloc for overlaying a tree onto allocations Ahmad Fatoum
2025-10-27  7:54 ` [PATCH RFC 2/3] test: self: add talloc selftest Ahmad Fatoum
2025-10-27  7:54 ` Ahmad Fatoum [this message]
2025-10-28  9:42 ` [PATCH RFC 1/3] lib: add talloc for overlaying a tree onto allocations Sascha Hauer
2025-10-28 10:26   ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251027075438.2480311-3-a.fatoum@barebox.org \
    --to=a.fatoum@barebox.org \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox