Re: [PATCH 2/2] perf/probe: Report permission error for tracefs error

From: Masami Hiramatsu <mhiramat@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>,
	Jiri Olsa <jolsa@kernel.org>,
	linux-kernel@vger.kernel.org, aneesh.kumar@linux.ibm.com,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>
Subject: Re: [PATCH 2/2] perf/probe: Report permission error for tracefs error
Date: Sat, 5 Jun 2021 12:56:50 +0900	[thread overview]
Message-ID: <20210605125650.acab55ee6c8e954541bfb85f@kernel.org> (raw)
In-Reply-To: <YLp8ipAslRq0wkOm@kernel.org>

Hi Arnaldo,

On Fri, 4 Jun 2021 16:18:34 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Jun 05, 2021 at 12:31:08AM +0900, Masami Hiramatsu escreveu:
> > Report permission error for the tracefs access error.
> > This can happen when non-superuser runs perf probe.
> > With this patch, perf probe shows the following message.
> > 
> >   $ perf probe -l
> >   No permission to access tracefs. Please run this command again with sudo.
> >     Error: Failed to show event list.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-file.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 52273542e6ef..52d878f5a44d 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -48,6 +48,8 @@ static void print_open_warning(int err, bool uprobe)
> >  			   uprobe ? 'u' : 'k', config);
> >  	} else if (err == -ENOTSUP)
> >  		pr_warning("Tracefs or debugfs is not mounted.\n");
> > +	else if (err == -EACCES)
> > +		pr_warning("No permission to access tracefs. Please run this command again with sudo.\n");
> >  	else
> >  		pr_warning("Failed to open %cprobe_events: %s\n",
> >  			   uprobe ? 'u' : 'k',
> > @@ -62,6 +64,8 @@ static void print_both_open_warning(int kerr, int uerr)
> >  	else if (kerr == -ENOENT && uerr == -ENOENT)
> >  		pr_warning("Please rebuild kernel with CONFIG_KPROBE_EVENTS "
> >  			   "or/and CONFIG_UPROBE_EVENTS.\n");
> > +	else if (kerr == -EACCES && uerr == -EACCES)
> > +		pr_warning("No permission to access tracefs. Please run this command again with sudo.\n");
> >  	else {
> >  		char sbuf[STRERR_BUFSIZE];
> >  		pr_warning("Failed to open kprobe events: %s.\n",
> 
> This one doesn't look so helpful, as running as root usually will allow
> things to proceed.
> 
> 'perf trace' does:
> 
> ⬢[acme@toolbox pahole]$ perf trace ls
> Error:	No permissions to read /sys/kernel/tracing/events/raw_syscalls/sys_(enter|exit)
> Hint:	Try 'sudo mount -o remount,mode=755 /sys/kernel/tracing/'
> 
> ⬢[acme@toolbox pahole]$
> 
> Which would be less drastic than requiring full superuser access.

Hmm, perf trace only read access to the tracefs, so that is easy to
suggest user to do remount it. However, perf probe usually requires
a write access. (Only perf probe -l requires read access)

I'll change this patch to check whether the read or write access and
switch the message. But if it is a write access, anyway it has to
requests superuser.

Let me update this patch.

Thank you,

> 
> - Arnaldo

-- 
Masami Hiramatsu <mhiramat@kernel.org>