On Thu, Nov 28, 2024 at 01:07:42AM +0100, Ilya Shipitsin wrote:
> This defect was found by the coccinelle script "unchecked-strdup.cocci".
> It can be backported to all supported branches.
> ---
> src/resolvers.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/resolvers.c b/src/resolvers.c
> index f8f0c8edf..bff5c290d 100644
> --- a/src/resolvers.c
> +++ b/src/resolvers.c
> @@ -3496,6 +3496,10 @@ static int resolvers_new(struct resolvers **resolvers,
> const char *id, const cha
> resolvers_setup_proxy(p);
> p->parent = r;
> p->id = strdup(id);
> + if (!p->id) {
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> p->conf.args.file = p->conf.file = copy_file_name(file);
> p->conf.args.line = p->conf.line = linenum;
> r->px = p;
> @@ -3503,8 +3507,16 @@ static int resolvers_new(struct resolvers **resolvers,
> const char *id, const cha
> /* default values */
> LIST_APPEND(&sec_resolvers, &r->list);
> r->conf.file = strdup(file);
> + if (!r->conf.file) {
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> r->conf.line = linenum;
> r->id = strdup(id);
> + if (!r->id) {
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> r->query_ids = EB_ROOT;
> /* default maximum response size */
> r->accepted_payload_size = 512;
Some previously allocated entries will leak because the out: label
doesn't take care of the ones you've spotted. Ideally we should a few
"err_free_xxx" labels at the end to unroll all allocations un reverse
order, that are easier to read. E.g:
out:
return err_code;
/* free all allocated stuff and return err_code */
err_free_conf_file:
free(p->conf_file);
err_free_p:
free(p);
err_free_r:
free(r);
return err_code;
And each error check just does "goto err_xxx" to free all the stuff
allocated before itself.
Thanks!
Willy
