Commit 312b4e22 authored by Ryan Mallon's avatar Ryan Mallon Committed by Linus Torvalds
Browse files

vsprintf: check real user/group id for %pK 



Some setuid binaries will allow reading of files which have read
permission by the real user id.  This is problematic with files which
use %pK because the file access permission is checked at open() time,
but the kptr_restrict setting is checked at read() time.  If a setuid
binary opens a %pK file as an unprivileged user, and then elevates
permissions before reading the file, then kernel pointer values may be
leaked.

This happens for example with the setuid pppd application on Ubuntu 12.04:

  $ head -1 /proc/kallsyms
  00000000 T startup_32

  $ pppd file /proc/kallsyms
  pppd: In file /proc/kallsyms: unrecognized option 'c1000000'

This will only leak the pointer value from the first line, but other
setuid binaries may leak more information.

Fix this by adding a check that in addition to the current process having
CAP_SYSLOG, that effective user and group ids are equal to the real ids.
If a setuid binary reads the contents of a file which uses %pK then the
pointer values will be printed as NULL if the real user is unprivileged.

Update the sysctl documentation to reflect the changes, and also correct
the documentation to state the kptr_restrict=0 is the default.

This is a only temporary solution to the issue.  The correct solution is
to do the permission check at open() time on files, and to replace %pK
with a function which checks the open() time permission.  %pK uses in
printk should be removed since no sane permission check can be done, and
instead protected by using dmesg_restrict.

Signed-off-by: default avatarRyan Mallon <rmallon@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Joe Perches <joe@perches.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 27083bac

Documentation/sysctl/kernel.txt

+18 −7
Original line number Diff line number Diff line
@@ -290,13 +290,24 @@ Default value is "/sbin/hotplug". 
kptr_restrict:



This toggle indicates whether restrictions are placed on

exposing kernel addresses via /proc and other interfaces.  When

kptr_restrict is set to (0), there are no restrictions.  When

kptr_restrict is set to (1), the default, kernel pointers

printed using the %pK format specifier will be replaced with 0's

unless the user has CAP_SYSLOG.  When kptr_restrict is set to

(2), kernel pointers printed using %pK will be replaced with 0's

regardless of privileges.

exposing kernel addresses via /proc and other interfaces.



When kptr_restrict is set to (0), the default, there are no restrictions.



When kptr_restrict is set to (1), kernel pointers printed using the %pK

format specifier will be replaced with 0's unless the user has CAP_SYSLOG

and effective user and group ids are equal to the real ids. This is

because %pK checks are done at read() time rather than open() time, so

if permissions are elevated between the open() and the read() (e.g via

a setuid binary) then %pK will not leak kernel pointers to unprivileged

users. Note, this is a temporary solution only. The correct long-term

solution is to do the permission checks at open() time. Consider removing

world read permissions from files that use %pK, and using dmesg_restrict

to protect against uses of %pK in dmesg(8) if leaking kernel pointer

values to unprivileged users is a concern.



When kptr_restrict is set to (2), kernel pointers printed using

%pK will be replaced with 0's regardless of privileges.



==============================================================

lib/vsprintf.c

+30 −3
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ 
#include <linux/uaccess.h>

#include <linux/ioport.h>

#include <linux/dcache.h>

#include <linux/cred.h>

#include <net/addrconf.h>



#include <asm/page.h>		/* for PAGE_SIZE */
@@ -1312,11 +1313,37 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, 
				spec.field_width = default_width;

			return string(buf, end, "pK-error", spec);

		}

		if (!((kptr_restrict == 0) ||

		      (kptr_restrict == 1 &&

		       has_capability_noaudit(current, CAP_SYSLOG))))



		switch (kptr_restrict) {

		case 0:

			/* Always print %pK values */

			break;

		case 1: {

			/*

			 * Only print the real pointer value if the current

			 * process has CAP_SYSLOG and is running with the

			 * same credentials it started with. This is because

			 * access to files is checked at open() time, but %pK

			 * checks permission at read() time. We don't want to

			 * leak pointer values if a binary opens a file using

			 * %pK and then elevates privileges before reading it.

			 */

			const struct cred *cred = current_cred();



			if (!has_capability_noaudit(current, CAP_SYSLOG) ||

			    !uid_eq(cred->euid, cred->uid) ||

			    !gid_eq(cred->egid, cred->gid))

				ptr = NULL;

			break;

		}

		case 2:

		default:

			/* Always print 0's for %pK */

			ptr = NULL;

			break;

		}

		break;



	case 'N':

		switch (fmt[1]) {

		case 'F':

CRA Git | Maintained and supported by SUSTech CRA and CCSE