| From 0188d5c025ca8fe756ba3193bd7d150139af5a88 Mon Sep 17 00:00:00 2001 |
| From: Stephen Smalley <sds@tycho.nsa.gov> |
| Date: Fri, 22 Nov 2019 12:22:45 -0500 |
| Subject: selinux: fall back to ref-walk if audit is required |
| |
| From: Stephen Smalley <sds@tycho.nsa.gov> |
| |
| commit 0188d5c025ca8fe756ba3193bd7d150139af5a88 upstream. |
| |
| commit bda0be7ad994 ("security: make inode_follow_link RCU-walk aware") |
| passed down the rcu flag to the SELinux AVC, but failed to adjust the |
| test in slow_avc_audit() to also return -ECHILD on LSM_AUDIT_DATA_DENTRY. |
| Previously, we only returned -ECHILD if generating an audit record with |
| LSM_AUDIT_DATA_INODE since this was only relevant from inode_permission. |
| Move the handling of MAY_NOT_BLOCK to avc_audit() and its inlined |
| equivalent in selinux_inode_permission() immediately after we determine |
| that audit is required, and always fall back to ref-walk in this case. |
| |
| Fixes: bda0be7ad994 ("security: make inode_follow_link RCU-walk aware") |
| Reported-by: Will Deacon <will@kernel.org> |
| Suggested-by: Al Viro <viro@zeniv.linux.org.uk> |
| Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> |
| Signed-off-by: Paul Moore <paul@paul-moore.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| security/selinux/avc.c | 24 +++++------------------- |
| security/selinux/hooks.c | 11 +++++++---- |
| security/selinux/include/avc.h | 8 +++++--- |
| 3 files changed, 17 insertions(+), 26 deletions(-) |
| |
| --- a/security/selinux/avc.c |
| +++ b/security/selinux/avc.c |
| @@ -424,7 +424,7 @@ static inline int avc_xperms_audit(struc |
| if (likely(!audited)) |
| return 0; |
| return slow_avc_audit(state, ssid, tsid, tclass, requested, |
| - audited, denied, result, ad, 0); |
| + audited, denied, result, ad); |
| } |
| |
| static void avc_node_free(struct rcu_head *rhead) |
| @@ -758,8 +758,7 @@ static void avc_audit_post_callback(stru |
| noinline int slow_avc_audit(struct selinux_state *state, |
| u32 ssid, u32 tsid, u16 tclass, |
| u32 requested, u32 audited, u32 denied, int result, |
| - struct common_audit_data *a, |
| - unsigned int flags) |
| + struct common_audit_data *a) |
| { |
| struct common_audit_data stack_data; |
| struct selinux_audit_data sad; |
| @@ -772,17 +771,6 @@ noinline int slow_avc_audit(struct selin |
| a->type = LSM_AUDIT_DATA_NONE; |
| } |
| |
| - /* |
| - * When in a RCU walk do the audit on the RCU retry. This is because |
| - * the collection of the dname in an inode audit message is not RCU |
| - * safe. Note this may drop some audits when the situation changes |
| - * during retry. However this is logically just as if the operation |
| - * happened a little later. |
| - */ |
| - if ((a->type == LSM_AUDIT_DATA_INODE) && |
| - (flags & MAY_NOT_BLOCK)) |
| - return -ECHILD; |
| - |
| sad.tclass = tclass; |
| sad.requested = requested; |
| sad.ssid = ssid; |
| @@ -855,16 +843,14 @@ static int avc_update_node(struct selinu |
| /* |
| * If we are in a non-blocking code path, e.g. VFS RCU walk, |
| * then we must not add permissions to a cache entry |
| - * because we cannot safely audit the denial. Otherwise, |
| + * because we will not audit the denial. Otherwise, |
| * during the subsequent blocking retry (e.g. VFS ref walk), we |
| * will find the permissions already granted in the cache entry |
| * and won't audit anything at all, leading to silent denials in |
| * permissive mode that only appear when in enforcing mode. |
| * |
| - * See the corresponding handling in slow_avc_audit(), and the |
| - * logic in selinux_inode_follow_link and selinux_inode_permission |
| - * for the VFS MAY_NOT_BLOCK flag, which is transliterated into |
| - * AVC_NONBLOCKING for avc_has_perm_noaudit(). |
| + * See the corresponding handling of MAY_NOT_BLOCK in avc_audit() |
| + * and selinux_inode_permission(). |
| */ |
| if (flags & AVC_NONBLOCKING) |
| return 0; |
| --- a/security/selinux/hooks.c |
| +++ b/security/selinux/hooks.c |
| @@ -3023,8 +3023,7 @@ static int selinux_inode_follow_link(str |
| |
| static noinline int audit_inode_permission(struct inode *inode, |
| u32 perms, u32 audited, u32 denied, |
| - int result, |
| - unsigned flags) |
| + int result) |
| { |
| struct common_audit_data ad; |
| struct inode_security_struct *isec = selinux_inode(inode); |
| @@ -3035,7 +3034,7 @@ static noinline int audit_inode_permissi |
| |
| rc = slow_avc_audit(&selinux_state, |
| current_sid(), isec->sid, isec->sclass, perms, |
| - audited, denied, result, &ad, flags); |
| + audited, denied, result, &ad); |
| if (rc) |
| return rc; |
| return 0; |
| @@ -3082,7 +3081,11 @@ static int selinux_inode_permission(stru |
| if (likely(!audited)) |
| return rc; |
| |
| - rc2 = audit_inode_permission(inode, perms, audited, denied, rc, flags); |
| + /* fall back to ref-walk if we have to generate audit */ |
| + if (flags & MAY_NOT_BLOCK) |
| + return -ECHILD; |
| + |
| + rc2 = audit_inode_permission(inode, perms, audited, denied, rc); |
| if (rc2) |
| return rc2; |
| return rc; |
| --- a/security/selinux/include/avc.h |
| +++ b/security/selinux/include/avc.h |
| @@ -100,8 +100,7 @@ static inline u32 avc_audit_required(u32 |
| int slow_avc_audit(struct selinux_state *state, |
| u32 ssid, u32 tsid, u16 tclass, |
| u32 requested, u32 audited, u32 denied, int result, |
| - struct common_audit_data *a, |
| - unsigned flags); |
| + struct common_audit_data *a); |
| |
| /** |
| * avc_audit - Audit the granting or denial of permissions. |
| @@ -135,9 +134,12 @@ static inline int avc_audit(struct selin |
| audited = avc_audit_required(requested, avd, result, 0, &denied); |
| if (likely(!audited)) |
| return 0; |
| + /* fall back to ref-walk if we have to generate audit */ |
| + if (flags & MAY_NOT_BLOCK) |
| + return -ECHILD; |
| return slow_avc_audit(state, ssid, tsid, tclass, |
| requested, audited, denied, result, |
| - a, flags); |
| + a); |
| } |
| |
| #define AVC_STRICT 1 /* Ignore permissive mode. */ |