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

Date:   Tue, 1 Aug 2023 22:56:18 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>
Cc:     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

(To: Oleg)

On Tue, 1 Aug 2023 09:55:07 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:

> Em Fri, Jul 28, 2023 at 11:19:30PM +0900, Masami Hiramatsu (Google) escreveu:
> > From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > 
> > Since @symbol variable access is not supported by uprobe event, it must be
> > correctly warn user instead of kernel version update.
> 
> Thanks, less cryptic now, applied.
> 
> But is that just a matter of writing code to support reading global
> variables from an uprobe?

Hm, maybe we can use "@+offset" for accessing symboled data, as far as
it is loaded with the text section.
Oleg, what do you think about accessing symbols in data section from
uprobes? Can we access it from user-land IP-relative address?

Thank you,

> 
> - Arnaldo
> 
>  
> > Reported-by: Arnaldo Carvalho de Melo <acme@...nel.org>
> > Closes: https://lore.kernel.org/all/ZLWDEjvFjrrEJODp@kernel.org/
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > ---
> >  tools/perf/util/probe-event.c |   13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 6e2110d605fb..c1ded85fe859 100644
> > --- 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;
> > +		}
> > +		if (strglobmatch(tev->args[i].value, "[$+-]*")) {
> > +			pr_warning("Please upgrade your kernel to at least 3.14 to have access to feature %s\n",
> >  				   tev->args[i].value);
> >  			break;
> >  		}
> > +	}
> >  out:
> >  	free(buf);
> >  }
> > 
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>