diff mbox series

[2/2] selinux: stop passing MAY_NOT_BLOCK to the AVC upon follow_link

Message ID 20181212151056.2938-2-sds@tycho.nsa.gov (mailing list archive)
State Accepted
Headers show
Series [1/2] selinux: avoid silent denials in permissive mode under RCU walk | expand

Commit Message

Stephen Smalley Dec. 12, 2018, 3:10 p.m. UTC
commit bda0be7ad9948 ("security: make inode_follow_link RCU-walk aware")
switched selinux_inode_follow_link() to use avc_has_perm_flags() and
pass down the MAY_NOT_BLOCK flag if called during RCU walk.  However,
the only test of MAY_NOT_BLOCK occurs during slow_avc_audit()
and only if passing an inode as audit data (LSM_AUDIT_DATA_INODE).  Since
selinux_inode_follow_link() passes a dentry directly, passing MAY_NOT_BLOCK
here serves no purpose.  Switch selinux_inode_follow_link() to use
avc_has_perm() and drop avc_has_perm_flags() since there are no other
users.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 security/selinux/avc.c         | 24 ++----------------------
 security/selinux/hooks.c       |  5 ++---
 security/selinux/include/avc.h |  5 -----
 3 files changed, 4 insertions(+), 30 deletions(-)

Comments

Paul Moore Jan. 11, 2019, 1:37 a.m. UTC | #1
On Wed, Dec 12, 2018 at 10:08 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> commit bda0be7ad9948 ("security: make inode_follow_link RCU-walk aware")
> switched selinux_inode_follow_link() to use avc_has_perm_flags() and
> pass down the MAY_NOT_BLOCK flag if called during RCU walk.  However,
> the only test of MAY_NOT_BLOCK occurs during slow_avc_audit()
> and only if passing an inode as audit data (LSM_AUDIT_DATA_INODE).  Since
> selinux_inode_follow_link() passes a dentry directly, passing MAY_NOT_BLOCK
> here serves no purpose.  Switch selinux_inode_follow_link() to use
> avc_has_perm() and drop avc_has_perm_flags() since there are no other
> users.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  security/selinux/avc.c         | 24 ++----------------------
>  security/selinux/hooks.c       |  5 ++---
>  security/selinux/include/avc.h |  5 -----
>  3 files changed, 4 insertions(+), 30 deletions(-)

I just merged both 1/2 and 2/2 into selinux/next, thanks Stephen.

> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 5de18a6d5c3f..9b63d8ee1687 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -867,9 +867,8 @@ static int avc_update_node(struct selinux_avc *avc,
>          * 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().
> +        * logic in selinux_inode_permission for the MAY_NOT_BLOCK flag,
> +        * which is transliterated into AVC_NONBLOCKING.
>          */
>         if (flags & AVC_NONBLOCKING)
>                 return 0;
> @@ -1209,25 +1208,6 @@ int avc_has_perm(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass,
>         return rc;
>  }
>
> -int avc_has_perm_flags(struct selinux_state *state,
> -                      u32 ssid, u32 tsid, u16 tclass, u32 requested,
> -                      struct common_audit_data *auditdata,
> -                      int flags)
> -{
> -       struct av_decision avd;
> -       int rc, rc2;
> -
> -       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,
> -                       auditdata, flags);
> -       if (rc2)
> -               return rc2;
> -       return rc;
> -}
> -
>  u32 avc_policy_seqno(struct selinux_state *state)
>  {
>         return state->avc->avc_cache.latest_notif;
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9b05f84808d9..f012d2eb159e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3139,9 +3139,8 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct inode *inode,
>         if (IS_ERR(isec))
>                 return PTR_ERR(isec);
>
> -       return avc_has_perm_flags(&selinux_state,
> -                                 sid, isec->sid, isec->sclass, FILE__READ, &ad,
> -                                 rcu ? MAY_NOT_BLOCK : 0);
> +       return avc_has_perm(&selinux_state,
> +                           sid, isec->sid, isec->sclass, FILE__READ, &ad);
>  }
>
>  static noinline int audit_inode_permission(struct inode *inode,
> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
> index 74ea50977c20..7be0e1e90e8b 100644
> --- a/security/selinux/include/avc.h
> +++ b/security/selinux/include/avc.h
> @@ -153,11 +153,6 @@ int avc_has_perm(struct selinux_state *state,
>                  u32 ssid, u32 tsid,
>                  u16 tclass, u32 requested,
>                  struct common_audit_data *auditdata);
> -int avc_has_perm_flags(struct selinux_state *state,
> -                      u32 ssid, u32 tsid,
> -                      u16 tclass, u32 requested,
> -                      struct common_audit_data *auditdata,
> -                      int flags);
>
>  int avc_has_extended_perms(struct selinux_state *state,
>                            u32 ssid, u32 tsid, u16 tclass, u32 requested,
> --
> 2.19.2
>
diff mbox series

Patch

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 5de18a6d5c3f..9b63d8ee1687 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -867,9 +867,8 @@  static int avc_update_node(struct selinux_avc *avc,
 	 * 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().
+	 * logic in selinux_inode_permission for the MAY_NOT_BLOCK flag,
+	 * which is transliterated into AVC_NONBLOCKING.
 	 */
 	if (flags & AVC_NONBLOCKING)
 		return 0;
@@ -1209,25 +1208,6 @@  int avc_has_perm(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass,
 	return rc;
 }
 
-int avc_has_perm_flags(struct selinux_state *state,
-		       u32 ssid, u32 tsid, u16 tclass, u32 requested,
-		       struct common_audit_data *auditdata,
-		       int flags)
-{
-	struct av_decision avd;
-	int rc, rc2;
-
-	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,
-			auditdata, flags);
-	if (rc2)
-		return rc2;
-	return rc;
-}
-
 u32 avc_policy_seqno(struct selinux_state *state)
 {
 	return state->avc->avc_cache.latest_notif;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9b05f84808d9..f012d2eb159e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3139,9 +3139,8 @@  static int selinux_inode_follow_link(struct dentry *dentry, struct inode *inode,
 	if (IS_ERR(isec))
 		return PTR_ERR(isec);
 
-	return avc_has_perm_flags(&selinux_state,
-				  sid, isec->sid, isec->sclass, FILE__READ, &ad,
-				  rcu ? MAY_NOT_BLOCK : 0);
+	return avc_has_perm(&selinux_state,
+			    sid, isec->sid, isec->sclass, FILE__READ, &ad);
 }
 
 static noinline int audit_inode_permission(struct inode *inode,
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index 74ea50977c20..7be0e1e90e8b 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -153,11 +153,6 @@  int avc_has_perm(struct selinux_state *state,
 		 u32 ssid, u32 tsid,
 		 u16 tclass, u32 requested,
 		 struct common_audit_data *auditdata);
-int avc_has_perm_flags(struct selinux_state *state,
-		       u32 ssid, u32 tsid,
-		       u16 tclass, u32 requested,
-		       struct common_audit_data *auditdata,
-		       int flags);
 
 int avc_has_extended_perms(struct selinux_state *state,
 			   u32 ssid, u32 tsid, u16 tclass, u32 requested,