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
next prev 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