| From 0c461cb727d146c9ef2d3e86214f498b78b7d125 Mon Sep 17 00:00:00 2001 |
| From: Stephen Smalley <sds@tycho.nsa.gov> |
| Date: Tue, 31 Jan 2017 11:54:04 -0500 |
| Subject: [PATCH] selinux: fix off-by-one in setprocattr |
| |
| commit 0c461cb727d146c9ef2d3e86214f498b78b7d125 upstream. |
| |
| 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: James Morris <james.l.morris@oracle.com> |
| |
| diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c |
| index c7c6619431d5..d98550abe16d 100644 |
| --- a/security/selinux/hooks.c |
| +++ b/security/selinux/hooks.c |
| @@ -5887,7 +5887,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--; |
| -- |
| 2.12.0 |
| |