From 3b9fc37280c521b086943f9aedda767f5bf3b2d3 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Thu, 26 Jul 2012 14:47:11 -0700 Subject: [PATCH 1/3] smack: off by one error Consider the input case of a rule that consists entirely of non space symbols followed by a \0. Say 64 + \0 In this case strlen(data) = 64 kzalloc of subject and object are 64 byte objects sscanfdata, "%s %s %s", subject, ...) will put 65 bytes into subject. Signed-off-by: Alan Cox Acked-by: Casey Schaufler Cc: stable@vger.kernel.org Signed-off-by: James Morris --- security/smack/smackfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index d31e6d957c21..b1b768e4049a 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -323,11 +323,11 @@ static int smk_parse_long_rule(const char *data, struct smack_rule *rule, int datalen; int rc = -1; - /* - * This is probably inefficient, but safe. - */ + /* This is inefficient */ datalen = strlen(data); - subject = kzalloc(datalen, GFP_KERNEL); + + /* Our first element can be 64 + \0 with no spaces */ + subject = kzalloc(datalen + 1, GFP_KERNEL); if (subject == NULL) return -1; object = kzalloc(datalen, GFP_KERNEL); From 5935e6dcaaa8f666dd7f1169fa87d36752ebeb94 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 23 Jul 2012 15:37:31 +0100 Subject: [PATCH 2/3] KEYS: linux/key-type.h needs linux/errno.h linux/key-type.h needs to #include linux/errno.h as it refers to ENOKEY. Without this, with sparc's allmodconfig in one of my test trees, the following error occurs: include/linux/key-type.h: In function 'key_negate_and_link': include/linux/key-type.h:122:43: error: 'ENOKEY' undeclared (first use in this function) include/linux/key-type.h:122:43: note: each undeclared identifier is reported only once for each fun Reported-by: Fengguang Wu Signed-off-by: David Howells Signed-off-by: James Morris --- include/linux/key-type.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/key-type.h b/include/linux/key-type.h index 39e3c082c49d..f0c651cda7b0 100644 --- a/include/linux/key-type.h +++ b/include/linux/key-type.h @@ -13,6 +13,7 @@ #define _LINUX_KEY_TYPE_H #include +#include #ifdef CONFIG_KEYS From e3fea3f70fd68af0574a5f24246cdb4ed07f2b74 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 9 Jun 2012 08:15:16 +0100 Subject: [PATCH 3/3] selinux: fix selinux_inode_setxattr oops OK, what we have so far is e.g. setxattr(path, name, whatever, 0, XATTR_REPLACE) with name being good enough to get through xattr_permission(). Then we reach security_inode_setxattr() with the desired value and size. Aha. name should begin with "security.selinux", or we won't get that far in selinux_inode_setxattr(). Suppose we got there and have enough permissions to relabel that sucker. We call security_context_to_sid() with value == NULL, size == 0. OK, we want ss_initialized to be non-zero. I.e. after everything had been set up and running. No problem... We do 1-byte kmalloc(), zero-length memcpy() (which doesn't oops, even thought the source is NULL) and put a NUL there. I.e. form an empty string. string_to_context_struct() is called and looks for the first ':' in there. Not found, -EINVAL we get. OK, security_context_to_sid_core() has rc == -EINVAL, force == 0, so it silently returns -EINVAL. All it takes now is not having CAP_MAC_ADMIN and we are fucked. All right, it might be a different bug (modulo strange code quoted in the report), but it's real. Easily fixed, AFAICS: Deal with size == 0, value == NULL case in selinux_inode_setxattr() Cc: stable@vger.kernel.org Signed-off-by: Al Viro Tested-by: Dave Jones Reported-by: Dave Jones Signed-off-by: James Morris --- security/selinux/hooks.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 94c45a1531a4..79690f401a58 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2791,11 +2791,16 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, /* We strip a nul only if it is at the end, otherwise the * context contains a nul and we should audit that */ - str = value; - if (str[size - 1] == '\0') - audit_size = size - 1; - else - audit_size = size; + if (value) { + str = value; + if (str[size - 1] == '\0') + audit_size = size - 1; + else + audit_size = size; + } else { + str = ""; + audit_size = 0; + } ab = audit_log_start(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR); audit_log_format(ab, "op=setxattr invalid_context="); audit_log_n_untrustedstring(ab, value, audit_size);