Re: [PATCH v4 27/42] selinux: introduce task_obj_perm()
From: Simon Horman <horms@kernel.org>
Date: 2025-06-13 14:15:28
Also in:
selinux
On Tue, Jun 10, 2025 at 01:21:58PM -0400, Stephen Smalley wrote:
Introduce task_obj_perm() for namespace-aware permission checking between two tasks using the objective SID for both tasks and without assuming that either task is current. Convert the permission checks of this form in the hook functions to use this new helper. Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
...
+int task_obj_has_perm(const struct task_struct *s,
+ const struct task_struct *t,
+ u16 tclass, u32 requested,
+ struct common_audit_data *ad)
+{
+ const struct cred *cred;
+ const struct task_security_struct *tsec;
+ struct selinux_state *state;
+ u32 ssid;
+ u32 tsid;
+ int rc;
+
+ state = current_selinux_state;
+ rcu_read_lock();
+ tsec = task_security(s);
+ if (tsec)
+ ssid = tsec->sid;
+ else
+ ssid = SECINITSID_UNLABELED;Hi Stephen, Above it is assumed that tsec may be NULL...
+ rcu_read_unlock();
+
+ do {
+ tsid = task_sid_obj_for_state(t, state);
+
+ rc = avc_has_perm(state, ssid, tsid, tclass, requested, ad);
+ if (rc)
+ return rc;
+
+ cred = tsec->parent_cred;... but here tsec is dereferenced without explicitly checking if it is not NULL. I'm wondering if this is safe, e.g. due to the call to avc_has_perm(). Flagged by Smatch.
+ if (!cred) + break; + + rcu_read_lock(); + tsec = selinux_cred(cred); + ssid = tsec->sid; + state = tsec->state; + rcu_read_unlock(); + } while (cred); + + return 0; +}
...