Hi Ilya,
Thanks for these patches. Please be careful, most parsing or init
functions that return ERR_ALERT are expected to have either printed
the alert or produced it in the "err" field. Sadly there's no general
rule to know which one uses what, generally it's needed to see what
other parts are doing in the same function. All generic keyword
parsers at least use memprintf(). I've checked them and here are the
needed changes for the 4 patches:
On Tue, Dec 10, 2024 at 01:23:08PM +0100, Ilia Shipitsin wrote:
> diff --git a/src/check.c b/src/check.c
> index 29a86b7dd..e636148b6 100644
> --- a/src/check.c
> +++ b/src/check.c
> @@ -1667,6 +1667,8 @@ static int start_checks()
> /* 0- init the dummy frontend used to create all checks sessions */
> init_new_proxy(&checks_fe);
> checks_fe.id = strdup("CHECKS-FE");
> + if ( !checks_fe.id )
(side note: ^ ^ please do not add these unneeded spaces).
This block should emit the error directly using ha_alert(), please see
around line 1715, there's one already, I think it should be something
like:
ha_alert("Out of memory creating the checks frontend.\n");
> + return ERR_ALERT | ERR_FATAL;
> checks_fe.cap = PR_CAP_FE | PR_CAP_BE;
> checks_fe.mode = PR_MODE_TCP;
> checks_fe.maxconn = 0;
> diff --git a/src/listener.c b/src/listener.c
> index 5f3a98b4a..5fb610a88 100644
> --- a/src/listener.c
> +++ b/src/listener.c
> @@ -2369,8 +2369,11 @@ static int bind_parse_name(char **args, int cur_arg,
> struct proxy *px, struct bi
> return ERR_ALERT | ERR_FATAL;
> }
>
> - list_for_each_entry(l, &conf->listeners, by_bind)
> + list_for_each_entry(l, &conf->listeners, by_bind) {
> l->name = strdup(args[cur_arg + 1]);
> + if (!l->name)
This block should do:
memprintf(err, "'%s %s' : out of memory", args[cur_arg], arg[cur_arg +
1]);
> + return ERR_ALERT | ERR_FATAL;
> + }
>
> return 0;
> }
> diff --git a/src/mux_h1.c b/src/mux_h1.c
> index 7eb18133d..0206dcbd0 100644
> --- a/src/mux_h1.c
> +++ b/src/mux_h1.c
> @@ -5663,7 +5663,10 @@ static int cfg_parse_h1_headers_case_adjust_file(char
> **args, int section_type,
> }
> free(hdrs_map.name);
> hdrs_map.name = strdup(args[1]);
> - return 0;
> + if (!hdrs_map.name)
This block should do:
memprintf(err, "'%s %s' : out of memory", args[0], arg[1]);
> + return -1;
> +
> + return 0;
> }
> diff --git a/src/debug.c b/src/debug.c
> index d052f5b63..2c5e35553 100644
> --- a/src/debug.c
> +++ b/src/debug.c
> @@ -2048,6 +2048,8 @@ static int debug_parse_cli_memstats(char **args, char
> *payload, struct appctx *a
> else if (strcmp(args[arg], "match") == 0 && *args[arg + 1]) {
> ha_free(&ctx->match);
> ctx->match = strdup(args[arg + 1]);
> + if (!ctx->match)
> + return 1;
For this one, it's a runtime CLI keyword parser, which is more delicate
than the other ones (and a *real* problem as it's possible to crash the
process at runtime while using the command). This one should do it:
return cli_err(appctx, "Out of memory.\n");
and you can drop the return 1.
Thank you!
Willy
