[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f035cbed-bff9-426d-9da2-1bc34ef644d3@huaweicloud.com>
Date: Tue, 7 Jan 2025 09:46:03 +0800
From: Luo Gengkun <luogengkun@...weicloud.com>
To: James Clark <james.clark@...aro.org>
Cc: mingo@...hat.com, acme@...nel.org, namhyung@...nel.org,
mark.rutland@....com, alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
irogers@...gle.com, adrian.hunter@...el.com, kan.liang@...ux.intel.com,
tglx@...utronix.de, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, hpa@...or.com, will@...nel.org, paul@...l-moore.com,
jmorris@...ei.org, serge@...lyn.com, rostedt@...dmis.org,
mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
stephen.smalley.work@...il.com, omosnace@...hat.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-security-module@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, selinux@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH linux-next 2/2] perf: Return EACCESS when need perfmon
capability
On 2025/1/6 23:59, James Clark wrote:
>
>
> On 23/12/2024 7:06 am, Luo Gengkun wrote:
>> For perf_allow_kernel and perf_allow_cpu, both return EACCES when
>> require
>> CAP_PERFMON or CAP_SYS_ADMIN permissions, so update
>> perf_allow_tracepoint
>> to keep them the same.
>>
>> Signed-off-by: Luo Gengkun <luogengkun@...weicloud.com>
>> ---
>> include/linux/perf_event.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index 5d2ec4283ebf..c1bc0d7a275b 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -1685,7 +1685,7 @@ static inline int perf_allow_cpu(void)
>> static inline int perf_allow_tracepoint(void)
>> {
>> if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
>> - return -EPERM;
>> + return -EACCES;
>
> Is this necessary other than for consistency? If not it might be best
> to leave it inconsistent even if it's wrong. I see quite a few "if
> EPERM do this..." type things in Perf, so changing this would break
> error messages being shown to users.
>
> If anything, EPERM seems more correct because EACCESS is more about
> file access.
I think so, from the perspective of capabilities and
sysctl_perf_event_paranoid, EPERM is more appropriate.
>
> Thanks
> James
Thanks for your review.
Actually, I am not sure if it's typo or intentional, so this patch is
more like a consultation. It's okay to keep it the same so it doesn't
torture the user.
Thanks
Gengkun
Powered by blists - more mailing lists