Subject: + vsprintf-check-real-user-group-id-for-%pk.patch added to -mm tree
To: rmallon@xxxxxxxxx,ebiederm@xxxxxxxxxxxx,joe@xxxxxxxxxxx,keescook@xxxxxxxxxxxx,stable@xxxxxxxxxxxxxxx,viro@xxxxxxxxxxxxxxxxxx
From: akpm@xxxxxxxxxxxxxxxxxxxx
Date: Tue, 15 Oct 2013 13:51:20 -0700
The patch titled
Subject: vsprintf: check real user/group id for %pK
has been added to the -mm tree. Its filename is
vsprintf-check-real-user-group-id-for-%pk.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/vsprintf-check-real-user-group-id-for-%pk.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/vsprintf-check-real-user-group-id-for-%pk.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/SubmitChecklist when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Ryan Mallon <rmallon@xxxxxxxxx>
Subject: 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: Ryan Mallon <rmallon@xxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Joe Perches <joe@xxxxxxxxxxx>
Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
Documentation/sysctl/kernel.txt | 25 ++++++++++++++++------
lib/vsprintf.c | 33 +++++++++++++++++++++++++++---
2 files changed, 48 insertions(+), 10 deletions(-)
diff -puN Documentation/sysctl/kernel.txt~vsprintf-check-real-user-group-id-for-%pk Documentation/sysctl/kernel.txt
--- a/Documentation/sysctl/kernel.txt~vsprintf-check-real-user-group-id-for-%pk
+++ a/Documentation/sysctl/kernel.txt
@@ -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.
==============================================================
diff -puN lib/vsprintf.c~vsprintf-check-real-user-group-id-for-%pk lib/vsprintf.c
--- a/lib/vsprintf.c~vsprintf-check-real-user-group-id-for-%pk
+++ a/lib/vsprintf.c
@@ -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
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':
_
Patches currently in -mm which might be from rmallon@xxxxxxxxx are
vsprintf-check-real-user-group-id-for-%pk.patch
linux-next.patch
--
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
