Messages in this thread | | | Date | Mon, 20 Dec 2021 08:41:48 -0800 | Subject | Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs | From | Casey Schaufler <> |
| |
On 12/20/2021 2:13 AM, Vishal Goel wrote: > Currently tracer process info is printed in object field in > smack error log for ptrace check which is wrong. > Object process should print the tracee process info. > Tracee info is not printed in the smack error logs. > So it is not possible to debug the ptrace smack issues. > > Now changes has been done to print both tracer and tracee > process info in smack error logs for ptrace scenarios > > Old logs:- > [ 378.098330] audit: type=1400 audit(1637212273.300:2): lsm=SMACK fn=smack_ptrace_access_check action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=9397 comm="tst_pt" opid=9397 ocomm="tst_pt" > [ 520.261605] audit: type=1400 audit(1637212415.464:3): lsm=SMACK fn=smack_ptrace_traceme action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=12685 comm="tst_pt_me" opid=12563 ocomm="bash" > [ 1445.259319] audit: type=1400 audit(1637213340.460:5): lsm=SMACK fn=smack_bprm_set_creds action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=1778 comm="tst_bprm" opid=1776 ocomm="tst_bprm" > > New logs:- > [ 378.098330] audit: type=1400 audit(1637212273.300:2): lsm=SMACK fn=smack_ptrace_access_check action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=5189 tracer-comm="tst_pt" pid=5189 comm="tst_pt" tracee-pid=962 tracee-comm="test_tracee" > [ 520.261605] audit: type=1400 audit(1637212415.464:3): lsm=SMACK fn=smack_ptrace_traceme action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=6161 tracer-comm="bash" pid=6310 comm="tst_pt_me" tracee-pid=6310 tracee-comm="tst_pt_me" > [ 1445.259319] audit: type=1400 audit(1637213340.460:5): lsm=SMACK fn=smack_bprm_set_creds action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=6435 tracer-comm="tst_bprm" pid=6436 comm="tst_bprm" tracee-pid=6436 tracee-comm="tst_bprm" > > Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
Added linux-audit to the CC list.
> --- > include/linux/lsm_audit.h | 1 + > security/lsm_audit.c | 15 +++++++++++++++ > security/smack/smack.h | 19 +++++++++++++++++++ > security/smack/smack_access.c | 16 ++++++++++++++++ > security/smack/smack_lsm.c | 33 ++++++++++++++++++++++----------- > 5 files changed, 73 insertions(+), 11 deletions(-) > > diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h > index 17d02eda9..6d752ea16 100644 > --- a/include/linux/lsm_audit.h > +++ b/include/linux/lsm_audit.h > @@ -76,6 +76,7 @@ struct common_audit_data { > #define LSM_AUDIT_DATA_IBENDPORT 14 > #define LSM_AUDIT_DATA_LOCKDOWN 15 > #define LSM_AUDIT_DATA_NOTIFICATION 16 > +#define LSM_AUDIT_DATA_PTRACE 17 > union { > struct path path; > struct dentry *dentry; > diff --git a/security/lsm_audit.c b/security/lsm_audit.c > index 1897cbf6f..069e0282c 100644 > --- a/security/lsm_audit.c > +++ b/security/lsm_audit.c > @@ -318,6 +318,21 @@ static void dump_common_audit_data(struct audit_buffer *ab, > } > break; > } > + case LSM_AUDIT_DATA_PTRACE: { > + struct task_struct *tsk = a->u.tsk; > + if (tsk) { > + pid_t pid = task_tgid_nr(tsk); > + > + if (pid) { > + char comm[sizeof(tsk->comm)]; > + > + audit_log_format(ab, " tracee-pid=%d tracee-comm=", pid); > + audit_log_untrustedstring(ab, > + memcpy(comm, tsk->comm, sizeof(comm))); > + } > + } > + break; > + } > case LSM_AUDIT_DATA_NET: > if (a->u.net->sk) { > const struct sock *sk = a->u.net->sk; > diff --git a/security/smack/smack.h b/security/smack/smack.h > index 99c342259..901228205 100644 > --- a/security/smack/smack.h > +++ b/security/smack/smack.h > @@ -266,6 +266,7 @@ struct smack_audit_data { > char *object; > char *request; > int result; > + struct task_struct *tracer_tsk; > }; > > /* > @@ -497,6 +498,16 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a, > { > a->a.u.net->sk = sk; > } > +static inline void smk_ad_setfield_u_tracer(struct smk_audit_info *a, > + struct task_struct *t) > +{ > + a->a.smack_audit_data->tracer_tsk = t; > +} > +static inline void smk_ad_setfield_u_tracee(struct smk_audit_info *a, > + struct task_struct *t) > +{ > + a->a.u.tsk = t; > +} > > #else /* no AUDIT */ > > @@ -524,6 +535,14 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a, > struct sock *sk) > { > } > +static inline void smk_ad_setfield_u_tracer(struct smk_audit_info *a, > + struct task_struct *t) > +{ > +} > +static inline void smk_ad_setfield_u_tracee(struct smk_audit_info *a, > + struct task_struct *t) > +{ > +} > #endif > > #endif /* _SECURITY_SMACK_H */ > diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c > index d2186e275..f39e3ac92 100644 > --- a/security/smack/smack_access.c > +++ b/security/smack/smack_access.c > @@ -323,6 +323,22 @@ static void smack_log_callback(struct audit_buffer *ab, void *a) > audit_log_format(ab, " labels_differ"); > else > audit_log_format(ab, " requested=%s", sad->request); > + > + if (ad->type == LSM_AUDIT_DATA_PTRACE) { > + struct task_struct *tsk = sad->tracer_tsk; > + > + if (tsk) { > + pid_t pid = task_tgid_nr(tsk); > + > + if (pid) { > + char comm[sizeof(tsk->comm)]; > + > + audit_log_format(ab, " tracer-pid=%d tracer-comm=", pid); > + audit_log_untrustedstring(ab, > + memcpy(comm, tsk->comm, sizeof(comm))); > + } > + } > + } > } > > /** > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index efd35b07c..47e8a9461 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -416,20 +416,13 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode) > */ > static int smk_ptrace_rule_check(struct task_struct *tracer, > struct smack_known *tracee_known, > - unsigned int mode, const char *func) > + unsigned int mode, struct smk_audit_info *saip) > { > int rc; > - struct smk_audit_info ad, *saip = NULL; > struct task_smack *tsp; > struct smack_known *tracer_known; > const struct cred *tracercred; > > - if ((mode & PTRACE_MODE_NOAUDIT) == 0) { > - smk_ad_init(&ad, func, LSM_AUDIT_DATA_TASK); > - smk_ad_setfield_u_tsk(&ad, tracer); > - saip = &ad; > - } > - > rcu_read_lock(); > tracercred = __task_cred(tracer); > tsp = smack_cred(tracercred); > @@ -480,10 +473,17 @@ static int smk_ptrace_rule_check(struct task_struct *tracer, > static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) > { > struct smack_known *skp; > + struct smk_audit_info ad, *saip = NULL; > > skp = smk_of_task_struct_obj(ctp); > + if ((mode & PTRACE_MODE_NOAUDIT) == 0) { > + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); > + smk_ad_setfield_u_tracer(&ad, current); > + smk_ad_setfield_u_tracee(&ad, ctp); > + saip = &ad; > + } > > - return smk_ptrace_rule_check(current, skp, mode, __func__); > + return smk_ptrace_rule_check(current, skp, mode, saip); > } > > /** > @@ -498,10 +498,15 @@ static int smack_ptrace_traceme(struct task_struct *ptp) > { > int rc; > struct smack_known *skp; > + struct smk_audit_info ad, *saip = NULL; > > skp = smk_of_task(smack_cred(current_cred())); > + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); > + smk_ad_setfield_u_tracer(&ad, ptp); > + smk_ad_setfield_u_tracee(&ad, current); > + saip = &ad; > > - rc = smk_ptrace_rule_check(ptp, skp, PTRACE_MODE_ATTACH, __func__); > + rc = smk_ptrace_rule_check(ptp, skp, PTRACE_MODE_ATTACH, saip); > return rc; > } > > @@ -897,15 +902,21 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm) > > if (bprm->unsafe & LSM_UNSAFE_PTRACE) { > struct task_struct *tracer; > + struct smk_audit_info ad, *saip = NULL; > rc = 0; > > + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); > + smk_ad_setfield_u_tracee(&ad, current); > + saip = &ad; > + > rcu_read_lock(); > tracer = ptrace_parent(current); > + smk_ad_setfield_u_tracer(&ad, tracer); > if (likely(tracer != NULL)) > rc = smk_ptrace_rule_check(tracer, > isp->smk_task, > PTRACE_MODE_ATTACH, > - __func__); > + saip); > rcu_read_unlock(); > > if (rc != 0)
| |