[PATCH] Smack: fix bug with empty label causing memory read beyond range

[Rafal Krypa]

Setting zero-length Smack label on a file crashes the kernel.
The following command:
 # setfattr -n security.SMACK64 /dev/null

causes kernel panic. Call Trace:
 [<601a3b1e>] smk_parse_smack+0x1e/0xb2
 [<601a3cb0>] smk_import_entry+0x16/0x180
 [<601a1d0f>] smack_inode_setxattr+0x1ac/0x269
 [<6001ea92>] ? strncpy_chunk_from_user+0x0/0x56
 [<6001ecdb>] ? do_op_one_page+0x142/0x1e9
 [<6019e88f>] security_inode_setxattr+0x1e/0x23
 [<600e23e9>] vfs_setxattr+0x79/0xbd
 [<600e2579>] setxattr+0x14c/0x1a2
 [<600e27c3>] SyS_setxattr+0x7e/0xd1
 [<60043fad>] ? ptrace_notify+0xbc/0xc3
 [<6001ea26>] handle_syscall+0x65/0x7c
 [<60031c10>] userspace+0x441/0x547
 [<6001b9b3>] ? interrupt_end+0x0/0x80
 [<6001ea69>] ? copy_chunk_to_user+0x0/0x29
 [<6002d98f>] ? save_registers+0x1f/0x39
 [<6003488e>] ? arch_prctl+0xf9/0x173
 [<6001b867>] fork_handler+0x85/0x87

It is caused by passing len=0 to function smk_parse_smack(). In such case
the function assumes that the string to parse is null-terminated and tries
to calculate it's length by strlen(). But the string in this case is empty
and not null-terminated.

During investigation of all other calls leading to smk_parse_smack()
another similar issue was found in smk_write_onlycap().

This patch fixes both cases by ensuring that smk_parse_smack() by handling
of zero-length labels.

Signed-off-by: Rafal Krypa <r.krypa@xxxxxxxxxxx>
---
 security/smack/smack_lsm.c | 9 ++++++---
 security/smack/smackfs.c   | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 14f52be..be8651d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -800,8 +800,8 @@ static int smack_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
  * smack_inode_setxattr - Smack check for setting xattrs
  * @dentry: the object
  * @name: name of the attribute
- * @value: unused
- * @size: unused
+ * @value: attribute value
+ * @size: attribute size
  * @flags: unused
  *
  * This protects the Smack attribute explicitly.
@@ -842,6 +842,9 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name,
 	if (check_priv && !smack_privileged(CAP_MAC_ADMIN))
 		rc = -EPERM;
 
+	if (value == NULL || size > SMK_LONGLABEL || size == 0)
+		return -EINVAL;
+
 	if (rc == 0 && check_import) {
 		skp = smk_import_entry(value, size);
 		if (skp == NULL || (check_star &&
@@ -2076,7 +2079,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	int rc = 0;
 
 	if (value == NULL || size > SMK_LONGLABEL || size == 0)
-		return -EACCES;
+		return -EINVAL;
 
 	skp = smk_import_entry(value, size);
 	if (skp == NULL)
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 3198cfe..c29e8ee 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1667,7 +1667,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 	if (smack_onlycap != NULL && smack_onlycap != skp)
 		return -EPERM;
 
-	data = kzalloc(count, GFP_KERNEL);
+	data = kzalloc(count + 1, GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/