|
Message-ID: <CAG48ez1xu_WJi5_UZvrTO_H6o=DHRvm5LMp_jg=GauLbpkYARg@mail.gmail.com> Date: Wed, 26 Sep 2018 23:16:35 +0200 From: Jann Horn <jannh@...gle.com> To: Casey Schaufler <casey.schaufler@...el.com> Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, kernel list <linux-kernel@...r.kernel.org>, linux-security-module <linux-security-module@...r.kernel.org>, selinux@...ho.nsa.gov, Dave Hansen <dave.hansen@...el.com>, deneen.t.dock@...el.com, kristen@...ux.intel.com, Arjan van de Ven <arjan@...ux.intel.com> Subject: Re: [PATCH v5 1/5] AppArmor: Prepare for PTRACE_MODE_SCHED On Wed, Sep 26, 2018 at 10:35 PM Casey Schaufler <casey.schaufler@...el.com> wrote: > A ptrace access check with mode PTRACE_MODE_SCHED gets called > from process switching code. This precludes the use of audit, > as the locking is incompatible. Don't do audit in the PTRACE_MODE_SCHED > case. Why is this separate from PTRACE_MODE_NOAUDIT? It looks like apparmor_ptrace_access_check() currently ignores PTRACE_MODE_NOAUDIT. Could you, instead of adding a new flag, fix the handling of PTRACE_MODE_NOAUDIT? > Signed-off-by: Casey Schaufler <casey.schaufler@...el.com> > --- > security/apparmor/domain.c | 2 +- > security/apparmor/include/ipc.h | 2 +- > security/apparmor/ipc.c | 8 +++++--- > security/apparmor/lsm.c | 5 +++-- > 4 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 08c88de0ffda..28300f4c3ef9 100644 > --- a/security/apparmor/domain.c > +++ b/security/apparmor/domain.c > @@ -77,7 +77,7 @@ static int may_change_ptraced_domain(struct aa_label *to_label, > if (!tracer || unconfined(tracerl)) > goto out; > > - error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH); > + error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH, true); > > out: > rcu_read_unlock(); > diff --git a/security/apparmor/include/ipc.h b/security/apparmor/include/ipc.h > index 5ffc218d1e74..299d1c45fef0 100644 > --- a/security/apparmor/include/ipc.h > +++ b/security/apparmor/include/ipc.h > @@ -34,7 +34,7 @@ struct aa_profile; > "xcpu xfsz vtalrm prof winch io pwr sys emt lost" > > int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee, > - u32 request); > + u32 request, bool audit); > int aa_may_signal(struct aa_label *sender, struct aa_label *target, int sig); > > #endif /* __AA_IPC_H */ > diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c > index 527ea1557120..9ed110afc822 100644 > --- a/security/apparmor/ipc.c > +++ b/security/apparmor/ipc.c > @@ -121,15 +121,17 @@ static int profile_tracer_perm(struct aa_profile *tracer, > * Returns: %0 else error code if permission denied or error > */ > int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee, > - u32 request) > + u32 request, bool audit) > { > struct aa_profile *profile; > u32 xrequest = request << PTRACE_PERM_SHIFT; > DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE); > > return xcheck_labels(tracer, tracee, profile, > - profile_tracer_perm(profile, tracee, request, &sa), > - profile_tracee_perm(profile, tracer, xrequest, &sa)); > + profile_tracer_perm(profile, tracee, request, > + audit ? &sa : NULL), > + profile_tracee_perm(profile, tracer, xrequest, > + audit ? &sa : NULL)); > } > > > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c > index 8b8b70620bbe..da9d0b228857 100644 > --- a/security/apparmor/lsm.c > +++ b/security/apparmor/lsm.c > @@ -118,7 +118,8 @@ static int apparmor_ptrace_access_check(struct task_struct *child, > tracee = aa_get_task_label(child); > error = aa_may_ptrace(tracer, tracee, > (mode & PTRACE_MODE_READ) ? AA_PTRACE_READ > - : AA_PTRACE_TRACE); > + : AA_PTRACE_TRACE, > + !(mode & PTRACE_MODE_SCHED)); > aa_put_label(tracee); > end_current_label_crit_section(tracer); > > @@ -132,7 +133,7 @@ static int apparmor_ptrace_traceme(struct task_struct *parent) > > tracee = begin_current_label_crit_section(); > tracer = aa_get_task_label(parent); > - error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE); > + error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE, true); > aa_put_label(tracer); > end_current_label_crit_section(tracee); > > -- > 2.17.1 >
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.