From ed79b86d8acf1f3d3bb83f04dc216c8dfa1d5970 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Thu, 20 Nov 2008 10:54:09 +0100 Subject: parisc: fix bug in compat_arch_ptrace Commit 81e192d6ce303b6792aa38ff35f41a1a7357f23a ("parisc: convert to generic compat_sys_ptrace") introduced a bug which segfaults the parisc 64bit kernel when stracing 32bit applications: Kernel Fault: Code=15 regs=00000000bafa42b0 (Addr=00000001baf5ab57) YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI PSW: 00001000000001101111111100001011 Tainted: G W r00-03 000000ff0806ff0b 000000004068edc0 00000000401203f8 00000000fb3e2508 r04-07 0000000040686dc0 00000000baf5a800 fffffffffffffffc fffffffffb3e2508 r08-11 00000000baf5a800 000000000004b068 00000000000402b0 0000000000040d68 r12-15 0000000000042a9c 0000000000040a9c 0000000000040d60 0000000000042e9c r16-19 000000000004b060 000000000004b058 0000000000042d9c ffffffffffffffff r20-23 000000000800000b 0000000000000000 000000000800000b fffffffffb3e2508 r24-27 00000000fffffffc 0000000000000003 00000000fffffffc 0000000040686dc0 r28-31 00000001baf5a7ff 00000000bafa4280 00000000bafa42b0 00000000000001d7 sr00-03 0000000000fca000 0000000000000000 0000000000000000 0000000000fca000 sr04-07 0000000000000000 0000000000000000 0000000000000000 0000000000000000 IASQ: 0000000000000000 0000000000000000 IAOQ: 0000000040120400 0000000040120404 IIR: 4b9a06b0 ISR: 0000000000000000 IOR: 00000001baf5ab57 CPU: 0 CR30: 00000000bafa4000 CR31: 00000000d22344e0 ORIG_R28: 00000000fb3e2248 IAOQ[0]: compat_arch_ptrace+0xb8/0x160 IAOQ[1]: compat_arch_ptrace+0xbc/0x160 RP(r2): compat_arch_ptrace+0xb0/0x160 Backtrace: [<00000000401612ac>] compat_sys_ptrace+0x15c/0x180 [<0000000040104ef8>] syscall_exit+0x0/0x14 The problem is that compat_arch_ptrace() enters with an addr value of type compat_ulong_t and calls translate_usr_offset() to translate the address offset into a struct pt_regs offset like this: addr = translate_usr_offset(addr) this means that any return value of translate_usr_offset() is stored back as compat_ulong_t type into the addr variable. But since translate_usr_offset() returns -1 for invalid offsets, addr can now get the value 0xffffffff which then fails the next return-value sanity check and thus the kernel tries to access invalid memory: if (addr < 0) break; Fix this bug by modifying translate_usr_offset() to take and return values of type compat_ulong_t, and by returning the value "sizeof(struct pt_regs)" as an error indicator. Additionally change the sanity check to check for return values for >= sizeof(struct pt_regs). This patch survived my compile and run-tests. Signed-off-by: Helge Deller Signed-off-by: Linus Torvalds --- arch/parisc/kernel/ptrace.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'arch/parisc') diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c index 90904f9..927db36 100644 --- a/arch/parisc/kernel/ptrace.c +++ b/arch/parisc/kernel/ptrace.c @@ -183,10 +183,10 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data) * being 64 bit in both cases. */ -static long translate_usr_offset(long offset) +static compat_ulong_t translate_usr_offset(compat_ulong_t offset) { if (offset < 0) - return -1; + return sizeof(struct pt_regs); else if (offset <= 32*4) /* gr[0..31] */ return offset * 2 + 4; else if (offset <= 32*4+32*8) /* gr[0..31] + fr[0..31] */ @@ -194,7 +194,7 @@ static long translate_usr_offset(long offset) else if (offset < sizeof(struct pt_regs)/2 + 32*4) return offset * 2 + 4 - 32*8; else - return -1; + return sizeof(struct pt_regs); } long compat_arch_ptrace(struct task_struct *child, compat_long_t request, @@ -209,7 +209,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, if (addr & (sizeof(compat_uint_t)-1)) break; addr = translate_usr_offset(addr); - if (addr < 0) + if (addr >= sizeof(struct pt_regs)) break; tmp = *(compat_uint_t *) ((char *) task_regs(child) + addr); @@ -236,7 +236,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, if (addr & (sizeof(compat_uint_t)-1)) break; addr = translate_usr_offset(addr); - if (addr < 0) + if (addr >= sizeof(struct pt_regs)) break; if (addr >= PT_FR0 && addr <= PT_FR31 + 4) { /* Special case, fp regs are 64 bits anyway */ -- cgit v1.1 From 9860d1b08b082ffb54c4b7827c48c2728e12ba21 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Sun, 16 Nov 2008 12:04:13 +0100 Subject: parisc: __kernel_time_t is always long __kernel_time_t is always long on PA-RISC, irrespective of CONFIG_64BIT, hence move it out of the #ifdef CONFIG_64BIT / #else / #endif block. Signed-off-by: Geert Uytterhoeven Signed-off-by: Kyle McMartin --- arch/parisc/include/asm/posix_types.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'arch/parisc') diff --git a/arch/parisc/include/asm/posix_types.h b/arch/parisc/include/asm/posix_types.h index bb725a6..00da29a 100644 --- a/arch/parisc/include/asm/posix_types.h +++ b/arch/parisc/include/asm/posix_types.h @@ -24,13 +24,12 @@ typedef int __kernel_daddr_t; typedef unsigned long __kernel_size_t; typedef long __kernel_ssize_t; typedef long __kernel_ptrdiff_t; -typedef long __kernel_time_t; #else typedef unsigned int __kernel_size_t; typedef int __kernel_ssize_t; typedef int __kernel_ptrdiff_t; -typedef long __kernel_time_t; #endif +typedef long __kernel_time_t; typedef char * __kernel_caddr_t; typedef unsigned short __kernel_uid16_t; -- cgit v1.1 From 7a3f5134a8f5bd7fa38b5645eef05e8a4eb62951 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Wed, 26 Nov 2008 12:46:22 -0800 Subject: parisc: fix kernel crash when unwinding a userspace process Any user on existing parisc 32- and 64bit-kernels can easily crash the kernel and as such enforce a DSO. A simple testcase is available here: http://gsyprf10.external.hp.com/~deller/crash.tgz The problem is introduced by the fact, that the handle_interruption() crash handler calls the show_regs() function, which in turn tries to unwind the stack by calling parisc_show_stack(). Since the stack contains userspace addresses, a try to unwind the stack is dangerous and useless and leads to the crash. The fix is trivial: For userspace processes a) avoid to unwind the stack, and b) avoid to resolve userspace addresses to kernel symbol names. While touching this code, I converted print_symbol() to %pS printk formats and made parisc_show_stack() static. An initial patch for this was written by Kyle McMartin back in August: http://marc.info/?l=linux-parisc&m=121805168830283&w=2 Compile and run-tested with a 64bit parisc kernel. Signed-off-by: Helge Deller Cc: Grant Grundler Cc: Matthew Wilcox Cc: [2.6.25.x, 2.6.26.x, 2.6.27.x, earlier...] Signed-off-by: Andrew Morton Signed-off-by: Kyle McMartin --- arch/parisc/kernel/traps.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) (limited to 'arch/parisc') diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c index 675f1d0..4c771cd 100644 --- a/arch/parisc/kernel/traps.c +++ b/arch/parisc/kernel/traps.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include @@ -51,7 +50,7 @@ DEFINE_SPINLOCK(pa_dbit_lock); #endif -void parisc_show_stack(struct task_struct *t, unsigned long *sp, +static void parisc_show_stack(struct task_struct *task, unsigned long *sp, struct pt_regs *regs); static int printbinary(char *buf, unsigned long x, int nbits) @@ -121,18 +120,19 @@ static void print_fr(char *level, struct pt_regs *regs) void show_regs(struct pt_regs *regs) { - int i; + int i, user; char *level; unsigned long cr30, cr31; - level = user_mode(regs) ? KERN_DEBUG : KERN_CRIT; + user = user_mode(regs); + level = user ? KERN_DEBUG : KERN_CRIT; print_gr(level, regs); for (i = 0; i < 8; i += 4) PRINTREGS(level, regs->sr, "sr", RFMT, i); - if (user_mode(regs)) + if (user) print_fr(level, regs); cr30 = mfctl(30); @@ -145,14 +145,18 @@ void show_regs(struct pt_regs *regs) printk("%s CPU: %8d CR30: " RFMT " CR31: " RFMT "\n", level, current_thread_info()->cpu, cr30, cr31); printk("%s ORIG_R28: " RFMT "\n", level, regs->orig_r28); - printk(level); - print_symbol(" IAOQ[0]: %s\n", regs->iaoq[0]); - printk(level); - print_symbol(" IAOQ[1]: %s\n", regs->iaoq[1]); - printk(level); - print_symbol(" RP(r2): %s\n", regs->gr[2]); - - parisc_show_stack(current, NULL, regs); + + if (user) { + printk("%s IAOQ[0]: " RFMT "\n", level, regs->iaoq[0]); + printk("%s IAOQ[1]: " RFMT "\n", level, regs->iaoq[1]); + printk("%s RP(r2): " RFMT "\n", level, regs->gr[2]); + } else { + printk("%s IAOQ[0]: %pS\n", level, (void *) regs->iaoq[0]); + printk("%s IAOQ[1]: %pS\n", level, (void *) regs->iaoq[1]); + printk("%s RP(r2): %pS\n", level, (void *) regs->gr[2]); + + parisc_show_stack(current, NULL, regs); + } } @@ -173,20 +177,15 @@ static void do_show_stack(struct unwind_frame_info *info) break; if (__kernel_text_address(info->ip)) { - printk("%s [<" RFMT ">] ", (i&0x3)==1 ? KERN_CRIT : "", info->ip); -#ifdef CONFIG_KALLSYMS - print_symbol("%s\n", info->ip); -#else - if ((i & 0x03) == 0) - printk("\n"); -#endif + printk(KERN_CRIT " [<" RFMT ">] %pS\n", + info->ip, (void *) info->ip); i++; } } - printk("\n"); + printk(KERN_CRIT "\n"); } -void parisc_show_stack(struct task_struct *task, unsigned long *sp, +static void parisc_show_stack(struct task_struct *task, unsigned long *sp, struct pt_regs *regs) { struct unwind_frame_info info; -- cgit v1.1 From 90f671301a5e2678cdc99f611cd842161c3bb87f Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Fri, 7 Nov 2008 01:42:46 +0100 Subject: parisc: struct device - replace bus_id with dev_name(), dev_set_name() (I did not compile or test it, please let me know, or help fixing it, if something is wrong with the conversion) This patch is part of a larger patch series which will remove the "char bus_id[20]" name string from struct device. The device name is managed in the kobject anyway, and without any size limitation, and just needlessly copied into "struct device". To set and read the device name dev_name(dev) and dev_set_name(dev) must be used. If your code uses static kobjects, which it shouldn't do, "const char *init_name" can be used to statically provide the name the registered device should have. At registration time, the init_name field is cleared, to enforce the use of dev_name(dev) to access the device name at a later time. We need to get rid of all occurrences of bus_id in the entire tree to be able to enable the new interface. Please apply this patch, and possibly convert any remaining remaining occurrences of bus_id. We want to submit a patch to -next, which will remove bus_id from "struct device", to find the remaining pieces to convert, and finally switch over to the new api, which will remove the 20 bytes array and does no longer have a size limitation. Thanks, Kay Cc: Matthew Wilcox Cc: linux-parisc@vger.kernel.org Acked-by: Greg Kroah-Hartman Signed-off-by: Kay Sievers Signed-off-by: Kyle McMartin --- arch/parisc/include/asm/parisc-device.h | 4 ++-- arch/parisc/kernel/drivers.c | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'arch/parisc') diff --git a/arch/parisc/include/asm/parisc-device.h b/arch/parisc/include/asm/parisc-device.h index 7aa13f2..9afdad6 100644 --- a/arch/parisc/include/asm/parisc-device.h +++ b/arch/parisc/include/asm/parisc-device.h @@ -42,9 +42,9 @@ struct parisc_driver { #define to_parisc_driver(d) container_of(d, struct parisc_driver, drv) #define parisc_parent(d) to_parisc_device(d->dev.parent) -static inline char *parisc_pathname(struct parisc_device *d) +static inline const char *parisc_pathname(struct parisc_device *d) { - return d->dev.bus_id; + return dev_name(&d->dev); } static inline void diff --git a/arch/parisc/kernel/drivers.c b/arch/parisc/kernel/drivers.c index 2ca654b..884b7ce 100644 --- a/arch/parisc/kernel/drivers.c +++ b/arch/parisc/kernel/drivers.c @@ -43,7 +43,7 @@ struct hppa_dma_ops *hppa_dma_ops __read_mostly; EXPORT_SYMBOL(hppa_dma_ops); static struct device root = { - .bus_id = "parisc", + .init_name = "parisc", }; static inline int check_dev(struct device *dev) @@ -393,7 +393,8 @@ EXPORT_SYMBOL(print_pci_hwpath); static void setup_bus_id(struct parisc_device *padev) { struct hardware_path path; - char *output = padev->dev.bus_id; + char name[20]; + char *output = name; int i; get_node_path(padev->dev.parent, &path); @@ -404,6 +405,7 @@ static void setup_bus_id(struct parisc_device *padev) output += sprintf(output, "%u:", (unsigned char) path.bc[i]); } sprintf(output, "%u", (unsigned char) padev->hw_path); + dev_set_name(&padev->dev, name); } struct parisc_device * create_tree_node(char id, struct device *parent) -- cgit v1.1 From 96b8936a9ed08746e47081458a5eb9e43a751e24 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 25 Nov 2008 08:10:03 +0100 Subject: remove __ARCH_WANT_COMPAT_SYS_PTRACE All architectures now use the generic compat_sys_ptrace, as should every new architecture that needs 32bit compat (if we'll ever get another). Remove the now superflous __ARCH_WANT_COMPAT_SYS_PTRACE define, and also kill a comment about __ARCH_SYS_PTRACE that was added after __ARCH_SYS_PTRACE was already gone. Signed-off-by: Christoph Hellwig Acked-by: David S. Miller Signed-off-by: Linus Torvalds --- arch/parisc/include/asm/ptrace.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'arch/parisc') diff --git a/arch/parisc/include/asm/ptrace.h b/arch/parisc/include/asm/ptrace.h index afa5333..302f68d 100644 --- a/arch/parisc/include/asm/ptrace.h +++ b/arch/parisc/include/asm/ptrace.h @@ -47,8 +47,6 @@ struct pt_regs { #define task_regs(task) ((struct pt_regs *) ((char *)(task) + TASK_REGS)) -#define __ARCH_WANT_COMPAT_SYS_PTRACE - struct task_struct; #define arch_has_single_step() 1 void user_disable_single_step(struct task_struct *task); -- cgit v1.1