| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUor0SPQ8PjEHHaE1qSnZ5TX1H=feUCERzwXGibqKAfNg@mail.gmail.com>
Date: Tue, 27 Jan 2026 11:07:41 -0800
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Adrian Hunter <adrian.hunter@...el.com>, James Clark <james.clark@...aro.org>,
Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 1/1 next] perf strlist: Remove dont_dupstr logic, used
only once
On Tue, Jan 27, 2026 at 8:09 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Ian Rogers noticed that 678ed6b707e4b2db ("perf strlist: Don't write to
> const memory") breaks the 'Remove thread map' 'perf test' entry, because
> it keeps pointers to the temporary string introduced to avoid touching
> the const memory.
>
> This is because the thread_map__new_by_[pt]id_str() were the only
> methods using the slist->dont_dupstr knob to keep pointers to the
> original const string list, as it uses strtol to parse numbers and it
> stops at the comma.
>
> As this is the only case of dont_dupstr use, dupstr being the default,
> and it gets in the way of getting rid of the last const-correctness,
> remove this knob, with it:
>
> $ perf test 37
> 37: Remove thread map : Ok
> $
>
> Fixes: 678ed6b707e4b2db ("perf strlist: Don't write to const memory")
> Reported-by: Ian Rogers <irogers@...gle.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
Tested-by: Ian Rogers <irogers@...gle.com>
Thanks,
Ian
> ---
> tools/perf/util/strlist.c | 25 ++++++++-----------------
> tools/perf/util/strlist.h | 2 --
> tools/perf/util/thread_map.c | 18 ++++++------------
> 3 files changed, 14 insertions(+), 31 deletions(-)
>
> diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
> index 98883672fcf47102..50add72575e0c60c 100644
> --- a/tools/perf/util/strlist.c
> +++ b/tools/perf/util/strlist.c
> @@ -12,20 +12,16 @@
> #include <linux/zalloc.h>
>
> static
> -struct rb_node *strlist__node_new(struct rblist *rblist, const void *entry)
> +struct rb_node *strlist__node_new(struct rblist *rblist __maybe_unused, const void *entry)
> {
> const char *s = entry;
> struct rb_node *rc = NULL;
> - struct strlist *strlist = container_of(rblist, struct strlist, rblist);
> struct str_node *snode = malloc(sizeof(*snode));
>
> if (snode != NULL) {
> - if (strlist->dupstr) {
> - s = strdup(s);
> - if (s == NULL)
> - goto out_delete;
> - }
> - snode->s = s;
> + snode->s = strdup(s);
> + if (snode->s == NULL)
> + goto out_delete;
> rc = &snode->rb_node;
> }
>
> @@ -36,20 +32,18 @@ struct rb_node *strlist__node_new(struct rblist *rblist, const void *entry)
> return NULL;
> }
>
> -static void str_node__delete(struct str_node *snode, bool dupstr)
> +static void str_node__delete(struct str_node *snode)
> {
> - if (dupstr)
> - zfree((char **)&snode->s);
> + zfree((char **)&snode->s);
> free(snode);
> }
>
> static
> -void strlist__node_delete(struct rblist *rblist, struct rb_node *rb_node)
> +void strlist__node_delete(struct rblist *rblist __maybe_unused, struct rb_node *rb_node)
> {
> - struct strlist *slist = container_of(rblist, struct strlist, rblist);
> struct str_node *snode = container_of(rb_node, struct str_node, rb_node);
>
> - str_node__delete(snode, slist->dupstr);
> + str_node__delete(snode);
> }
>
> static int strlist__node_cmp(struct rb_node *rb_node, const void *entry)
> @@ -165,12 +159,10 @@ struct strlist *strlist__new(const char *list, const struct strlist_config *conf
> struct strlist *slist = malloc(sizeof(*slist));
>
> if (slist != NULL) {
> - bool dupstr = true;
> bool file_only = false;
> const char *dirname = NULL;
>
> if (config) {
> - dupstr = !config->dont_dupstr;
> dirname = config->dirname;
> file_only = config->file_only;
> }
> @@ -180,7 +172,6 @@ struct strlist *strlist__new(const char *list, const struct strlist_config *conf
> slist->rblist.node_new = strlist__node_new;
> slist->rblist.node_delete = strlist__node_delete;
>
> - slist->dupstr = dupstr;
> slist->file_only = file_only;
>
> if (list && strlist__parse_list(slist, list, dirname) != 0)
> diff --git a/tools/perf/util/strlist.h b/tools/perf/util/strlist.h
> index 7e82c71dcc422d7e..3e9533e66ca9ee11 100644
> --- a/tools/perf/util/strlist.h
> +++ b/tools/perf/util/strlist.h
> @@ -14,7 +14,6 @@ struct str_node {
>
> struct strlist {
> struct rblist rblist;
> - bool dupstr;
> bool file_only;
> };
>
> @@ -24,7 +23,6 @@ struct strlist {
> * found
> */
> struct strlist_config {
> - bool dont_dupstr;
> bool file_only;
> const char *dirname;
> };
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index ca193c1374ed4823..48c70f149e9201dc 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -164,19 +164,16 @@ static struct perf_thread_map *thread_map__new_by_pid_str(const char *pid_str)
> struct dirent **namelist = NULL;
> int i, j = 0;
> pid_t pid, prev_pid = INT_MAX;
> - char *end_ptr;
> struct str_node *pos;
> - struct strlist_config slist_config = { .dont_dupstr = true, };
> - struct strlist *slist = strlist__new(pid_str, &slist_config);
> + struct strlist *slist = strlist__new(pid_str, NULL);
>
> if (!slist)
> return NULL;
>
> strlist__for_each_entry(pos, slist) {
> - pid = strtol(pos->s, &end_ptr, 10);
> + pid = strtol(pos->s, NULL, 10);
>
> - if (pid == INT_MIN || pid == INT_MAX ||
> - (*end_ptr != '\0' && *end_ptr != ','))
> + if (pid == INT_MIN || pid == INT_MAX)
> goto out_free_threads;
>
> if (pid == prev_pid)
> @@ -223,24 +220,21 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str)
> struct perf_thread_map *threads = NULL, *nt;
> int ntasks = 0;
> pid_t tid, prev_tid = INT_MAX;
> - char *end_ptr;
> struct str_node *pos;
> - struct strlist_config slist_config = { .dont_dupstr = true, };
> struct strlist *slist;
>
> /* perf-stat expects threads to be generated even if tid not given */
> if (!tid_str)
> return perf_thread_map__new_dummy();
>
> - slist = strlist__new(tid_str, &slist_config);
> + slist = strlist__new(tid_str, NULL);
> if (!slist)
> return NULL;
>
> strlist__for_each_entry(pos, slist) {
> - tid = strtol(pos->s, &end_ptr, 10);
> + tid = strtol(pos->s, NULL, 10);
>
> - if (tid == INT_MIN || tid == INT_MAX ||
> - (*end_ptr != '\0' && *end_ptr != ','))
> + if (tid == INT_MIN || tid == INT_MAX)
> goto out_free_threads;
>
> if (tid == prev_tid)
> --
> 2.52.0
>
Powered by blists - more mailing lists