| 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
| ||
|
Message-ID: <20081020110018.GA24579@elte.hu>
Date: Mon, 20 Oct 2008 13:00:18 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc: Linus Torvalds <torvalds@...l.org>, linux-kernel@...r.kernel.org,
David Miller <davem@...emloft.net>, linux-pci@...r.kernel.org,
yhlu.kernel@...il.com, Andrew Morton <akpm@...ux-foundation.org>,
Jesse Barnes <jbarnes@...tuousgeek.org>
Subject: Re: [PATCH] x86, ioremap: use %pR in printk
* Ingo Molnar <mingo@...e.hu> wrote:
> > No, you don't pass it a phys_addr_t or a resource_size_t, you pass
> > it a struct resource *
>
> oops ... i only looked at:
>
> + char sym[4*sizeof(resource_size_t) + 8];
>
> and assumed that it was a resource_size_t printout format :-/
>
> while printing out ranges is useful too, it's by far not the only
> source of ugliness all around resource_size_t.
so how about something like the two patches below (ontop of Linus's
patch): the first patch introduces a "small" resource pointer printout
format: %pr - the little brother of %pR.
The output format is [0x00001234] - minimum width is 8.
The second patch takes advantage of it in ioremap.c.
Not tested yet but should work. One thing that would be nice is to
extend it to phys_addr_t as well. Right now there's no guarantee that
sizeof(resource_size_t) == sizeof(phys_addr_t).
What do you think? Passing it by reference makes it somewhat ugly [and
restricts it to lvalues], but that's the safest we can get at the moment
i guess, as it guarantees followup vararg parameter correctness, because
gcc does not check these extensions.
Worst-case we get gibberish output if it's used incorrectly - but we
dont get a kernel security hole if a %s follows it in the format string
and an attacker can control some of the parameters, etc.
Ingo
---------------->
>From bec931cb306f0901b9e2017c845d5ad6016574e4 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...e.hu>
Date: Mon, 20 Oct 2008 12:39:17 +0200
Subject: [PATCH] vsprintf: implement %pr to print resource_size_t
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
lib/vsprintf.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a013bbc..ddcaa6e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -581,6 +581,21 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
return string(buf, end, sym, field_width, precision, flags);
}
+static char *resource_size_string(char *buf, char *end, resource_size_t *val, int field_width, int precision, int flags)
+{
+ /* room for the actual number, the one "0x", [, ] and the final zero */
+ char sym[2*sizeof(resource_size_t) + 5];
+ char *p = sym, *pend = sym + sizeof(sym);
+ int size = 8;
+
+ *p++ = '[';
+ p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
+ *p++ = ']';
+ *p = 0;
+
+ return string(buf, end, sym, field_width, precision, flags);
+}
+
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -592,6 +607,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
* - 'S' For symbolic direct pointers
* - 'R' For a struct resource pointer, it prints the range of
* addresses (not the name nor the flags)
+ * - 'r' For a resource_size_t pointer, it prints the resource
+ * addresses (in %pR format)
*
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a
@@ -607,6 +624,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
return symbol_string(buf, end, ptr, field_width, precision, flags);
case 'R':
return resource_string(buf, end, ptr, field_width, precision, flags);
+ case 'r':
+ return resource_size_string(buf, end, ptr, field_width, precision, flags);
}
flags |= SMALL;
if (field_width == -1) {
@@ -627,6 +646,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
* %pS output the name of a text symbol
* %pF output the name of a function pointer
* %pR output the address range in a struct resource
+ * %pr output the address in a pointer to a resource_size_t type
*
* The return value is the number of characters which would
* be generated for the given input, excluding the trailing
>From cfbd2eec241a53bc5c1b95bb68f269036e3e92a4 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@...e.hu>
Date: Mon, 20 Oct 2008 09:08:57 +0200
Subject: [PATCH] x86, ioremap: use %pr in printk
use the new %pr IO resource pointer/address/size printk type conversion
specifier.
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
arch/x86/mm/ioremap.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index ae71e11..4725c7a 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
return NULL;
if (!phys_addr_valid(phys_addr)) {
- printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
- (unsigned long long)phys_addr);
+ printk(KERN_WARNING "ioremap: invalid physical address %pr\n",
+ &phys_addr);
WARN_ON_ONCE(1);
return NULL;
}
@@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
(prot_val == _PAGE_CACHE_WC &&
new_prot_val == _PAGE_CACHE_WB)) {
pr_debug(
- "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
- (unsigned long long)phys_addr,
- (unsigned long long)(phys_addr + size),
+ "ioremap error for %pr [%lx], requested 0x%lx, got 0x%lx\n",
+ &phys_addr,
+ size,
prot_val, new_prot_val);
free_memtype(phys_addr, phys_addr + size);
return NULL;
--
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/
Powered by blists - more mailing lists