diff mbox

smack: parse mnt opts after privileges check

Message ID 1478774992-13222-1-git-send-email-himanshu.sh@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Himanshu Shukla Nov. 10, 2016, 10:49 a.m. UTC
In smack_set_mnt_opts()first the SMACK mount options are being
parsed and later it is being checked whether the user calling
mount has CAP_MAC_ADMIN capability.
This sequence of operationis will allow unauthorized user to add
SMACK labels in label list and may cause denial of security attack
by adding many labels by allocating kernel memory by unauthorized user.
Superblock smack flag is also being set as initialized though function
may return with EPERM error.
First check the capability of calling user then set the SMACK attributes
and smk_flags.

Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com>
---
 security/smack/smack_lsm.c | 50 +++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

Comments

Casey Schaufler Nov. 10, 2016, 7:23 p.m. UTC | #1
On 11/10/2016 2:49 AM, Himanshu Shukla wrote:
> In smack_set_mnt_opts()first the SMACK mount options are being
> parsed and later it is being checked whether the user calling
> mount has CAP_MAC_ADMIN capability.
> This sequence of operationis will allow unauthorized user to add
> SMACK labels in label list and may cause denial of security attack
> by adding many labels by allocating kernel memory by unauthorized user.
> Superblock smack flag is also being set as initialized though function
> may return with EPERM error.
> First check the capability of calling user then set the SMACK attributes
> and smk_flags.
>
> Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Applied to git://github.com/cschaufler/smack-next.git#smack-for-4.10

> ---
>  security/smack/smack_lsm.c | 50 +++++++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 1cb0602..a793872 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -769,6 +769,31 @@ static int smack_set_mnt_opts(struct super_block *sb,
>  	if (sp->smk_flags & SMK_SB_INITIALIZED)
>  		return 0;
>  
> +	if (!smack_privileged(CAP_MAC_ADMIN)) {
> +		/*
> +		 * Unprivileged mounts don't get to specify Smack values.
> +		 */
> +		if (num_opts)
> +			return -EPERM;
> +		/*
> +		 * Unprivileged mounts get root and default from the caller.
> +		 */
> +		skp = smk_of_current();
> +		sp->smk_root = skp;
> +		sp->smk_default = skp;
> +		/*
> +		 * For a handful of fs types with no user-controlled
> +		 * backing store it's okay to trust security labels
> +		 * in the filesystem. The rest are untrusted.
> +		 */
> +		if (sb->s_user_ns != &init_user_ns &&
> +		    sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC &&
> +		    sb->s_magic != RAMFS_MAGIC) {
> +			transmute = 1;
> +			sp->smk_flags |= SMK_SB_UNTRUSTED;
> +		}
> +	}
> +
>  	sp->smk_flags |= SMK_SB_INITIALIZED;
>  
>  	for (i = 0; i < num_opts; i++) {
> @@ -809,31 +834,6 @@ static int smack_set_mnt_opts(struct super_block *sb,
>  		}
>  	}
>  
> -	if (!smack_privileged(CAP_MAC_ADMIN)) {
> -		/*
> -		 * Unprivileged mounts don't get to specify Smack values.
> -		 */
> -		if (num_opts)
> -			return -EPERM;
> -		/*
> -		 * Unprivileged mounts get root and default from the caller.
> -		 */
> -		skp = smk_of_current();
> -		sp->smk_root = skp;
> -		sp->smk_default = skp;
> -		/*
> -		 * For a handful of fs types with no user-controlled
> -		 * backing store it's okay to trust security labels
> -		 * in the filesystem. The rest are untrusted.
> -		 */
> -		if (sb->s_user_ns != &init_user_ns &&
> -		    sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC &&
> -		    sb->s_magic != RAMFS_MAGIC) {
> -			transmute = 1;
> -			sp->smk_flags |= SMK_SB_UNTRUSTED;
> -		}
> -	}
> -
>  	/*
>  	 * Initialize the root inode.
>  	 */

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 1cb0602..a793872 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -769,6 +769,31 @@  static int smack_set_mnt_opts(struct super_block *sb,
 	if (sp->smk_flags & SMK_SB_INITIALIZED)
 		return 0;
 
+	if (!smack_privileged(CAP_MAC_ADMIN)) {
+		/*
+		 * Unprivileged mounts don't get to specify Smack values.
+		 */
+		if (num_opts)
+			return -EPERM;
+		/*
+		 * Unprivileged mounts get root and default from the caller.
+		 */
+		skp = smk_of_current();
+		sp->smk_root = skp;
+		sp->smk_default = skp;
+		/*
+		 * For a handful of fs types with no user-controlled
+		 * backing store it's okay to trust security labels
+		 * in the filesystem. The rest are untrusted.
+		 */
+		if (sb->s_user_ns != &init_user_ns &&
+		    sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC &&
+		    sb->s_magic != RAMFS_MAGIC) {
+			transmute = 1;
+			sp->smk_flags |= SMK_SB_UNTRUSTED;
+		}
+	}
+
 	sp->smk_flags |= SMK_SB_INITIALIZED;
 
 	for (i = 0; i < num_opts; i++) {
@@ -809,31 +834,6 @@  static int smack_set_mnt_opts(struct super_block *sb,
 		}
 	}
 
-	if (!smack_privileged(CAP_MAC_ADMIN)) {
-		/*
-		 * Unprivileged mounts don't get to specify Smack values.
-		 */
-		if (num_opts)
-			return -EPERM;
-		/*
-		 * Unprivileged mounts get root and default from the caller.
-		 */
-		skp = smk_of_current();
-		sp->smk_root = skp;
-		sp->smk_default = skp;
-		/*
-		 * For a handful of fs types with no user-controlled
-		 * backing store it's okay to trust security labels
-		 * in the filesystem. The rest are untrusted.
-		 */
-		if (sb->s_user_ns != &init_user_ns &&
-		    sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC &&
-		    sb->s_magic != RAMFS_MAGIC) {
-			transmute = 1;
-			sp->smk_flags |= SMK_SB_UNTRUSTED;
-		}
-	}
-
 	/*
 	 * Initialize the root inode.
 	 */