Re: [PATCH v8 28/29] perf kwork: Fix address sanitizer issues

sashiko-bot
2026-04-11T07:20:08Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Memory leak of the `rec_argv` array if the `to_free` allocation fails.
- [Medium] Memory leak of duplicated argument strings on the `STRDUP_FAIL_EXIT` error path.
--

commit 4177ff06047a8ec115a7e41c18ecb9f7b6bbf2d5
Author: Ian Rogers <irogers@google.com>

perf kwork: Fix address sanitizer issues

This commit addresses a double free in the record array caused by how
parse_options mutates the array. It fixes this by maintaining a separate,
unmutated array to track the pointers for freeing.

> diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> --- a/tools/perf/builtin-kwork.c
> +++ b/tools/perf/builtin-kwork.c
> @@ -2279,6 +2279,9 @@ static int perf_kwork__record(struct perf_kwork *kwork,
> rec_argv = calloc(rec_argc + 1, sizeof(char *));
> if (rec_argv == NULL)
> return -ENOMEM;
> + to_free = calloc(rec_argc + 1, sizeof(char *));
> + if (to_free == NULL)
> + return -ENOMEM;

If the allocation for to_free fails, does this code leak the newly allocated
rec_argv?

>
> for (i = 0; i < ARRAY_SIZE(record_args); i++)
> rec_argv[i] = STRDUP_FAIL_EXIT(record_args[i]);

[ ... ]

> @@ -2296,15 +2299,20 @@ static int perf_kwork__record(struct perf_kwork *kwork,
> BUG_ON(i != rec_argc);
>
> pr_debug("record comm: ");
> - for (j = 0; j < rec_argc; j++)
> + for (j = 0; j < rec_argc; j++) {
> pr_debug("%s ", rec_argv[j]);
> + /* Save the pointers as the array will be mutated by cmd_record. */
> + to_free[j] = rec_argv[j];
> + }
> pr_debug("\n");
>
> ret = cmd_record(i, rec_argv);
>
> EXIT:
> for (i = 0; i < rec_argc; i++)
> - free((void *)rec_argv[i]);
> + free((void *)to_free[i]);

Since to_free is only populated immediately before calling cmd_record(),
what happens if STRDUP_FAIL_EXIT() triggers a jump to EXIT midway through
the string allocations?

Would the loop here iterate over NULL pointers in to_free and leave the
successfully duplicated strings in rec_argv leaked?

> +
> + free(to_free);
> free(rec_argv);
> return ret;
> }

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260411065718.372240-1-irogers@google.com?part=28