linux-kernel - [PATCH 2/2] use %pK and %Klx for /proc/kallsyms and /proc/modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110125020755.GM4979@outflux.net>
Date:	Mon, 24 Jan 2011 18:07:55 -0800
From:	Kees Cook <kees.cook@...onical.com>
To:	linux-kernel@...r.kernel.org
Cc:	Rusty Russell <rusty@...tcorp.com.au>, Tejun Heo <tj@...nel.org>,
	Marcus Meissner <meissner@...e.de>,
	Jason Wessel <jason.wessel@...driver.com>,
	Eugene Teo <eugeneteo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Joe Perches <joe@...ches.com>,
	Bjorn Helgaas <bjorn.helgaas@...com>,
	Len Brown <len.brown@...el.com>,
	Changli Gao <xiaosuo@...il.com>,
	Dan Rosenberg <drosenberg@...curity.com>
Subject: [PATCH 2/2] use %pK and %Klx for /proc/kallsyms and /proc/modules

Instead of messing with permissions on these files, use %pK and %Klx
for kernel addresses to reduce potential information leaks that might
be used to help target kernel privilege escalation exploits.

Note that instead of changing %x to %p, this introduces the "K" flag
to printf numbers. Without this, some legitimately 0x0 values
in /proc/kallsyms would have changed from 00000000 to "(null)".

However, build-time warnings are emitted when built with -Wformat
  warning: unknown conversion type character 'K' in format
even though it produced sensible results.

Signed-off-by: Kees Cook <kees.cook@...onical.com>
---
 kernel/kallsyms.c |    4 ++--
 kernel/module.c   |    4 ++--
 lib/vsprintf.c    |   23 +++++++++++++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 6f6d091..080294d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -477,11 +477,11 @@ static int s_show(struct seq_file *m, void *p)
 		 */
 		type = iter->exported ? toupper(iter->type) :
 					tolower(iter->type);
-		seq_printf(m, "%0*lx %c %s\t[%s]\n",
+		seq_printf(m, "%K0*lx %c %s\t[%s]\n",
 			   (int)(2 * sizeof(void *)),
 			   iter->value, type, iter->name, iter->module_name);
 	} else
-		seq_printf(m, "%0*lx %c %s\n",
+		seq_printf(m, "%K0*lx %c %s\n",
 			   (int)(2 * sizeof(void *)),
 			   iter->value, iter->type, iter->name);
 	return 0;
diff --git a/kernel/module.c b/kernel/module.c
index 34e00b7..0f46775 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1168,7 +1168,7 @@ static ssize_t module_sect_show(struct module_attribute *mattr,
 {
 	struct module_sect_attr *sattr =
 		container_of(mattr, struct module_sect_attr, mattr);
-	return sprintf(buf, "0x%lx\n", sattr->address);
+	return sprintf(buf, "0x%Klx\n", sattr->address);
 }
 
 static void free_sect_attrs(struct module_sect_attrs *sect_attrs)
@@ -3224,7 +3224,7 @@ static int m_show(struct seq_file *m, void *p)
 		   mod->state == MODULE_STATE_COMING ? "Loading":
 		   "Live");
 	/* Used by oprofile and other similar tools. */
-	seq_printf(m, " 0x%p", mod->module_core);
+	seq_printf(m, " 0x%pK", mod->module_core);
 
 	/* Taints info */
 	if (mod->taints)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d3023df..736c174 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -382,6 +382,7 @@ char *put_dec(char *buf, unsigned long long num)
 #define LEFT	16		/* left justified */
 #define SMALL	32		/* use lowercase in hex (must be 32 == 0x20) */
 #define SPECIAL	64		/* prefix hex with "0x", octal with "0" */
+#define HIDEVAL	128		/* only CAP_SYSLOG can see real value */
 
 enum format_type {
 	FORMAT_TYPE_NONE, /* Just a string part */
@@ -416,6 +417,9 @@ struct printf_spec {
 };
 
 static noinline_for_stack
+char *string(char *buf, char *end, const char *s, struct printf_spec spec);
+
+static noinline_for_stack
 char *number(char *buf, char *end, unsigned long long num,
 	     struct printf_spec spec)
 {
@@ -428,6 +432,24 @@ char *number(char *buf, char *end, unsigned long long num,
 	int need_pfx = ((spec.flags & SPECIAL) && spec.base != 10);
 	int i;
 
+	if (spec.flags & HIDEVAL) {
+		/*
+		 * HIDEVAL cannot be used in IRQ context because its test
+		 * for CAP_SYSLOG would be meaningless.
+		 */
+		if (in_irq() || in_serving_softirq() || in_nmi()) {
+			if (spec.field_width == -1)
+				spec.field_width = 2 * sizeof(void *);
+			return string(buf, end, "K-error", spec);
+		} else if ((kptr_restrict == 0) ||
+			 (kptr_restrict == 1 &&
+			  has_capability_noaudit(current, CAP_SYSLOG))) {
+			/* do not hide value */
+		} else {
+			num = 0;
+		}
+	}
+
 	/* locase = 0 or 0x20. ORing digits or letters with 'locase'
 	 * produces same digits or (maybe lowercased) letters */
 	locase = (spec.flags & SMALL);
@@ -1138,6 +1160,7 @@ int format_decode(const char *fmt, struct printf_spec *spec)
 		case ' ': spec->flags |= SPACE;   break;
 		case '#': spec->flags |= SPECIAL; break;
 		case '0': spec->flags |= ZEROPAD; break;
+		case 'K': spec->flags |= HIDEVAL; break;
 		default:  found = false;
 		}
 
-- 
1.7.2.3


-- 
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/