linux-kernel - Re: [PATCH] perf probe: Show correct error about @symbol for uprobe

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230803173106.GA15866@redhat.com>
Date:   Thu, 3 Aug 2023 19:31:06 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        linux-perf-users@...r.kernel.org,
        Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf probe: Show correct error about @symbol for uprobe

Hi Masami,

On 08/01, Masami Hiramatsu wrote:
>
> Oleg, what do you think about accessing symbols in data section from
> uprobes? Can we access it from user-land IP-relative address?

Sorry, I don't I understand... I don't even understand the context, can't
find the whole thread on lore.kernel.org/lkml. Plus I forgot EVERYTHING
I knew about tracing/uprobes anyway ;)

but when I look at traceprobe_parse_probe_arg() paths it seems to me that
uprobe can fetch the IP-relative address, not sure about the syntax but
perhaps something like xxx=+OFFSET(%ip). The question is how tools/perf
can calculate this OFFSET. But I am sure you understand this all much
better than me.

> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -2800,13 +2800,18 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
> > >  	if (!tev->uprobes || tev->nargs == 0 || !buf)
> > >  		goto out;
> > >
> > > -	for (i = 0; i < tev->nargs; i++)
> > > -		if (strglobmatch(tev->args[i].value, "[$@+-]*")) {
> > > -			pr_warning("Please upgrade your kernel to at least "
> > > -				   "3.14 to have access to feature %s\n",
> > > +	for (i = 0; i < tev->nargs; i++) {
> > > +		if (strchr(tev->args[i].value, '@')) {
> > > +			pr_warning("%s accesses a variable by symbol name, but that is not supported for user application probe.\n",
> > > +				   tev->args[i].value);
> > > +			break;

IIUC without this change @symbol will trigger the

			/* uprobes don't support symbols */
			if (!(ctx->flags & TPARG_FL_KERNEL)) {
				trace_probe_log_err(ctx->offset, SYM_ON_UPROBE);
				return -EINVAL;
			}

in parse_probe_arg(), right?

So FWIW the patch looks fine to me, but as you have mentioned tools/perf
could probably (try to) turn @symbol into @+symbol_offset_in_file...

In short, sorry for spam, I can't help ;)

And just in case, I am on PTO till Aug 14, won't be able to read emails
till then.

Oleg.