# \[PATCH\] perf/core: fix unconditional security\_locked\_down() call - Subsystems: PERFORMANCE EVENTS SUBSYSTEM, THE REST - Maintainers: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Linus Torvalds - Status: LANDED (08ef1af4de5f ยท 2021-03-16 20:44) - Landed in mainline as 08ef1af4de5f on 2021-03-16. - List: linux-security-module - 4 messages, 3 authors - Page: https://ratatoskr.run/linux-security-module/2021/02/6278328/t ## \[PATCH\] perf/core: fix unconditional security\_locked\_down() call - From: Ondrej Mosnacek \ - Date: 2021-02-24 21:58:18 - Message: https://ratatoskr.run/linux-security-module/2021/02/6278328.md ```text Currently, the lockdown state is queried unconditionally, even though its result is used only if the PERF_SAMPLE_REGS_INTR bit is set in attr.sample_type. While that doesn't matter in case of the Lockdown LSM, it causes trouble with the SELinux's lockdown hook implementation. SELinux implements the locked_down hook with a check whether the current task's type has the corresponding "lockdown" class permission ("integrity" or "confidentiality") allowed in the policy. This means that calling the hook when the access control decision would be ignored generates a bogus permission check and audit record. Fix this by checking sample_type first and only calling the hook when its result would be honored. Fixes: b0c8fdc7fdb7 ("lockdown: Lock down perf when in confidentiality mode") Signed-off-by: Ondrej Mosnacek --- kernel/events/core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 129dee540a8b..0f857307e9bd 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11796,12 +11796,12 @@ SYSCALL_DEFINE5(perf_event_open, return err; } - err = security_locked_down(LOCKDOWN_PERF); - if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR)) - /* REGS_INTR can leak data, lockdown must prevent this */ - return err; - - err = 0; + /* REGS_INTR can leak data, lockdown must prevent this */ + if (attr.sample_type & PERF_SAMPLE_REGS_INTR) { + err = security_locked_down(LOCKDOWN_PERF); + if (err) + return err; + } /* * In cgroup mode, the pid argument is used to pass the fd -- 2.29.2 ``` ## Re: \[PATCH\] perf/core: fix unconditional security\_locked\_down() call - From: Paul Moore \ - Date: 2021-03-16 13:54:15 - Message: https://ratatoskr.run/linux-security-module/2021/03/6303305.md ```text On Wed, Feb 24, 2021 at 4:59 PM Ondrej Mosnacek [ref] wrote: > > Currently, the lockdown state is queried unconditionally, even though > its result is used only if the PERF_SAMPLE_REGS_INTR bit is set in > attr.sample_type. While that doesn't matter in case of the Lockdown LSM, > it causes trouble with the SELinux's lockdown hook implementation. > > SELinux implements the locked_down hook with a check whether the current > task's type has the corresponding "lockdown" class permission > ("integrity" or "confidentiality") allowed in the policy. This means > that calling the hook when the access control decision would be ignored > generates a bogus permission check and audit record. > > Fix this by checking sample_type first and only calling the hook when > its result would be honored. > > Fixes: b0c8fdc7fdb7 ("lockdown: Lock down perf when in confidentiality mode") > Signed-off-by: Ondrej Mosnacek > --- > kernel/events/core.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) Perf/core folks, do you want to pull this in via your tree? If I don't hear anything in the next day I'll pull this in via the selinux/next tree. Reviewed-by: Paul Moore > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 129dee540a8b..0f857307e9bd 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -11796,12 +11796,12 @@ SYSCALL_DEFINE5(perf_event_open, > return err; > } > > - err = security_locked_down(LOCKDOWN_PERF); > - if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR)) > - /* REGS_INTR can leak data, lockdown must prevent this */ > - return err; > - > - err = 0; > + /* REGS_INTR can leak data, lockdown must prevent this */ > + if (attr.sample_type & PERF_SAMPLE_REGS_INTR) { > + err = security_locked_down(LOCKDOWN_PERF); > + if (err) > + return err; > + } > > /* > * In cgroup mode, the pid argument is used to pass the fd > -- > 2.29.2 -- paul moore www.paul-moore.com ``` ## Re: \[PATCH\] perf/core: fix unconditional security\_locked\_down() call - From: Peter Zijlstra \ - Date: 2021-03-16 14:31:43 - Message: https://ratatoskr.run/linux-security-module/2021/03/6303340.md ```text On Tue, Mar 16, 2021 at 09:53:21AM -0400, Paul Moore wrote: > On Wed, Feb 24, 2021 at 4:59 PM Ondrej Mosnacek [ref] wrote: > > > > Currently, the lockdown state is queried unconditionally, even though > > its result is used only if the PERF_SAMPLE_REGS_INTR bit is set in > > attr.sample_type. While that doesn't matter in case of the Lockdown LSM, > > it causes trouble with the SELinux's lockdown hook implementation. > > > > SELinux implements the locked_down hook with a check whether the current > > task's type has the corresponding "lockdown" class permission > > ("integrity" or "confidentiality") allowed in the policy. This means > > that calling the hook when the access control decision would be ignored > > generates a bogus permission check and audit record. > > > > Fix this by checking sample_type first and only calling the hook when > > its result would be honored. > > > > Fixes: b0c8fdc7fdb7 ("lockdown: Lock down perf when in confidentiality mode") > > Signed-off-by: Ondrej Mosnacek > > --- > > kernel/events/core.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > Perf/core folks, do you want to pull this in via your tree? If I > don't hear anything in the next day I'll pull this in via the > selinux/next tree. > > Reviewed-by: Paul Moore Ah, fell in the cracks... I've no idea what Changelog is trying to tell me. It is pure gibberish to me. But the patch seems harmless enough to me. Let me queue it then. ``` ## Re: \[PATCH\] perf/core: fix unconditional security\_locked\_down() call - From: Paul Moore \ - Date: 2021-03-16 15:15:06 - Message: https://ratatoskr.run/linux-security-module/2021/03/6303396.md ```text On Tue, Mar 16, 2021 at 10:30 AM Peter Zijlstra [ref] wrote: > On Tue, Mar 16, 2021 at 09:53:21AM -0400, Paul Moore wrote: > > On Wed, Feb 24, 2021 at 4:59 PM Ondrej Mosnacek [ref] wrote: > > > > > > Currently, the lockdown state is queried unconditionally, even though > > > its result is used only if the PERF_SAMPLE_REGS_INTR bit is set in > > > attr.sample_type. While that doesn't matter in case of the Lockdown LSM, > > > it causes trouble with the SELinux's lockdown hook implementation. > > > > > > SELinux implements the locked_down hook with a check whether the current > > > task's type has the corresponding "lockdown" class permission > > > ("integrity" or "confidentiality") allowed in the policy. This means > > > that calling the hook when the access control decision would be ignored > > > generates a bogus permission check and audit record. > > > > > > Fix this by checking sample_type first and only calling the hook when > > > its result would be honored. > > > > > > Fixes: b0c8fdc7fdb7 ("lockdown: Lock down perf when in confidentiality mode") > > > Signed-off-by: Ondrej Mosnacek > > > --- > > > kernel/events/core.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > Perf/core folks, do you want to pull this in via your tree? If I > > don't hear anything in the next day I'll pull this in via the > > selinux/next tree. > > > > Reviewed-by: Paul Moore > > Ah, fell in the cracks... I've no idea what Changelog is trying to tell > me. It is pure gibberish to me. But the patch seems harmless enough to me. > > Let me queue it then. Great, thanks. -- paul moore www.paul-moore.com ```