On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe")
> results in no audit messages at all if in permissive mode because the
> cache is updated during the rcu walk and thus no denial occurs on
> the subsequent ref walk. Fix this by not updating the cache when
> performing a non-blocking permission check. This only affects search
> and symlink read checks during rcu walk.
>
> Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe")
> Reported-by: BMK <bmktuwien@xxxxxxxxx>
> Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
> ---
> security/selinux/avc.c | 9 ++++++---
> security/selinux/hooks.c | 4 +++-
> security/selinux/include/avc.h | 1 +
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 635e5c1e3e48..f0e7bc0dc442 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state,
> !(avd->flags & AVD_FLAGS_PERMISSIVE))
> return -EACCES;
>
> - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver,
> - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags);
> + if (!(flags & AVC_NONBLOCKING))
> + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested,
> + driver, xperm, ssid, tsid, tclass, avd->seqno,
> + NULL, flags);
I realize the issue here is specific to the VFS code paths, but if we
are going to introduce a general AVC flag to tell the AVC code not to
block, it seems like we should honor it everywhere. I would suggest
moving the check into avc_update_node() itself (it's static, so
hopefully any perf impact will be negligible).
> return 0;
> }
>
> @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state,
> struct av_decision avd;
> int rc, rc2;
>
> - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0,
> + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested,
> + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0,
> &avd);
>
> rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7ce012d9ec51..9b05f84808d9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask)
> return PTR_ERR(isec);
>
> rc = avc_has_perm_noaudit(&selinux_state,
> - sid, isec->sid, isec->sclass, perms, 0, &avd);
> + sid, isec->sid, isec->sclass, perms,
> + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0,
> + &avd);
> audited = avc_audit_required(perms, &avd, rc,
> from_access ? FILE__AUDIT_ACCESS : 0,
> &denied);
> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
> index ef899bcfd2cb..74ea50977c20 100644
> --- a/security/selinux/include/avc.h
> +++ b/security/selinux/include/avc.h
> @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state,
>
> #define AVC_STRICT 1 /* Ignore permissive mode. */
> #define AVC_EXTENDED_PERMS 2 /* update extended permissions */
> +#define AVC_NONBLOCKING 4 /* non blocking */
> int avc_has_perm_noaudit(struct selinux_state *state,
> u32 ssid, u32 tsid,
> u16 tclass, u32 requested,
> --
> 2.19.2
>
--
paul moore
www.paul-moore.com
