Menu
▾
▴
user-mode-linux-user
|
From: Joe P. <jo...@pe...> - 2012-12-12 18:19:42
|
Remove the somewhat awkward uses of print_symbol and convert all the
existing uses to a new vsprintf pointer type of %pSR.
print_symbol can be interleaved when it is used in a sequence like:
printk("something: ...");
print_symbol("%s", addr);
printk("\n");
Instead use:
printk("something: %pSR\n", (void *)addr);
Add a new %p[SsFf]R vsprintf extension that can perform the same
symbol function/address/offset formatting as print_symbol to
reduce the number and styles of message logging functions.
print_symbol used __builtin_extract_return_addr for those architectures
like S/390 and SPARC that have offset or masked addressing.
%p[FfSs]R uses the same gcc __builtin
Joe Perches (26):
vsprintf: Add extension %pSR - print_symbol replacement
alpha: Convert print_symbol to %pSR
arm: Convert print_symbol to %pSR
arm64: Convert print_symbol to %pSR
avr32: Convert print_symbol to %pSR
c6x: Convert print_symbol to %pSR
ia64: Convert print_symbol to %pSR
m32r: Convert print_symbol to %pSR
mn10300: Convert print_symbol to %pSR
openrisc: Convert print_symbol to %pSR
powerpc: Convert print_symbol to %pSR
s390: Convert print_symbol to %pSR
sh: Convert print_symbol to %pSR
um: Convert print_symbol to %pSR
unicore32: Convert print_symbol to %pSR
x86: Convert print_symbol to %pSR
xtensa: Convert print_symbol to %pSR
drivers: base: Convert print_symbol to %pSR
gfs2: Convert print_symbol to %pSR
sysfs: Convert print_symbol to %pSR
irq: Convert print_symbol to %pSR
smp_processor_id: Convert print_symbol to %pSR
mm: Convert print_symbol to %pSR
xtensa: Convert print_symbol to %pSR
x86: head_64.S: Use vsprintf extension %pSR not print_symbol
kallsyms: Remove print_symbol
Documentation/filesystems/sysfs.txt | 4 +-
Documentation/printk-formats.txt | 2 +
Documentation/zh_CN/filesystems/sysfs.txt | 4 +-
arch/alpha/kernel/traps.c | 8 ++----
arch/arm/kernel/process.c | 4 +-
arch/arm64/kernel/process.c | 4 +-
arch/avr32/kernel/process.c | 25 ++++++-----------------
arch/c6x/kernel/traps.c | 3 +-
arch/ia64/kernel/process.c | 13 ++++-------
arch/m32r/kernel/traps.c | 6 +---
arch/mn10300/kernel/traps.c | 8 +++---
arch/openrisc/kernel/traps.c | 7 +----
arch/powerpc/platforms/cell/spu_callbacks.c | 12 ++++------
arch/s390/kernel/traps.c | 28 +++++++++++++++-----------
arch/sh/kernel/process_32.c | 4 +-
arch/um/kernel/sysrq.c | 6 +---
arch/unicore32/kernel/process.c | 5 ++-
arch/x86/kernel/cpu/mcheck/mce.c | 13 ++++++-----
arch/x86/kernel/dumpstack.c | 5 +--
arch/x86/kernel/head_64.S | 4 +-
arch/x86/kernel/process_32.c | 2 +-
arch/x86/mm/mmio-mod.c | 4 +-
arch/x86/um/sysrq_32.c | 9 ++-----
arch/xtensa/kernel/traps.c | 6 +---
drivers/base/core.c | 4 +-
fs/gfs2/glock.c | 4 +-
fs/gfs2/trans.c | 3 +-
fs/sysfs/file.c | 4 +-
include/linux/kallsyms.h | 18 -----------------
kernel/irq/debug.h | 15 ++++++-------
kernel/kallsyms.c | 11 ----------
lib/smp_processor_id.c | 2 +-
lib/vsprintf.c | 18 ++++++++++++----
mm/memory.c | 8 +++---
mm/slab.c | 8 ++----
35 files changed, 117 insertions(+), 164 deletions(-)
--
1.7.8.112.g3fd21
|
|
From: Joe P. <jo...@pe...> - 2012-12-12 18:21:55
|
Use the new vsprintf extension to avoid any possible
message interleaving.
Signed-off-by: Joe Perches <jo...@pe...>
---
arch/um/kernel/sysrq.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/um/kernel/sysrq.c b/arch/um/kernel/sysrq.c
index e562ff8..6fa632c 100644
--- a/arch/um/kernel/sysrq.c
+++ b/arch/um/kernel/sysrq.c
@@ -24,10 +24,8 @@ void show_trace(struct task_struct *task, unsigned long * stack)
while (((long) stack & (THREAD_SIZE-1)) != 0) {
addr = *stack;
if (__kernel_text_address(addr)) {
- printk(KERN_INFO "%08lx: [<%08lx>]",
- (unsigned long) stack, addr);
- print_symbol(KERN_CONT " %s", addr);
- printk(KERN_CONT "\n");
+ printk(KERN_INFO "%08lx: [<%08lx>] %pSR\n",
+ (unsigned long)stack, addr, (void *)addr);
}
stack++;
}
--
1.7.8.112.g3fd21
|
|
From: Joe P. <jo...@pe...> - 2012-12-12 18:22:16
|
Use the new vsprintf extension to avoid any possible
message interleaving.
Signed-off-by: Joe Perches <jo...@pe...>
---
arch/x86/kernel/cpu/mcheck/mce.c | 13 +++++++------
arch/x86/kernel/dumpstack.c | 5 ++---
arch/x86/kernel/process_32.c | 2 +-
arch/x86/mm/mmio-mod.c | 4 ++--
arch/x86/um/sysrq_32.c | 9 +++------
5 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 80dbda8..996f98a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
m->extcpu, m->mcgstatus, m->bank, m->status);
if (m->ip) {
- pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
- !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
- m->cs, m->ip);
-
if (m->cs == __KERNEL_CS)
- print_symbol("{%s}", m->ip);
- pr_cont("\n");
+ pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+ !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+ m->cs, m->ip, (void *)m->ip);
+ else
+ pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+ !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+ m->cs, m->ip);
}
pr_emerg(HW_ERR "TSC %llx ", m->tsc);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index ae42418b..b6e5bdb 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -281,9 +281,8 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
sp = kernel_stack_pointer(regs);
savesegment(ss, ss);
}
- printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip);
- print_symbol("%s", regs->ip);
- printk(" SS:ESP %04x:%08lx\n", ss, sp);
+ printk(KERN_EMERG "EIP: [<%08lx>] %pSR SS:ESP %04x:%08lx\n",
+ regs->ip, (void *)regs->ip, ss, sp);
#else
/* Executive summary in case the oops scrolled away */
printk(KERN_ALERT "RIP ");
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index b5a8905..0db77e0 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -89,7 +89,7 @@ void __show_regs(struct pt_regs *regs, int all)
printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n",
(u16)regs->cs, regs->ip, regs->flags,
smp_processor_id());
- print_symbol("EIP is at %s\n", regs->ip);
+ printk(KERN_DEFAULT "EIP is at %pSR\n", (void *)regs->ip);
printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n",
regs->ax, regs->bx, regs->cx, regs->dx);
diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index dc0b727..c0ca484 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -123,8 +123,8 @@ static void die_kmmio_nesting_error(struct pt_regs *regs, unsigned long addr)
pr_emerg("unexpected fault for address: 0x%08lx, last fault for address: 0x%08lx\n",
addr, my_reason->addr);
print_pte(addr);
- print_symbol(KERN_EMERG "faulting IP is at %s\n", regs->ip);
- print_symbol(KERN_EMERG "last faulting IP was at %s\n", my_reason->ip);
+ pr_emerg("faulting IP is at %pSR\n", (void *)regs->ip);
+ pr_emerg("last faulting IP was at %pSR\n", (void *)my_reason->ip);
#ifdef __i386__
pr_emerg("eax: %08lx ebx: %08lx ecx: %08lx edx: %08lx\n",
regs->ax, regs->bx, regs->cx, regs->dx);
diff --git a/arch/x86/um/sysrq_32.c b/arch/x86/um/sysrq_32.c
index c9bee5b..30b4200 100644
--- a/arch/x86/um/sysrq_32.c
+++ b/arch/x86/um/sysrq_32.c
@@ -50,18 +50,15 @@ static inline unsigned long print_context_stack(struct thread_info *tinfo,
#ifdef CONFIG_FRAME_POINTER
while (valid_stack_ptr(tinfo, (void *)ebp)) {
addr = *(unsigned long *)(ebp + 4);
- printk("%08lx: [<%08lx>]", ebp + 4, addr);
- print_symbol(" %s", addr);
- printk("\n");
+ printk("%08lx: [<%08lx>] %pSR\n", ebp + 4, addr, (void *)addr);
ebp = *(unsigned long *)ebp;
}
#else
while (valid_stack_ptr(tinfo, stack)) {
addr = *stack;
if (__kernel_text_address(addr)) {
- printk("%08lx: [<%08lx>]", (unsigned long) stack, addr);
- print_symbol(" %s", addr);
- printk("\n");
+ printk("%08lx: [<%08lx>] %pSR\n",
+ (unsigned long)stack, addr, (void *)addr);
}
stack++;
}
--
1.7.8.112.g3fd21
|
|
From: Borislav P. <bp...@al...> - 2012-12-12 21:28:17
|
On Wed, Dec 12, 2012 at 10:19:05AM -0800, Joe Perches wrote:
> Use the new vsprintf extension to avoid any possible
> message interleaving.
>
> Signed-off-by: Joe Perches <jo...@pe...>
> ---
> arch/x86/kernel/cpu/mcheck/mce.c | 13 +++++++------
> arch/x86/kernel/dumpstack.c | 5 ++---
> arch/x86/kernel/process_32.c | 2 +-
> arch/x86/mm/mmio-mod.c | 4 ++--
> arch/x86/um/sysrq_32.c | 9 +++------
> 5 files changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index 80dbda8..996f98a 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
> m->extcpu, m->mcgstatus, m->bank, m->status);
>
> if (m->ip) {
> - pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
> - !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> - m->cs, m->ip);
> -
> if (m->cs == __KERNEL_CS)
> - print_symbol("{%s}", m->ip);
> - pr_cont("\n");
> + pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
> + !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> + m->cs, m->ip, (void *)m->ip);
> + else
> + pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
> + !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> + m->cs, m->ip);
> }
I think I'd go ahead and ACK this unless Tony has some comments. I'm not
happy about the two pr_emerg calls based on the conditional.
Or, Tony, what do you think, could we get away if we printed empty
string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any
userspace which is parsing that output and seeing "... {}" in that case.
This assumes that vsprintf can print (void *)0 when passed as an
argument through %pSR. Joe?
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
|
|
From: Joe P. <jo...@pe...> - 2012-12-12 21:30:12
|
On Wed, 2012-12-12 at 22:09 +0100, Borislav Petkov wrote:
> On Wed, Dec 12, 2012 at 10:19:05AM -0800, Joe Perches wrote:
> > Use the new vsprintf extension to avoid any possible
> > message interleaving.
[]
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
[]
> > @@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
> > m->extcpu, m->mcgstatus, m->bank, m->status);
> >
> > if (m->ip) {
> > - pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
> > - !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> > - m->cs, m->ip);
> > -
> > if (m->cs == __KERNEL_CS)
> > - print_symbol("{%s}", m->ip);
> > - pr_cont("\n");
> > + pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
> > + !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> > + m->cs, m->ip, (void *)m->ip);
> > + else
> > + pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
> > + !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
> > + m->cs, m->ip);
> > }
>
> I think I'd go ahead and ACK this unless Tony has some comments. I'm not
> happy about the two pr_emerg calls based on the conditional.
It was done to avoid interleaving.
> Or, Tony, what do you think, could we get away if we printed empty
> string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any
> userspace which is parsing that output and seeing "... {}" in that case.
>
> This assumes that vsprintf can print (void *)0 when passed as an
> argument through %pSR. Joe?
Definitely yes when not #defined CONFIG_KALLSYMS
I believe no object exists at address 0 for all
arches so I believe yes for CONFIG_KALLSYMS too,
My preference is to eventually do away with all the
"[%0(size)lx] %pSR", addr, (void *)addr
uses and create another %pSx that emits the pointer
address and the function/offset in one go in a
standardized style without caring about the pointer
size.
Something like:
"%pSp", (void *)addr
would emit
[<7def0123>] function_name+offset/size
|
|
From: Borislav P. <bp...@al...> - 2012-12-12 21:49:08
|
On Wed, Dec 12, 2012 at 01:30:03PM -0800, Joe Perches wrote:
> > I think I'd go ahead and ACK this unless Tony has some comments. I'm
> > not happy about the two pr_emerg calls based on the conditional.
>
> It was done to avoid interleaving.
Right.
> > Or, Tony, what do you think, could we get away if we printed empty
> > string for when m->cs != __KERNEL_CS? I'm thinking of not breaking any
> > userspace which is parsing that output and seeing "... {}" in that case.
> >
> > This assumes that vsprintf can print (void *)0 when passed as an
> > argument through %pSR. Joe?
>
> Definitely yes when not #defined CONFIG_KALLSYMS
>
> I believe no object exists at address 0 for all
> arches so I believe yes for CONFIG_KALLSYMS too,
Ok, here's what I'm thinking more specifically. Have single call like this:
pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
!(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
m->cs, m->ip,
(m->cs == __KERNEL_CS) ? (void *)m->ip : (void *)0);
and when vsprintf gets to it, it recognizes the special case of
(void *)0 and dumps the empty string "" for that argument.
Hmm.
> My preference is to eventually do away with all the
> "[%0(size)lx] %pSR", addr, (void *)addr
> uses and create another %pSx that emits the pointer
> address and the function/offset in one go in a
> standardized style without caring about the pointer
> size.
>
> Something like:
>
> "%pSp", (void *)addr
>
> would emit
>
> [<7def0123>] function_name+offset/size
Right, this is another way of looking it. And in order to make it more
robust, it should be able to handle the (void *)0 case so that callers
don't have to check the arg.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
|
|
From: Luck, T. <ton...@in...> - 2012-12-12 21:49:46
|
> I think I'd go ahead and ACK this unless Tony has some comments. I'm not
> happy about the two pr_emerg calls based on the conditional.
As written the patch has the nice property of not making any changes to the
console output (except to eliminate the possibility of interleaved output that
the original code had). Whereas making the change you suggest would end
up with a useless "{}" in the usermode case (or worse if %pSR did something
with m->ip .. which is most probably not NULL ... it should be a valid user mode
instruction pointer when m->cs != KERNEL_CS).
So Acked-by: Tony Luck <ton...@in...>
|
|
From: Joe P. <jo...@pe...> - 2012-12-12 22:24:08
|
On Wed, 2012-12-12 at 21:49 +0000, Luck, Tony wrote: > > I think I'd go ahead and ACK this unless Tony has some comments. I'm not > > happy about the two pr_emerg calls based on the conditional. > > As written the patch has the nice property of not making any changes to the > console output (except to eliminate the possibility of interleaved output that > the original code had). Well, it does eliminate a trailing space when m->cs != KERNEL_CS. That probably won't hurt anything, but... |
|
From: Borislav P. <bp...@al...> - 2012-12-12 22:45:59
|
On Wed, Dec 12, 2012 at 02:23:59PM -0800, Joe Perches wrote:
> On Wed, 2012-12-12 at 21:49 +0000, Luck, Tony wrote:
> > > I think I'd go ahead and ACK this unless Tony has some comments. I'm not
> > > happy about the two pr_emerg calls based on the conditional.
> >
> > As written the patch has the nice property of not making any changes to the
> > console output (except to eliminate the possibility of interleaved output that
> > the original code had).
>
> Well, it does eliminate a trailing space when m->cs != KERNEL_CS.
> That probably won't hurt anything, but...
Yeah, that won't be an issue. So let's take it as is, as Tony suggests.
Acked-by: Borislav Petkov <bp...@al...>
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
|
|
From: Joe P. <jo...@pe...> - 2012-12-13 18:23:19
|
On Wed, 2012-12-12 at 23:45 +0100, Borislav Petkov wrote: > On Wed, Dec 12, 2012 at 02:23:59PM -0800, Joe Perches wrote: > > On Wed, 2012-12-12 at 21:49 +0000, Luck, Tony wrote: > > > > I think I'd go ahead and ACK this unless Tony has some comments. I'm not > > > > happy about the two pr_emerg calls based on the conditional. > > > > > > As written the patch has the nice property of not making any changes to the > > > console output (except to eliminate the possibility of interleaved output that > > > the original code had). > > > > Well, it does eliminate a trailing space when m->cs != KERNEL_CS. > > That probably won't hurt anything, but... > > Yeah, that won't be an issue. So let's take it as is, as Tony suggests. Hi again Boris. I should mention one more thing. m->ip is a u64 so, when compiling x86-32, there's a new warning "cast to pointer from integer of different size". This isn't new different behavior, just a new warning. The previous print_symbol took a ulong and the u64 was silently truncated. CC arch/x86/kernel/cpu/mcheck/mce.o arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’: arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] If appropriate, the code could be changed to (void *)(unsigned long)m->ip to shut the compiler up. |
|
From: Borislav P. <bp...@al...> - 2012-12-13 18:37:14
|
On Thu, Dec 13, 2012 at 10:23:10AM -0800, Joe Perches wrote:
> m->ip is a u64 so, when compiling x86-32, there's a new warning
> "cast to pointer from integer of different size". This isn't new
> different behavior, just a new warning. The previous print_symbol
> took a ulong and the u64 was silently truncated.
>
> CC arch/x86/kernel/cpu/mcheck/mce.o
> arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’:
> arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>
> If appropriate, the code could be changed to
>
> (void *)(unsigned long)m->ip
Can we explicitly cast it to what it is so that we can be explicit as to
what we're casting it? IOW:
(void *)(__u64)m->ip;
Does that even work on 32bit?
Also, does the compiler bitch about this useless cast when building with
W=123?
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
|
|
From: Joe P. <jo...@pe...> - 2012-12-13 18:57:31
|
On Thu, 2012-12-13 at 19:37 +0100, Borislav Petkov wrote:
> On Thu, Dec 13, 2012 at 10:23:10AM -0800, Joe Perches wrote:
> > m->ip is a u64 so, when compiling x86-32, there's a new warning
> > "cast to pointer from integer of different size". This isn't new
> > different behavior, just a new warning. The previous print_symbol
> > took a ulong and the u64 was silently truncated.
> >
> > CC arch/x86/kernel/cpu/mcheck/mce.o
> > arch/x86/kernel/cpu/mcheck/mce.c: In function ‘print_mce’:
> > arch/x86/kernel/cpu/mcheck/mce.c:246:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >
> > If appropriate, the code could be changed to
> >
> > (void *)(unsigned long)m->ip
>
> Can we explicitly cast it to what it is so that we can be explicit as to
> what we're casting it? IOW:
>
> (void *)(__u64)m->ip;
>
> Does that even work on 32bit?
No, it already is __u64. You need (unsigned long)
or something else the same size as (void *)
from: arch/x86/include/asm/mce.h
/* Fields are zero when not available */
struct mce {
...
__u64 ip;
> Also, does the compiler bitch about this useless cast when building with
> W=123?
For x86-32:
With the cast the line is silent.
Without the cast, just the warning about cast.
|
|
From: H. P. A. <hp...@zy...> - 2012-12-13 18:43:35
|
On 12/13/2012 10:37 AM, Borislav Petkov wrote: >> >> If appropriate, the code could be changed to >> >> (void *)(unsigned long)m->ip > > Can we explicitly cast it to what it is so that we can be explicit as to > what we're casting it? IOW: > > (void *)(__u64)m->ip; > > Does that even work on 32bit? > > Also, does the compiler bitch about this useless cast when building with > W=123? > Uh... no. The point is that (void *)(unsigned long) casts it to an integer of pointer size -- in userspace you would to (void *)(size_t) or (void *)(uintptr_t) -- so that the compiler knows "I meant to do that." -hpa |
|
From: Borislav P. <bp...@al...> - 2012-12-13 19:05:56
|
On Thu, Dec 13, 2012 at 10:42:28AM -0800, H. Peter Anvin wrote:
> On 12/13/2012 10:37 AM, Borislav Petkov wrote:
> >>
> >> If appropriate, the code could be changed to
> >>
> >> (void *)(unsigned long)m->ip
> >
> > Can we explicitly cast it to what it is so that we can be explicit as to
> > what we're casting it? IOW:
> >
> > (void *)(__u64)m->ip;
> >
> > Does that even work on 32bit?
> >
> > Also, does the compiler bitch about this useless cast when building with
> > W=123?
> >
>
> Uh... no.
>
> The point is that (void *)(unsigned long) casts it to an integer of
> pointer size -- in userspace you would to (void *)(size_t) or (void
> *)(uintptr_t) -- so that the compiler knows "I meant to do that."
Ok, I grok it now. So, in most cases, mce.ip comes from pt_regs.ip which
is unsigned long so we're fine for both 32- and 64-bit. There's only the
case with precise RIP reporting where we get the rip from an MSR but
even then, it should be 4 bytes on 32-bit since our addresses there are
4 bytes.
So, actually struct mce should've had ->ip defined as an unsigned long
from the very beginning. But we can't change that now because this
struct is shared with userspace :(
So yes Joe, make it (void *)(unsigned long).
Thanks guys.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
|
|
From: H. P. A. <hp...@zy...> - 2012-12-13 21:36:38
|
On 12/13/2012 11:05 AM, Borislav Petkov wrote: > > Ok, I grok it now. So, in most cases, mce.ip comes from pt_regs.ip which > is unsigned long so we're fine for both 32- and 64-bit. There's only the > case with precise RIP reporting where we get the rip from an MSR but > even then, it should be 4 bytes on 32-bit since our addresses there are > 4 bytes. > > So, actually struct mce should've had ->ip defined as an unsigned long > from the very beginning. But we can't change that now because this > struct is shared with userspace :( > Good thing, then,. otherwise we'd have a compat headache. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. |
|
From: Joe P. <jo...@pe...> - 2012-12-13 19:13:37
|
Use the new vsprintf extension to avoid any possible
message interleaving.
Signed-off-by: Joe Perches <jo...@pe...>
Acked-by: Tony Luck <ton...@in...>
Acked-by: Borislav Petkov <bp...@al...>
---
V2: mce.c: Add cast to (unsigned long) before cast to (void *)
to quiet the compiler on x86-32
arch/x86/kernel/cpu/mcheck/mce.c | 13 +++++++------
arch/x86/kernel/dumpstack.c | 5 ++---
arch/x86/kernel/process_32.c | 2 +-
arch/x86/mm/mmio-mod.c | 4 ++--
arch/x86/um/sysrq_32.c | 9 +++------
5 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 80dbda8..996f98a 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -242,13 +242,14 @@ static void print_mce(struct mce *m)
m->extcpu, m->mcgstatus, m->bank, m->status);
if (m->ip) {
- pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> ",
- !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
- m->cs, m->ip);
-
if (m->cs == __KERNEL_CS)
- print_symbol("{%s}", m->ip);
- pr_cont("\n");
+ pr_emerg(HW_ERR "RIP%s %02x:<%016Lx> {%pSR}\n",
+ !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+ m->cs, m->ip, (void *)(unsigned long)m->ip);
+ else
+ pr_emerg(HW_ERR "RIP%s %02x:<%016Lx>\n",
+ !(m->mcgstatus & MCG_STATUS_EIPV) ? " !INEXACT!" : "",
+ m->cs, m->ip);
}
pr_emerg(HW_ERR "TSC %llx ", m->tsc);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index ae42418b..b6e5bdb 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -281,9 +281,8 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
sp = kernel_stack_pointer(regs);
savesegment(ss, ss);
}
- printk(KERN_EMERG "EIP: [<%08lx>] ", regs->ip);
- print_symbol("%s", regs->ip);
- printk(" SS:ESP %04x:%08lx\n", ss, sp);
+ printk(KERN_EMERG "EIP: [<%08lx>] %pSR SS:ESP %04x:%08lx\n",
+ regs->ip, (void *)regs->ip, ss, sp);
#else
/* Executive summary in case the oops scrolled away */
printk(KERN_ALERT "RIP ");
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index b5a8905..0db77e0 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -89,7 +89,7 @@ void __show_regs(struct pt_regs *regs, int all)
printk(KERN_DEFAULT "EIP: %04x:[<%08lx>] EFLAGS: %08lx CPU: %d\n",
(u16)regs->cs, regs->ip, regs->flags,
smp_processor_id());
- print_symbol("EIP is at %s\n", regs->ip);
+ printk(KERN_DEFAULT "EIP is at %pSR\n", (void *)regs->ip);
printk(KERN_DEFAULT "EAX: %08lx EBX: %08lx ECX: %08lx EDX: %08lx\n",
regs->ax, regs->bx, regs->cx, regs->dx);
diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c
index dc0b727..c0ca484 100644
--- a/arch/x86/mm/mmio-mod.c
+++ b/arch/x86/mm/mmio-mod.c
@@ -123,8 +123,8 @@ static void die_kmmio_nesting_error(struct pt_regs *regs, unsigned long addr)
pr_emerg("unexpected fault for address: 0x%08lx, last fault for address: 0x%08lx\n",
addr, my_reason->addr);
print_pte(addr);
- print_symbol(KERN_EMERG "faulting IP is at %s\n", regs->ip);
- print_symbol(KERN_EMERG "last faulting IP was at %s\n", my_reason->ip);
+ pr_emerg("faulting IP is at %pSR\n", (void *)regs->ip);
+ pr_emerg("last faulting IP was at %pSR\n", (void *)my_reason->ip);
#ifdef __i386__
pr_emerg("eax: %08lx ebx: %08lx ecx: %08lx edx: %08lx\n",
regs->ax, regs->bx, regs->cx, regs->dx);
diff --git a/arch/x86/um/sysrq_32.c b/arch/x86/um/sysrq_32.c
index c9bee5b..30b4200 100644
--- a/arch/x86/um/sysrq_32.c
+++ b/arch/x86/um/sysrq_32.c
@@ -50,18 +50,15 @@ static inline unsigned long print_context_stack(struct thread_info *tinfo,
#ifdef CONFIG_FRAME_POINTER
while (valid_stack_ptr(tinfo, (void *)ebp)) {
addr = *(unsigned long *)(ebp + 4);
- printk("%08lx: [<%08lx>]", ebp + 4, addr);
- print_symbol(" %s", addr);
- printk("\n");
+ printk("%08lx: [<%08lx>] %pSR\n", ebp + 4, addr, (void *)addr);
ebp = *(unsigned long *)ebp;
}
#else
while (valid_stack_ptr(tinfo, stack)) {
addr = *stack;
if (__kernel_text_address(addr)) {
- printk("%08lx: [<%08lx>]", (unsigned long) stack, addr);
- print_symbol(" %s", addr);
- printk("\n");
+ printk("%08lx: [<%08lx>] %pSR\n",
+ (unsigned long)stack, addr, (void *)addr);
}
stack++;
}
--
1.7.8.112.g3fd21
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to maj...@vg...
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
|
Thanks for helping keep SourceForge clean.
X