linux-kernel - Re: [PATCH 3/4] perf list: Don't write to const memory

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aXF4-vLA9DvhsPdc@x1>
Date: Wed, 21 Jan 2026 22:10:18 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: David Laight <david.laight.linux@...il.com>
Cc: Namhyung Kim <namhyung@...nel.org>, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	James Clark <james.clark@...aro.org>, Jiri Olsa <jolsa@...nel.org>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Clark Williams <williams@...hat.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH 3/4] perf list: Don't write to const memory

On Wed, Jan 21, 2026 at 10:17:13PM +0000, David Laight wrote:
> On Tue, 20 Jan 2026 19:08:59 -0300
> Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> 
> > From: Arnaldo Carvalho de Melo <acme@...hat.com>
> > 
> > Something now detected on fedora 44, where strchr() returns const if it
> > is passed a const pointer:
> > 
> >   util/print-events.c: In function 'print_sdt_events':
> >   util/print-events.c:89:29: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
> >      89 |                 char *bid = strchr(sdt_name->s, '@');
> >         |                             ^~~~~~
> > 
> > Fix it by using strnchr() if strchr finds the separator instead of
> > temporarily scrubbing it with '\0'.
> 
> I've just looked at the full function to see WTF it is doing.
> You've fixed the second strchr() not the one the compiler bleated about.

It complained about both, no?

> Line 89 is followed by:
> 	if (bid)
> 		*bid++ = 0;
> but if it is NULL there is a fair chance the code will just explode a bit later on.

Why, can you elaborate?

> I suspect it is an error if the strings aren't "name@bar"
 
> 'show_detail' seems to be set if either the previous or next entries in the
> list/tree have the same "name" - which seems strange to me.

All this is strange, a corner case, it looks as if we may have multiple
binaries, differentiated by buildids, that would have different SDTs.

The code seems to be trying to reduce the amount of info (not showing
the buildid if there are no more at that point multiple versions of a
DSO with SDTs in place).

Further analysis of the changesets that put all this in place is needed
to clarify, but looking just at fixing what the compiler is complaining,
do you think the patches are bad?

- Arnaldo
 
> The while thing needs more work :-(
> 
> 	David
> 
> > 
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> > ---
> >  tools/perf/util/print-events.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> > index 8f3ed83853a9e468..898cf426509790cd 100644
> > --- a/tools/perf/util/print-events.c
> > +++ b/tools/perf/util/print-events.c
> > @@ -97,14 +97,11 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state)
> >  		} else {
> >  			next_sdt_name = strlist__next(sdt_name);
> >  			if (next_sdt_name) {
> > -				char *bid2 = strchr(next_sdt_name->s, '@');
> > -
> > -				if (bid2)
> > -					*bid2 = '\0';
> > -				if (strcmp(sdt_name->s, next_sdt_name->s) == 0)
> > -					show_detail = true;
> > -				if (bid2)
> > -					*bid2 = '@';
> > +				const char *bid2 = strchr(next_sdt_name->s, '@');
> > +
> > +				show_detail = bid2 ?
> > +						strncmp(sdt_name->s, next_sdt_name->s, bid2 - next_sdt_name->s) == 0 :
> > +						strcmp(sdt_name->s, next_sdt_name->s) == 0;
> >  			}
> >  		}
> >  		last_sdt_name = sdt_name->s;