diff mbox

[GIT,PULL,SECURITY] selinux: fix off-by-one in setprocattr

Message ID alpine.LRH.2.20.1702081835250.9813@namei.org (mailing list archive)
State New, archived
Headers show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus

Commit Message

James Morris Feb. 8, 2017, 7:37 a.m. UTC
Please pull this fix for a bug in SELinux, which fixes CVE-2017-2618.

The following changes since commit 926af6273fc683cd98cd0ce7bf0d04a02eed6742:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-02-07 12:10:57 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus

Stephen Smalley (1):
      selinux: fix off-by-one in setprocattr

 security/selinux/hooks.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

---
commit 0c461cb727d146c9ef2d3e86214f498b78b7d125
Author: Stephen Smalley <sds@tycho.nsa.gov>
Date:   Tue Jan 31 11:54:04 2017 -0500

    selinux: fix off-by-one in setprocattr
    
    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>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c7c6619..d98550a 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--;