* [PATCH v2] use %pK for /proc/kallsyms and /proc/modules
@ 2011-01-27 0:41 Kees Cook
2011-02-04 22:03 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2011-01-27 0:41 UTC (permalink / raw)
To: linux-kernel
Cc: Joe Perches, Rusty Russell, Tejun Heo, Marcus Meissner,
Jason Wessel, Eugene Teo, Andrew Morton, Bjorn Helgaas, Len Brown,
Changli Gao, Dan Rosenberg
In an effort to reduce kernel address leaks that might be used to
help target kernel privilege escalation exploits, this patch uses
%pK when displaying addresses in /proc/kallsyms, /proc/modules, and
/sys/module/*/sections/*.
Note that this changes %x to %p, so some legitimately 0 values in
/proc/kallsyms would have changed from 00000000 to "(null)". To avoid
this, "(null)" is not used when using the "K" format. Anything that was
already successfully parsing "(null)" in addition to full hex digits
should have no problem with this change. (Thanks to Joe Perches for
the suggestion.)
Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
v2:
- ditch %0* with 2*sizeof(void*) prefixing since %p is already rendered
to that width, thanks to Joe Perches.
---
kernel/kallsyms.c | 6 ++----
kernel/module.c | 4 ++--
lib/vsprintf.c | 2 +-
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 6f6d091..ff37fbd 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -477,12 +477,10 @@ 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",
- (int)(2 * sizeof(void *)),
+ seq_printf(m, "%pK %c %s\t[%s]\n",
iter->value, type, iter->name, iter->module_name);
} else
- seq_printf(m, "%0*lx %c %s\n",
- (int)(2 * sizeof(void *)),
+ seq_printf(m, "%pK %c %s\n",
iter->value, iter->type, iter->name);
return 0;
}
diff --git a/kernel/module.c b/kernel/module.c
index 34e00b7..748465c 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%pK\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..288d770 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -991,7 +991,7 @@ static noinline_for_stack
char *pointer(const char *fmt, char *buf, char *end, void *ptr,
struct printf_spec spec)
{
- if (!ptr) {
+ if (!ptr && *fmt != 'K') {
/*
* Print (null) with the same width as a pointer so it makes
* tabular output look nice.
--
1.7.2.3
--
Kees Cook
Ubuntu Security Team
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] use %pK for /proc/kallsyms and /proc/modules
2011-01-27 0:41 [PATCH v2] use %pK for /proc/kallsyms and /proc/modules Kees Cook
@ 2011-02-04 22:03 ` Andrew Morton
2011-02-04 22:09 ` Kees Cook
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2011-02-04 22:03 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Joe Perches, Rusty Russell, Tejun Heo,
Marcus Meissner, Jason Wessel, Eugene Teo, Bjorn Helgaas,
Len Brown, Changli Gao, Dan Rosenberg
On Wed, 26 Jan 2011 16:41:29 -0800
Kees Cook <kees.cook@canonical.com> wrote:
> In an effort to reduce kernel address leaks that might be used to
> help target kernel privilege escalation exploits, this patch uses
> %pK when displaying addresses in /proc/kallsyms, /proc/modules, and
> /sys/module/*/sections/*.
>
> Note that this changes %x to %p, so some legitimately 0 values in
> /proc/kallsyms would have changed from 00000000 to "(null)". To avoid
> this, "(null)" is not used when using the "K" format. Anything that was
> already successfully parsing "(null)" in addition to full hex digits
> should have no problem with this change. (Thanks to Joe Perches for
> the suggestion.)
>
> ...
>
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -477,12 +477,10 @@ 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",
> - (int)(2 * sizeof(void *)),
> + seq_printf(m, "%pK %c %s\t[%s]\n",
> iter->value, type, iter->name, iter->module_name);
> } else
> - seq_printf(m, "%0*lx %c %s\n",
> - (int)(2 * sizeof(void *)),
> + seq_printf(m, "%pK %c %s\n",
> iter->value, iter->type, iter->name);
> return 0;
> }
kernel/kallsyms.c: In function 's_show':
kernel/kallsyms.c:481: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
kernel/kallsyms.c:484: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
kernel/module.c: In function 'module_sect_show':
kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
I'm struggling to see how this could have been compile-time or runtime
tested?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] use %pK for /proc/kallsyms and /proc/modules
2011-02-04 22:03 ` Andrew Morton
@ 2011-02-04 22:09 ` Kees Cook
2011-02-04 22:21 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2011-02-04 22:09 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Joe Perches, Rusty Russell, Tejun Heo,
Marcus Meissner, Jason Wessel, Eugene Teo, Bjorn Helgaas,
Len Brown, Changli Gao, Dan Rosenberg
Hi Andrew,
On Fri, Feb 04, 2011 at 02:03:51PM -0800, Andrew Morton wrote:
> kernel/kallsyms.c: In function 's_show':
> kernel/kallsyms.c:481: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> kernel/kallsyms.c:484: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> kernel/module.c: In function 'module_sect_show':
> kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
>
> I'm struggling to see how this could have been compile-time or runtime
> tested?
I run-time tested it plenty. The thread contains the various discussions
about compile-time warnings, so I suspect in the last version, I didn't go
examine the warnings (since the origin of the other warnings went away).
I can send a patch to fix it up to cast everything to (void*) if you want?
-Kees
--
Kees Cook
Ubuntu Security Team
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] use %pK for /proc/kallsyms and /proc/modules
2011-02-04 22:09 ` Kees Cook
@ 2011-02-04 22:21 ` Andrew Morton
2011-02-04 22:34 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2011-02-04 22:21 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Joe Perches, Rusty Russell, Tejun Heo,
Marcus Meissner, Jason Wessel, Eugene Teo, Bjorn Helgaas,
Len Brown, Changli Gao, Dan Rosenberg
On Fri, 4 Feb 2011 14:09:44 -0800
Kees Cook <kees.cook@canonical.com> wrote:
> Hi Andrew,
>
> On Fri, Feb 04, 2011 at 02:03:51PM -0800, Andrew Morton wrote:
> > kernel/kallsyms.c: In function 's_show':
> > kernel/kallsyms.c:481: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> > kernel/kallsyms.c:484: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> > kernel/module.c: In function 'module_sect_show':
> > kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> > kernel/module.c:1171: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> >
> > I'm struggling to see how this could have been compile-time or runtime
> > tested?
>
> I run-time tested it plenty. The thread contains the various discussions
> about compile-time warnings, so I suspect in the last version, I didn't go
> examine the warnings (since the origin of the other warnings went away).
>
It's passing `unsigned long kallsym_iter.value' into vsprintf as a
pointer. Won't vsprintf end up dereferenceing that unsigned long?
> I can send a patch to fix it up to cast everything to (void*) if you want?
No typecasts, please. Get the types *correct* and they won't be needed.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] use %pK for /proc/kallsyms and /proc/modules
2011-02-04 22:21 ` Andrew Morton
@ 2011-02-04 22:34 ` Joe Perches
0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2011-02-04 22:34 UTC (permalink / raw)
To: Andrew Morton
Cc: Kees Cook, linux-kernel, Rusty Russell, Tejun Heo,
Marcus Meissner, Jason Wessel, Eugene Teo, Bjorn Helgaas,
Len Brown, Changli Gao, Dan Rosenberg
On Fri, 2011-02-04 at 14:21 -0800, Andrew Morton wrote:
> On Fri, 4 Feb 2011 14:09:44 -0800
> Kees Cook <kees.cook@canonical.com> wrote:
> > On Fri, Feb 04, 2011 at 02:03:51PM -0800, Andrew Morton wrote:
> > > kernel/kallsyms.c:481: warning: format '%p' expects type 'void *', but argument 3 has type 'long unsigned int'
> > > I'm struggling to see how this could have been compile-time or runtime
> > > tested?
> > I run-time tested it plenty. The thread contains the various discussions
> > about compile-time warnings, so I suspect in the last version, I didn't go
> > examine the warnings (since the origin of the other warnings went away).
> It's passing `unsigned long kallsym_iter.value' into vsprintf as a
> pointer. Won't vsprintf end up dereferenceing that unsigned long?
No it won't. %p and %pK output the pointer value.
> > I can send a patch to fix it up to cast everything to (void*) if you want?
> No typecasts, please. Get the types *correct* and they won't be needed.
There are many printk casts of longs to void *.
grep -rP --include=*.[ch] "\b(printk|pr_*[a-z]+).*\".*\(void \*\)" *
Typecast is the right way to fix this warning using %pK,
though there might be a different/better way altogether.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-04 22:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-27 0:41 [PATCH v2] use %pK for /proc/kallsyms and /proc/modules Kees Cook
2011-02-04 22:03 ` Andrew Morton
2011-02-04 22:09 ` Kees Cook
2011-02-04 22:21 ` Andrew Morton
2011-02-04 22:34 ` Joe Perches
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.