All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: kernel-hardening@lists.openwall.com
Cc: "Tobin C. Harding" <me@tobin.cc>,
	linux-kernel@vger.kernel.org,
	"Network Development" <netdev@vger.kernel.org>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Tycho Andersen" <tycho@tycho.ws>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Petr Mladek" <pmladek@suse.com>, "Baoquan He" <bhe@redhat.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Ian Abbott" <abbotti@mev.co.uk>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Larry Finger" <Larry.Finger@lwfinger.net>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Joe Perches" <joe@perches.com>,
	"William Roberts" <william.c.roberts@intel.com>,
	"Rob Herring" <robh@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Pantelis Antoniou" <pantelis.antoniou@konsulko.com>,
	"Alexey Dobriyan" <adobriyan@gmail.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>
Subject: [RFC 2/3] vsprintf: print <no-symbol> if symbol not found
Date: Tue, 28 Nov 2017 09:30:18 +1100	[thread overview]
Message-ID: <1511821819-5496-3-git-send-email-me@tobin.cc> (raw)
In-Reply-To: <1511821819-5496-1-git-send-email-me@tobin.cc>

Depends on: commit bd6b239cdbb2 ("kallsyms: don't leak address when
symbol not found")

Currently vsprintf for specifiers %p[SsB] relies on the behaviour of
kallsyms (sprint_symbol()) and prints the actual address if a symbol is
not found. Previous patch changes this behaviour so tha sprint_symbol()
returns an error if symbol not found. With this patch in place we can
print a sanitized message '<no-symbol>' instead of leaking the address.

Future users of vsprintf may wish to know, after a call that uses
specifier %p[sSB], whether or not a symbol was found. The actual
sanitized string should be contained (isolated) within the vsprintf.c
therefore we should provide a predicate function. This also allows the
sanitized string to be updated at a later stage with minimal risk to
calling code.

Print '<no-symbol>' for printk specifier %s[sSB] if no symbol is
found. Provide predicate function string_is_no_symbol().

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 include/linux/kernel.h |  2 ++
 lib/vsprintf.c         | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ce51455e2adf..89e8ce79c2d1 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -460,6 +460,8 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
 extern __printf(2, 0)
 const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
 
+extern int string_is_no_symbol(const char *s);
+
 extern __scanf(2, 3)
 int sscanf(const char *, const char *, ...);
 extern __scanf(2, 0)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1746bae94d41..01e18a8c63fd 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -665,6 +665,8 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 }
 #endif
 
+#define PRINTK_NO_SYMBOL_STR "<no-symbol>"
+
 static noinline_for_stack
 char *symbol_string(char *buf, char *end, void *ptr,
 		    struct printf_spec spec, const char *fmt)
@@ -672,6 +674,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 	unsigned long value;
 #ifdef CONFIG_KALLSYMS
 	char sym[KSYM_SYMBOL_LEN];
+	int ret;
 #endif
 
 	if (fmt[1] == 'R')
@@ -680,11 +683,14 @@ char *symbol_string(char *buf, char *end, void *ptr,
 
 #ifdef CONFIG_KALLSYMS
 	if (*fmt == 'B')
-		sprint_backtrace(sym, value);
+		ret = sprint_backtrace(sym, value);
 	else if (*fmt != 'f' && *fmt != 's')
-		sprint_symbol(sym, value);
+		ret = sprint_symbol(sym, value);
 	else
-		sprint_symbol_no_offset(sym, value);
+		ret = sprint_symbol_no_offset(sym, value);
+
+	if (ret == -1)
+		strcpy(sym, PRINTK_NO_SYMBOL_STR);
 
 	return string(buf, end, sym, spec);
 #else
@@ -692,6 +698,12 @@ char *symbol_string(char *buf, char *end, void *ptr,
 #endif
 }
 
+int string_is_no_symbol(const char *s)
+{
+	return !!strstr(s, PRINTK_NO_SYMBOL_STR);
+}
+EXPORT_SYMBOL(string_is_no_symbol);
+
 static noinline_for_stack
 char *resource_string(char *buf, char *end, struct resource *res,
 		      struct printf_spec spec, const char *fmt)
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: "Tobin C. Harding" <me@tobin.cc>
To: kernel-hardening@lists.openwall.com
Cc: "Tobin C. Harding" <me@tobin.cc>,
	linux-kernel@vger.kernel.org,
	"Network Development" <netdev@vger.kernel.org>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Tycho Andersen" <tycho@tycho.ws>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Petr Mladek" <pmladek@suse.com>, "Baoquan He" <bhe@redhat.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Ian Abbott" <abbotti@mev.co.uk>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Larry Finger" <Larry.Finger@lwfinger.net>,
	"Andy Shevchenko" <andriy.shevchenko@linux.in>
Subject: [RFC 2/3] vsprintf: print <no-symbol> if symbol not found
Date: Tue, 28 Nov 2017 09:30:18 +1100	[thread overview]
Message-ID: <1511821819-5496-3-git-send-email-me@tobin.cc> (raw)
In-Reply-To: <1511821819-5496-1-git-send-email-me@tobin.cc>

Depends on: commit bd6b239cdbb2 ("kallsyms: don't leak address when
symbol not found")

Currently vsprintf for specifiers %p[SsB] relies on the behaviour of
kallsyms (sprint_symbol()) and prints the actual address if a symbol is
not found. Previous patch changes this behaviour so tha sprint_symbol()
returns an error if symbol not found. With this patch in place we can
print a sanitized message '<no-symbol>' instead of leaking the address.

Future users of vsprintf may wish to know, after a call that uses
specifier %p[sSB], whether or not a symbol was found. The actual
sanitized string should be contained (isolated) within the vsprintf.c
therefore we should provide a predicate function. This also allows the
sanitized string to be updated at a later stage with minimal risk to
calling code.

Print '<no-symbol>' for printk specifier %s[sSB] if no symbol is
found. Provide predicate function string_is_no_symbol().

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 include/linux/kernel.h |  2 ++
 lib/vsprintf.c         | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ce51455e2adf..89e8ce79c2d1 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -460,6 +460,8 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
 extern __printf(2, 0)
 const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
 
+extern int string_is_no_symbol(const char *s);
+
 extern __scanf(2, 3)
 int sscanf(const char *, const char *, ...);
 extern __scanf(2, 0)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1746bae94d41..01e18a8c63fd 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -665,6 +665,8 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 }
 #endif
 
+#define PRINTK_NO_SYMBOL_STR "<no-symbol>"
+
 static noinline_for_stack
 char *symbol_string(char *buf, char *end, void *ptr,
 		    struct printf_spec spec, const char *fmt)
@@ -672,6 +674,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 	unsigned long value;
 #ifdef CONFIG_KALLSYMS
 	char sym[KSYM_SYMBOL_LEN];
+	int ret;
 #endif
 
 	if (fmt[1] == 'R')
@@ -680,11 +683,14 @@ char *symbol_string(char *buf, char *end, void *ptr,
 
 #ifdef CONFIG_KALLSYMS
 	if (*fmt == 'B')
-		sprint_backtrace(sym, value);
+		ret = sprint_backtrace(sym, value);
 	else if (*fmt != 'f' && *fmt != 's')
-		sprint_symbol(sym, value);
+		ret = sprint_symbol(sym, value);
 	else
-		sprint_symbol_no_offset(sym, value);
+		ret = sprint_symbol_no_offset(sym, value);
+
+	if (ret == -1)
+		strcpy(sym, PRINTK_NO_SYMBOL_STR);
 
 	return string(buf, end, sym, spec);
 #else
@@ -692,6 +698,12 @@ char *symbol_string(char *buf, char *end, void *ptr,
 #endif
 }
 
+int string_is_no_symbol(const char *s)
+{
+	return !!strstr(s, PRINTK_NO_SYMBOL_STR);
+}
+EXPORT_SYMBOL(string_is_no_symbol);
+
 static noinline_for_stack
 char *resource_string(char *buf, char *end, struct resource *res,
 		      struct printf_spec spec, const char *fmt)
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: "Tobin C. Harding" <me@tobin.cc>
To: kernel-hardening@lists.openwall.com
Cc: "Tobin C. Harding" <me@tobin.cc>,
	linux-kernel@vger.kernel.org,
	"Network Development" <netdev@vger.kernel.org>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Tycho Andersen" <tycho@tycho.ws>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Petr Mladek" <pmladek@suse.com>, "Baoquan He" <bhe@redhat.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Ian Abbott" <abbotti@mev.co.uk>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Larry Finger" <Larry.Finger@lwfinger.net>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Joe Perches" <joe@perches.com>,
	"William Roberts" <william.c.roberts@intel.com>,
	"Rob Herring" <robh@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Pantelis Antoniou" <pantelis.antoniou@konsulko.com>,
	"Alexey Dobriyan" <adobriyan@gmail.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>
Subject: [kernel-hardening] [RFC 2/3] vsprintf: print <no-symbol> if symbol not found
Date: Tue, 28 Nov 2017 09:30:18 +1100	[thread overview]
Message-ID: <1511821819-5496-3-git-send-email-me@tobin.cc> (raw)
In-Reply-To: <1511821819-5496-1-git-send-email-me@tobin.cc>

Depends on: commit bd6b239cdbb2 ("kallsyms: don't leak address when
symbol not found")

Currently vsprintf for specifiers %p[SsB] relies on the behaviour of
kallsyms (sprint_symbol()) and prints the actual address if a symbol is
not found. Previous patch changes this behaviour so tha sprint_symbol()
returns an error if symbol not found. With this patch in place we can
print a sanitized message '<no-symbol>' instead of leaking the address.

Future users of vsprintf may wish to know, after a call that uses
specifier %p[sSB], whether or not a symbol was found. The actual
sanitized string should be contained (isolated) within the vsprintf.c
therefore we should provide a predicate function. This also allows the
sanitized string to be updated at a later stage with minimal risk to
calling code.

Print '<no-symbol>' for printk specifier %s[sSB] if no symbol is
found. Provide predicate function string_is_no_symbol().

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 include/linux/kernel.h |  2 ++
 lib/vsprintf.c         | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ce51455e2adf..89e8ce79c2d1 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -460,6 +460,8 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
 extern __printf(2, 0)
 const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
 
+extern int string_is_no_symbol(const char *s);
+
 extern __scanf(2, 3)
 int sscanf(const char *, const char *, ...);
 extern __scanf(2, 0)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1746bae94d41..01e18a8c63fd 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -665,6 +665,8 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev,
 }
 #endif
 
+#define PRINTK_NO_SYMBOL_STR "<no-symbol>"
+
 static noinline_for_stack
 char *symbol_string(char *buf, char *end, void *ptr,
 		    struct printf_spec spec, const char *fmt)
@@ -672,6 +674,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 	unsigned long value;
 #ifdef CONFIG_KALLSYMS
 	char sym[KSYM_SYMBOL_LEN];
+	int ret;
 #endif
 
 	if (fmt[1] == 'R')
@@ -680,11 +683,14 @@ char *symbol_string(char *buf, char *end, void *ptr,
 
 #ifdef CONFIG_KALLSYMS
 	if (*fmt == 'B')
-		sprint_backtrace(sym, value);
+		ret = sprint_backtrace(sym, value);
 	else if (*fmt != 'f' && *fmt != 's')
-		sprint_symbol(sym, value);
+		ret = sprint_symbol(sym, value);
 	else
-		sprint_symbol_no_offset(sym, value);
+		ret = sprint_symbol_no_offset(sym, value);
+
+	if (ret == -1)
+		strcpy(sym, PRINTK_NO_SYMBOL_STR);
 
 	return string(buf, end, sym, spec);
 #else
@@ -692,6 +698,12 @@ char *symbol_string(char *buf, char *end, void *ptr,
 #endif
 }
 
+int string_is_no_symbol(const char *s)
+{
+	return !!strstr(s, PRINTK_NO_SYMBOL_STR);
+}
+EXPORT_SYMBOL(string_is_no_symbol);
+
 static noinline_for_stack
 char *resource_string(char *buf, char *end, struct resource *res,
 		      struct printf_spec spec, const char *fmt)
-- 
2.7.4

  parent reply	other threads:[~2017-11-27 22:30 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 22:30 [RFC 0/3] kallsyms: don't leak address when printing symbol Tobin C. Harding
2017-11-27 22:30 ` [kernel-hardening] " Tobin C. Harding
2017-11-27 22:30 ` [RFC 1/3] kallsyms: don't leak address when symbol not found Tobin C. Harding
2017-11-27 22:30   ` [kernel-hardening] " Tobin C. Harding
2017-11-30  0:16   ` Tobin C. Harding
2017-11-30  0:16     ` [kernel-hardening] " Tobin C. Harding
2017-11-27 22:30 ` Tobin C. Harding [this message]
2017-11-27 22:30   ` [kernel-hardening] [RFC 2/3] vsprintf: print <no-symbol> if " Tobin C. Harding
2017-11-27 22:30   ` Tobin C. Harding
2017-11-27 22:30 ` [RFC 3/3] trace: print address " Tobin C. Harding
2017-11-27 22:30   ` [kernel-hardening] " Tobin C. Harding
2017-11-28  0:52 ` [RFC 0/3] kallsyms: don't leak address when printing symbol Kees Cook
2017-11-28  0:52   ` [kernel-hardening] " Kees Cook
2017-11-28  1:50   ` Tobin C. Harding
2017-11-28  1:50     ` [kernel-hardening] " Tobin C. Harding
2017-11-28  3:28     ` Kaiwan N Billimoria
2017-11-29 23:58       ` Tobin C. Harding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1511821819-5496-3-git-send-email-me@tobin.cc \
    --to=me@tobin.cc \
    --cc=Larry.Finger@lwfinger.net \
    --cc=abbotti@mev.co.uk \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bhe@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=pantelis.antoniou@konsulko.com \
    --cc=pmladek@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=robh@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tycho@tycho.ws \
    --cc=william.c.roberts@intel.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.