summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephen Smalley <sds@tycho.nsa.gov>2017-01-31 11:54:04 -0500
committerSasha Levin <alexander.levin@verizon.com>2017-03-06 17:31:13 -0500
commit96e74ad7ac38ca330d16222e6da38c9a196deb40 (patch)
tree265e1c3638537b8596e110f911bbd722dc3a578d
parent2808bb2adc213d36185cbd7ce9de4c39c56c6b55 (diff)
selinux: fix off-by-one in setprocattr
[ Upstream commit a050a570db0190164e7250013214e29a5a9803ee ] SELinux tries to support setting/clearing of /proc/pid/attr attributes from the shell by ignoring terminating newlines and treating an attribute value that begins with a NUL or newline as an attempt to clear the attribute. However, the test for clearing attributes has always been wrong; it has an off-by-one error, and this could further lead to reading past the end of the allocated buffer since commit bb646cdb12e75d82258c2f2e7746d5952d3e321a ("proc_pid_attr_write(): switch to memdup_user()"). Fix the off-by-one error. Even with this fix, setting and clearing /proc/pid/attr attributes from the shell is not straightforward since the interface does not support multiple write() calls (so shells that write the value and newline separately will set and then immediately clear the attribute, requiring use of echo -n to set the attribute), whereas trying to use echo -n "" to clear the attribute causes the shell to skip the write() call altogether since POSIX says that a zero-length write causes no side effects. Thus, one must use echo -n to set and echo without -n to clear, as in the following example: $ echo -n unconfined_u:object_r:user_home_t:s0 > /proc/$$/attr/fscreate $ cat /proc/$$/attr/fscreate unconfined_u:object_r:user_home_t:s0 $ echo "" > /proc/$$/attr/fscreate $ cat /proc/$$/attr/fscreate Note the use of /proc/$$ rather than /proc/self, as otherwise the cat command will read its own attribute value, not that of the shell. There are no users of this facility to my knowledge; possibly we should just get rid of it. UPDATE: Upon further investigation it appears that a local process with the process:setfscreate permission can cause a kernel panic as a result of this bug. This patch fixes CVE-2017-2618. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> [PM: added the update about CVE-2017-2618 to the commit description] Cc: stable@vger.kernel.org # 3.5: d6ea83ec6864e Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
-rw-r--r--security/selinux/hooks.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 280235cc3a98..0034eb420b0e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5626,7 +5626,7 @@ static int selinux_setprocattr(struct task_struct *p,
return error;
/* Obtain a SID for the context, if one was specified. */
- if (size && str[1] && str[1] != '\n') {
+ if (size && str[0] && str[0] != '\n') {
if (str[size-1] == '\n') {
str[size-1] = 0;
size--;