commit: e3fea3f70fd68af0574a5f24246cdb4ed07f2b74
From: Al Viro <[email protected]>
Date: Sat, 9 Jun 2012 08:15:16 +0100
Subject: 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: [email protected]
Signed-off-by: Al Viro <[email protected]>
Tested-by: Dave Jones <[email protected]>
Reported-by: Dave Jones <[email protected]>
Signed-off-by: James Morris <[email protected]>
---
security/selinux/hooks.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 94c45a1..79690f4 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);
--
1.7.3.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
