Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Thu, Sep 10, 2015 at 10:53:43PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > > I've been wondering whether we can teach GCC that set_domain modifies > > the value that get_domain returns, rather than throwing a volatile > > onto the asm in get_domain. The issue with a volatile there is that > > even if the result is unused, but the code is reachable, gcc still has > > to output the code to read the register. > > > > We might be able to get away with a memory clobber on the set_domain, > > and fake a memory read in get_domain, eg, by passing > > "m" (current_thread_info()->cpu_domain)) > > to the get_domain asm. > Ok, let's say we do it that way. > > I have some concerns about it: > (a) I see an inbalance, as set_domain() doesn't actually modify > current_thread_info()->cpu_domain. I don't see how it will protect use > from this scenario : > - get_domain() > - set_domain() > - set_domain() That should be fine, because if you've only got one get_domain(), then you only get the value of the DACR once. > (b) domain.h is included from thread_info.h, not the other way around > => current_thread_info() is not accessible from domain.h > => that would require a bit of moving things around, as thread_info > structure description should be available for example. > This is currently my biggest problem with this approach. It's not a problem since 1eef5d2f1b46 removed the need for domain.h to be included by thread_info.h - the existing include can be dropped. > (c) I was also wondering if a case like this could happen : > - a function foo() does a get_domain() >=> an exception/irq whatever happens and modifies the DACR We always preserve the value of DACR across an exception. > - foo() continues a makes a modify_domain() >=> and here the modify_domain() uses the old DACR value > Or said differently, I wonder if there is a case of 2 get_domain() calls > in a row with a DACR modification in between. I > > What about something such as [1], without a memory clobber, but a "fake" > memory > variable link ? The problem is the compiler will need to issue instructions to arrange for the address of this variable to end up in registers even though the assembly doesn't use it. That's true of my suggestion as well, but looking at the callsites, we generally already have, or very shortly there-after have the current thread_info address in a register. Patches to follow - I've not been able to confirm the instruction ordering you've observed with my compiler, so I can't prove whether this solves the problem locally. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: > I've been wondering whether we can teach GCC that set_domain modifies > the value that get_domain returns, rather than throwing a volatile > onto the asm in get_domain. The issue with a volatile there is that > even if the result is unused, but the code is reachable, gcc still has > to output the code to read the register. > > We might be able to get away with a memory clobber on the set_domain, > and fake a memory read in get_domain, eg, by passing > "m" (current_thread_info()->cpu_domain)) > to the get_domain asm. Ok, let's say we do it that way. I have some concerns about it: (a) I see an inbalance, as set_domain() doesn't actually modify current_thread_info()->cpu_domain. I don't see how it will protect use from this scenario : - get_domain() - set_domain() - set_domain() (b) domain.h is included from thread_info.h, not the other way around => current_thread_info() is not accessible from domain.h => that would require a bit of moving things around, as thread_info structure description should be available for example. This is currently my biggest problem with this approach. (c) I was also wondering if a case like this could happen : - a function foo() does a get_domain() => an exception/irq whatever happens and modifies the DACR - foo() continues a makes a modify_domain() => and here the modify_domain() uses the old DACR value Or said differently, I wonder if there is a case of 2 get_domain() calls in a row with a DACR modification in between. I What about something such as [1], without a memory clobber, but a "fake" memory variable link ? Cheers. -- Robert [1] get_domain() / set_domain() link diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h index e878129f2fee..fc1d9c43aa08 100644 --- a/arch/arm/include/asm/domain.h +++ b/arch/arm/include/asm/domain.h @@ -83,13 +83,17 @@ #ifndef __ASSEMBLY__ +static int domain_barrier; +/* + * how to get the current stack pointer in C + */ static inline unsigned int get_domain(void) { unsigned int domain; asm( "mrcp15, 0, %0, c3, c0 @ get domain" -: "=r" (domain)); +: "=r" (domain), "=m" (domain_barrier)); return domain; } @@ -97,8 +101,8 @@ static inline unsigned int get_domain(void) static inline void set_domain(unsigned val) { asm volatile( - "mcrp15, 0, %0, c3, c0 @ set domain" - : : "r" (val)); + "mcrp15, 0, %1, c3, c0 @ set domain" + : "=m" (domain_barrier) : "r" (val)); isb(); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Thu, Sep 10, 2015 at 09:01:41PM +0200, Robert Jarzmik wrote: > And I have the proof of gcc optimization, which I'll add to the commit message > if you want : > 0728 : > ... > 770: ee134f10mrc 15, 0, r4, cr3, cr0, {0} > ... no r4 or mrc/mcr usage > 788: e3842030orr r2, r4, #48 ; 0x30 > ... no r2/r4 or mrc/mcr usage > 794: ee032f10mcr 15, 0, r2, cr3, cr0, {0} > 798: ee07cf95mcr 15, 0, ip, cr7, cr5, {4} > ... no r4 or mrc/mcr usage > 7ac: e3c4300cbic r3, r4, #12 > 7b0: e3833004orr r3, r3, #4 > 7b4: ee033f10mcr 15, 0, r3, cr3, cr0, {0} > ... no mrc/mcr usage > 7cc: ebfebl 0 > > Here, we have in probe_kernel_address() in do_alignment(): > - @770 : r4 = DACR > - @794 : DACR = r4 | 0x30 > - @7b4 : DACR = (r4 & 0x0c) | 0x04 => the 0x30 is lost !!! > > I'll send my patch to the mailing list tomorrow, as well as the other one to > align the __bug_table session. I've been wondering whether we can teach GCC that set_domain modifies the value that get_domain returns, rather than throwing a volatile onto the asm in get_domain. The issue with a volatile there is that even if the result is unused, but the code is reachable, gcc still has to output the code to read the register. We might be able to get away with a memory clobber on the set_domain, and fake a memory read in get_domain, eg, by passing "m" (current_thread_info()->cpu_domain)) to the get_domain asm. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Robert Jarzmik writes: > Russell King - ARM Linux writes: > >> On Tue, Sep 08, 2015 at 07:01:00PM +0200, Robert Jarzmik wrote: >>> Russell King - ARM Linux writes: >> At the point we call into this code, the DACR should be 0x75, which >> should allow us to read the instruction at 0xbf00202c. But this is >> failing with a permission error - which it would do if it thought >> the kernel domain was in manager mode (iow, 0x55). > > Okay Russell, I have a good idea what's happening now. Basically, it boils > down > to compiler optimization of get_domain() which is called twice (set_fs() -> > modify_domain() -> get_domain()). See the piece in [1] for a more complete > explanation. > > I still haven't finished my work, as I need to disassemble the do_alignment() And I have the proof of gcc optimization, which I'll add to the commit message if you want : 0728 : ... 770: ee134f10mrc 15, 0, r4, cr3, cr0, {0} ... no r4 or mrc/mcr usage 788: e3842030orr r2, r4, #48 ; 0x30 ... no r2/r4 or mrc/mcr usage 794: ee032f10mcr 15, 0, r2, cr3, cr0, {0} 798: ee07cf95mcr 15, 0, ip, cr7, cr5, {4} ... no r4 or mrc/mcr usage 7ac: e3c4300cbic r3, r4, #12 7b0: e3833004orr r3, r3, #4 7b4: ee033f10mcr 15, 0, r3, cr3, cr0, {0} ... no mrc/mcr usage 7cc: ebfebl 0 Here, we have in probe_kernel_address() in do_alignment(): - @770 : r4 = DACR - @794 : DACR = r4 | 0x30 - @7b4 : DACR = (r4 & 0x0c) | 0x04 => the 0x30 is lost !!! I'll send my patch to the mailing list tomorrow, as well as the other one to align the __bug_table session. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: > On Tue, Sep 08, 2015 at 07:01:00PM +0200, Robert Jarzmik wrote: >> Russell King - ARM Linux writes: > At the point we call into this code, the DACR should be 0x75, which > should allow us to read the instruction at 0xbf00202c. But this is > failing with a permission error - which it would do if it thought > the kernel domain was in manager mode (iow, 0x55). Okay Russell, I have a good idea what's happening now. Basically, it boils down to compiler optimization of get_domain() which is called twice (set_fs() -> modify_domain() -> get_domain()). See the piece in [1] for a more complete explanation. I still haven't finished my work, as I need to disassemble the do_alignment() function to confirm the DACR read by get_domain() is only done once in the probe_kernel_address() call, and yet my hopes are high this is the cause as I traced the DACR modifications, which led me to : [-0] 0xc0017080: set_domain(0x0055) => dacr = 0x0055 => second set_domain() [-1] 0xc0017080: set_domain(0x0071) => dacr = 0x0071 => first set_domain() [-2] 0xc008a124: set_domain(0x0051) => dacr = 0x0051 Once I have my disassembly properly analyzed (ie. the second set_fs() doesn't do a mrc instruction), I'll have my proof. My setup is too full of traces and attempts to stall pipeline/prefetch to conclude yet, but there is a fair chance I'm closer to the solution now. Cheers. -- Robert [1] Current patch = ---8<--- >From 07cdda877b1b0c4fcdba3d756f6a22ce035ee672 Mon Sep 17 00:00:00 2001 From: Robert Jarzmik Date: Thu, 10 Sep 2015 00:32:26 +0200 Subject: [PATCH] ARM: fix domain access On the pxa310 platform, unaligned accesses are not fixed anymore with the SW_DOMAIN_PAN configuration activated. The trouble is that probe_kernel_address() cannot read the faulting instruction, because of a permission error. The permission error comes from the DACR register, whose value is incorrectly set to 0x55 instead of 0x75. I appears that at least gcc 4.8.2 optmizes the do_alignment() function, and inside probe_kernel_address(), where set_fs() is called twice, ie. modify_domain() is called twice, the get_domain() is called only once, and the first call's value is reused. As a consequence, instead of changing DACR as 0x51 -> 0x71 -> 0x75, it instead does 0x51 -> 0x71 -> 0x55. This is because instead of doing the last transition as : - (as (0x71 & ~0x03) | 0x04 = 0x75) it does - (as (0x51 & ~0x03) | 0x04 = 0x55) As a consequence, the alignment fault cannot be fixed, and triggers an Oops: Unable to handle kernel paging request at virtual address e1c20ec3 pgd = e1ce4000 [e1c20ec3] *pgd=c1c0044e(bad) Internal error: Oops: 823 [#1] ARM Modules linked in: unalign(+) CPU: 0 PID: 610 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #946 Hardware name: CM-X300 module task: e1c69500 ti: e1cc task.ti: e1cc PC is at u_init+0x2c/0x40 [unalign] LR is at u_init+0x14/0x40 [unalign] pc : []lr : []psr: a013 sp : e1cc1df0 ip : e1c20640 fp : 1e3dffdc r10: 0001 r9 : e1c20040 r8 : r7 : bf002000 r6 : e1c5ee80 r5 : c0be5b80 r4 : c0be5b80 r3 : e1c20ec0 r2 : 0004 r1 : a013 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 397f Table: c1ce4018 DAC: 0051 Process insmod (pid: 610, stack limit = 0xe1cc0198) Stack: (0xe1cc1df0 to 0xe1cc2000) Signed-off-by: Robert Jarzmik --- arch/arm/include/asm/domain.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h index 10c9a38636ac..47f3b8445237 100644 --- a/arch/arm/include/asm/domain.h +++ b/arch/arm/include/asm/domain.h @@ -87,7 +87,7 @@ static inline unsigned int get_domain(void) { unsigned int domain; - asm( + asm volatile( "mrcp15, 0, %0, c3, c0 @ get domain" : "=r" (domain)); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: > What should happen is: Thanks very much for the explanation, hopefully I have enough material to fly on my own now. > Now, when you get the fault inside arm_copy_from_user(), you can > print the DACR value saved at the time the fault was generated by > printing the word above struct pt_regs on the stack - add to > arch/arm/mm/fault.c:do_DataAbort(): > > if (addr == 0xbf00202c) printk("DACR=0x%08x\n", *(u32 *)(regs + 1)); > > before the "if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))" line. > That'll tell us what the DACR register was when we saved it. > > If it isn't 0x75, then the next part of the puzzle is going to be > working out why it isn't. It's 0x55. I'll track down how this happens, there are not that many places where DACR is touched, and I'm in a very controlled environement, so I can cunningly place JTAG breakpoints and watch DACR. I'll report once I have a better idea what is happening, that might take me a couple of days given that most of my workforce is available on weekends only. Thanks again for the free lesson. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Tue, Sep 08, 2015 at 07:01:00PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > >> Gah, silly me. But even with [1], I still get an error [2]. I have a > >> confirmation that I have a "Page Permission" fault on the > >> probe_kernel_address(). > > > > Hmm, that's not right. If it's the DACR, then it should be a page domain > > fault, not a page permission fault. > > > >> [2] Oops > >> > >> # insmod /tmp/unalign.ko > >> RJK1: fsr=23 far=e1c23643 dacr=51 > >> RJK2: fsr=23 far=e1c23643 dacr=51 > >> RJK3: fsr=2f far=bf00202c dacr=51 > >> RJK: fault=4 instr=0x instrptr=bf00202c > > > > Can you add a show_pte(current->mm, instrptr) to dump those page > > table entries please? > Most certainly, here we go : > > # insmod /tmp/unalign.ko > RJK1: fsr=23 far=e1c1f743 dacr=51 > RJK2: fsr=23 far=e1c1f743 dacr=51 > pgd = e1cc4000 > [bf00202c] *pgd=c1cab851, *pte=c1cb504f, *ppte=c1cb501f > RJK3: fsr=2f far=bf00202c dacr=51 > RJK4: fault=4 instr=0x instrptr=bf00202c > pgd = e1cc4000 > [bf00202c] *pgd=c1cab851, *pte=c1cb504f, *ppte=c1cb501f Okay, so domain = 2 (which for an Xscale 3 kernel is DOMAIN_KERNEL). The page table entry has AP=01, which is user no-access, svc read/write. What should happen is: #define probe_kernel_address(addr, retval) \ ({ \ long ret; \ mm_segment_t old_fs = get_fs(); \ \ set_fs(KERNEL_DS); \ This should update the DACR from 0x51 to 0x71 - switching domain 2 to manager mode. pagefault_disable();\ ret = __copy_from_user_inatomic(&(retval), \ (__force typeof(retval) __user *)(addr), \ sizeof(retval)); \ ... __copy_from_user_inatomic is an alias for __copy_from_user: static inline unsigned long __must_check __copy_from_user(void *to, const void __user *from, unsigned long n) { unsigned int __ua_flags = uaccess_save_and_enable(); and uaccess_save_and_enable() does: unsigned int old_domain = get_domain(); /* Set the current domain access to permit user accesses */ set_domain((old_domain & ~domain_mask(DOMAIN_USER)) | domain_val(DOMAIN_USER, DOMAIN_CLIENT)); So this should then end up changing the DACR from 0x71 to 0x75, enabling user access (because this function may access userspace.) What this means is that (going back to __copy_from_user): n = arm_copy_from_user(to, from, n); At the point we call into this code, the DACR should be 0x75, which should allow us to read the instruction at 0xbf00202c. But this is failing with a permission error - which it would do if it thought the kernel domain was in manager mode (iow, 0x55). I think some debugging in the above areas is needed - but I can't see anything wrong. If something were wrong, I'm pretty sure we'd have every pre-ARMv6 machine exploding right now because of it - and oopses wouldn't work. You're clearly getting oopses reported correctly, which rather proves out this code path. Now, when you get the fault inside arm_copy_from_user(), you can print the DACR value saved at the time the fault was generated by printing the word above struct pt_regs on the stack - add to arch/arm/mm/fault.c:do_DataAbort(): if (addr == 0xbf00202c) printk("DACR=0x%08x\n", *(u32 *)(regs + 1)); before the "if (!inf->fn(addr, fsr & ~FSR_LNX_PF, regs))" line. That'll tell us what the DACR register was when we saved it. If it isn't 0x75, then the next part of the puzzle is going to be working out why it isn't. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: >> Gah, silly me. But even with [1], I still get an error [2]. I have a >> confirmation that I have a "Page Permission" fault on the >> probe_kernel_address(). > > Hmm, that's not right. If it's the DACR, then it should be a page domain > fault, not a page permission fault. > >> [2] Oops >> >> # insmod /tmp/unalign.ko >> RJK1: fsr=23 far=e1c23643 dacr=51 >> RJK2: fsr=23 far=e1c23643 dacr=51 >> RJK3: fsr=2f far=bf00202c dacr=51 >> RJK: fault=4 instr=0x instrptr=bf00202c > > Can you add a show_pte(current->mm, instrptr) to dump those page > table entries please? Most certainly, here we go : # insmod /tmp/unalign.ko RJK1: fsr=23 far=e1c1f743 dacr=51 RJK2: fsr=23 far=e1c1f743 dacr=51 pgd = e1cc4000 [bf00202c] *pgd=c1cab851, *pte=c1cb504f, *ppte=c1cb501f RJK3: fsr=2f far=bf00202c dacr=51 RJK4: fault=4 instr=0x instrptr=bf00202c pgd = e1cc4000 [bf00202c] *pgd=c1cab851, *pte=c1cb504f, *ppte=c1cb501f Unable to handle kernel paging request at virtual address e1c1f743 pgd = e1cc4000 [e1c1f743] *pgd=c1c0044e(bad) Internal error: Oops: 823 [#1] ARM Modules linked in: unalign(+) CPU: 0 PID: 608 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #926 Hardware name: CM-X300 module task: e1c68380 ti: e1c84000 task.ti: e1c84000 PC is at u_init+0x2c/0x40 [unalign] LR is at u_init+0x14/0x40 [unalign] pc : []lr : []psr: a013 sp : e1c85df8 ip : e1c1f700 fp : 1e3e041c r10: e1c1fc00 r9 : 0001 r8 : r7 : bf002000 r6 : e1cad660 r5 : c0b85b80 r4 : c0b85b80 r3 : e1c1f740 r2 : 0004 r1 : a013 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 397f Table: c1cc4018 DAC: 0051 Process insmod (pid: 608, stack limit = 0xe1c84198) It happens on both mioa701(pxa270) and cm-x300(pxa310), with the same cross-compiler+host and kernel source. Yet doesn't happen on zylonite(pxa310), but different cross-compiler+host. I'll try to have a single kernel (binary) tried over the cm-x300 and zylonite to cross-check. Cheers. -- Robert PS: unalign.ko is a module which does a p=kmalloc(4096), then dereferences *(p+3) [1] Personal memo: memory pagetables # cat /sys/kernel/debug/kernel_page_tables ---[ Modules ]--- 0xbf00-0xbf001000 4K RW x MEM/CACHED/WBRA 0xbf002000-0xbf003000 4K RW x MEM/CACHED/WBRA ---[ Kernel Mapping ]--- 0xc000-0xc400 64M RW x 0xe000-0xe400 64M RW x ---[ vmalloc() Area ]--- 0xe4804000-0xe4844000 256K RW NX SO/UNCACHED 0xe4845000-0xe485 44K RW NX MEM/CACHED/WBRA 0xe485a000-0xe485b000 4K RW NX SHD DEV/SHARED 0xe485c000-0xe485d000 4K RW NX SHD DEV/SHARED 0xe485e000-0xe485f000 4K RW NX SO/UNCACHED 0xe486-0xe487 64K RW NX SHD DEV/SHARED 0xe487a000-0xe487d000 12K RW NX MEM/CACHED/WBRA 0xe488-0xe48c 256K RW NX SHD DEV/SHARED 0xe48c1000-0xe4903000 264K RW NX MEM/CACHED/WBRA 0xe4904000-0xe491e000 104K RW NX SO/UNCACHED 0xe49a-0xe49b 64K RW NX SHD DEV/SHARED 0xe49b1000-0xe49d5000 144K RW NX MEM/CACHED/WBRA 0xe49d6000-0xe49e1000 44K RW NX MEM/CACHED/WBRA 0xf200-0xf400 32M RW x SHD 0xf600-0xf620 2M RW x SHD 0xf620-0xf6201000 4K RW NX SHD DEV/SHARED 0xf630-0xf640 1M RW x SHD ---[ vmalloc() End ]--- ---[ Fixmap Area ]--- ---[ Vectors ]--- 0x-0x1000 4K USR ro x MEM/CACHED/WBRA 0x1000-0x2000 4K ro x MEM/CACHED/WBRA ---[ Vectors End ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Sun, Sep 06, 2015 at 11:31:34PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > >> [1] Approach 1 : translation table sync > >> === > ... > > The important place is in arch/arm/include/asm/domain.h, which is where > > we manipulate the DACR within probe_kernel_address(). > > Gah, silly me. But even with [1], I still get an error [2]. I have a > confirmation that I have a "Page Permission" fault on the > probe_kernel_address(). Hmm, that's not right. If it's the DACR, then it should be a page domain fault, not a page permission fault. > [2] Oops > > # insmod /tmp/unalign.ko > RJK1: fsr=23 far=e1c23643 dacr=51 > RJK2: fsr=23 far=e1c23643 dacr=51 > RJK3: fsr=2f far=bf00202c dacr=51 > RJK: fault=4 instr=0x instrptr=bf00202c Can you add a show_pte(current->mm, instrptr) to dump those page table entries please? -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: >> [1] Approach 1 : translation table sync >> === ... > The important place is in arch/arm/include/asm/domain.h, which is where > we manipulate the DACR within probe_kernel_address(). Gah, silly me. But even with [1], I still get an error [2]. I have a confirmation that I have a "Page Permission" fault on the probe_kernel_address(). Next thing I'll check is if I can read the TLB cache for the code entry. It's a very instructive bug for me :) Cheers. -- Robert [1] Approach 1 : translation table sync + PrefetchFlush === diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h index 7bbf325a4f31..73d5ad456e32 100644 --- a/arch/arm/include/asm/assembler.h +++ b/arch/arm/include/asm/assembler.h @@ -449,6 +449,13 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) #endif .endm + .macro dacr_sync, rd + mrc p15, 0, \rd, c2, c0, 0 + mov \rd, \rd + sub pc, pc, #4 + mcr p15, 0, \rd, c7, c5, 4 + .endm + .macro uaccess_disable, tmp, isb=1 #ifdef CONFIG_CPU_SW_DOMAIN_PAN /* @@ -457,6 +464,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) */ mov \tmp, #DACR_UACCESS_DISABLE mcr p15, 0, \tmp, c3, c0, 0 @ Set domain register + dacr_sync \tmp .if \isb instr_sync .endif @@ -471,6 +479,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) */ mov \tmp, #DACR_UACCESS_ENABLE mcr p15, 0, \tmp, c3, c0, 0 + dacr_sync \tmp .if \isb instr_sync .endif @@ -488,6 +497,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) #ifdef CONFIG_CPU_SW_DOMAIN_PAN ldr r0, [sp, #S_FRAME_SIZE] mcr p15, 0, r0, c3, c0, 0 + dacr_sync r0 #endif .endm diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h index e878129f2fee..10c9a38636ac 100644 --- a/arch/arm/include/asm/domain.h +++ b/arch/arm/include/asm/domain.h @@ -97,7 +97,11 @@ static inline unsigned int get_domain(void) static inline void set_domain(unsigned val) { asm volatile( - "mcrp15, 0, %0, c3, c0 @ set domain" + "mcrp15, 0, %0, c3, c0; @ set domain\ + mrc p15, 0, %0, c2, c0, 0; \ + mov %0, %0; \ + sub pc, pc, #4; \ + mcr p15, 0, %0, c7, c5, 4" : : "r" (val)); isb(); } diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c index 9769f1eefe3b..c9c454129344 100644 --- a/arch/arm/mm/alignment.c +++ b/arch/arm/mm/alignment.c @@ -747,6 +747,27 @@ do_alignment_t32_to_handler(unsigned long *pinstr, struct pt_regs *regs, return NULL; } +static u32 far_read(void) +{ +u32 far; +asm("mrcp15, 0, %0, c6, c0, 0" : "=r" (far)); +return far; +} + +static u32 fsr_read(void) +{ +u32 fsr; +asm("mrcp15, 0, %0, c5, c0, 0" : "=r" (fsr)); +return fsr; +} + +static u32 dacr_read(void) +{ +u32 dacr; +asm("mrcp15, 0, %0, c3, c0, 0" : "=r" (dacr)); +return dacr; +} + static int do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) { @@ -763,6 +784,8 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) local_irq_enable(); instrptr = instruction_pointer(regs); + pr_info("RJK1: fsr=%x far=%x dacr=%x\n", fsr_read(), far_read(), dacr_read()); + pr_info("RJK2: fsr=%x far=%x dacr=%x\n", fsr_read(), far_read(), dacr_read()); if (thumb_mode(regs)) { u16 *ptr = (u16 *)(instrptr & ~1); @@ -787,6 +810,8 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) instr = __mem_to_opcode_arm(instr); } + pr_info("RJK3: fsr=%x far=%x dacr=%x\n", fsr_read(), far_read(), dacr_read()); + pr_info("RJK: fault=%d instr=0x%08x instrptr=%p\n", fault, instr, instrptr); if (fault) { type = TYPE_FAULT; goto bad_or_fault; [2] Oops # insmod /tmp/unalign.ko RJK1: fsr=23 far=e1c23643 dacr=51 RJK2: fsr=23 far=e1c23643 dacr=51 RJK3: fsr=2f far=bf00202c dacr=51 RJK: fault=4 instr=0x instrptr=bf00202c Unable to handle kernel paging request at virtual address e1c23643 pgd = e1cd4000 [e1c23643] *pgd=c1c0044e(bad) Internal error: Oops: 823 [#1] ARM Modules linked in: unalign(+) CPU: 0 PID: 608 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #919 Hardware name: CM-X300 module task: e1c69880 ti: e1cb task.ti: e1cb PC is at u_init+0x2c/0x40 [unalign] LR is at u_init+0x14/0x40 [unalign] pc : []lr : []psr: a013 sp : e1cb1df8 ip : e1c23e00 fp : 1e3dc89c r10: e1c2378
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Sun, Sep 06, 2015 at 07:25:01PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > > On Sat, Sep 05, 2015 at 07:10:49PM +0200, Robert Jarzmik wrote: > >> Russell King - ARM Linux writes: > >> So the issue is around this SW_DOMAIN_PAN, at least on PXA. > > > > If so, you may need to add: > > > > mrc p15, 0, \rd, c2, c0, 0 > > mov \rd, \rd > > sub pc, pc, #4 > > > > to the places we update the domain access register to ensure that the > > Xscale pipeline stalls to allow the CP15 DACR update to hit. > Nope, that didn't work. > I have tried 2 different patches : > - in [1], your proposed solution > - in [2], a PrefetchFlush as adviced by ARM Architecture Reference Manual > > None of them worked. I confirmed by disassembling __dabt_svc my changes hit > the > abort routine, and they did. I'll continue next week by trying to have a > closer > look at the SW_DOMAIN_PAN commit (a5e090acbf545), and take time to walk > through > the whole Oops for information I have missed. > > Cheers. > > -- > Robert > > [1] Approach 1 : translation table sync > === > diff --cc arch/arm/include/asm/assembler.h > index 7bbf325a4f31,7bbf325a4f31..6bb46198fd08 > --- a/arch/arm/include/asm/assembler.h > +++ b/arch/arm/include/asm/assembler.h > @@@ -449,6 -449,6 +449,12 @@@ THUMB(orr \reg , \reg , #PSR_T_BIT > > #endif > .endm > > ++ .macro dacr_sync, rd > ++ mrc p15, 0, \rd, c2, c0, 0 > ++ mov \rd, \rd > ++ sub pc, pc, #4 > ++ .endm > ++ > .macro uaccess_disable, tmp, isb=1 > #ifdef CONFIG_CPU_SW_DOMAIN_PAN > /* > @@@ -457,6 -457,6 +463,7 @@@ > */ > mov \tmp, #DACR_UACCESS_DISABLE > mcr p15, 0, \tmp, c3, c0, 0 @ Set domain register > ++ dacr_sync \tmp > .if \isb > instr_sync > .endif > @@@ -471,6 -471,6 +478,7 @@@ > */ > mov \tmp, #DACR_UACCESS_ENABLE > mcr p15, 0, \tmp, c3, c0, 0 > ++ dacr_sync \tmp > .if \isb > instr_sync > .endif > @@@ -488,6 -488,6 +496,7 @@@ > #ifdef CONFIG_CPU_SW_DOMAIN_PAN > ldr r0, [sp, #S_FRAME_SIZE] > mcr p15, 0, r0, c3, c0, 0 > ++ dacr_sync r0 > #endif > .endm The important place is in arch/arm/include/asm/domain.h, which is where we manipulate the DACR within probe_kernel_address(). -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: > On Sat, Sep 05, 2015 at 07:10:49PM +0200, Robert Jarzmik wrote: >> Russell King - ARM Linux writes: >> So the issue is around this SW_DOMAIN_PAN, at least on PXA. > > If so, you may need to add: > > mrc p15, 0, \rd, c2, c0, 0 > mov \rd, \rd > sub pc, pc, #4 > > to the places we update the domain access register to ensure that the > Xscale pipeline stalls to allow the CP15 DACR update to hit. Nope, that didn't work. I have tried 2 different patches : - in [1], your proposed solution - in [2], a PrefetchFlush as adviced by ARM Architecture Reference Manual None of them worked. I confirmed by disassembling __dabt_svc my changes hit the abort routine, and they did. I'll continue next week by trying to have a closer look at the SW_DOMAIN_PAN commit (a5e090acbf545), and take time to walk through the whole Oops for information I have missed. Cheers. -- Robert [1] Approach 1 : translation table sync === diff --cc arch/arm/include/asm/assembler.h index 7bbf325a4f31,7bbf325a4f31..6bb46198fd08 --- a/arch/arm/include/asm/assembler.h +++ b/arch/arm/include/asm/assembler.h @@@ -449,6 -449,6 +449,12 @@@ THUMB(orr \reg , \reg , #PSR_T_BIT #endif .endm ++ .macro dacr_sync, rd ++ mrc p15, 0, \rd, c2, c0, 0 ++ mov \rd, \rd ++ sub pc, pc, #4 ++ .endm ++ .macro uaccess_disable, tmp, isb=1 #ifdef CONFIG_CPU_SW_DOMAIN_PAN /* @@@ -457,6 -457,6 +463,7 @@@ */ mov \tmp, #DACR_UACCESS_DISABLE mcr p15, 0, \tmp, c3, c0, 0 @ Set domain register ++ dacr_sync \tmp .if \isb instr_sync .endif @@@ -471,6 -471,6 +478,7 @@@ */ mov \tmp, #DACR_UACCESS_ENABLE mcr p15, 0, \tmp, c3, c0, 0 ++ dacr_sync \tmp .if \isb instr_sync .endif @@@ -488,6 -488,6 +496,7 @@@ #ifdef CONFIG_CPU_SW_DOMAIN_PAN ldr r0, [sp, #S_FRAME_SIZE] mcr p15, 0, r0, c3, c0, 0 ++ dacr_sync r0 #endif .endm [2] Approach 2 : PrefetchFlush == diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h index 7bbf325a4f31..2152d43d7ede 100644 --- a/arch/arm/include/asm/assembler.h +++ b/arch/arm/include/asm/assembler.h @@ -449,6 +449,10 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) #endif .endm + .macro dacr_sync, rd + mcr p15, 0, \rd, c7, c5, 4 + .endm + .macro uaccess_disable, tmp, isb=1 #ifdef CONFIG_CPU_SW_DOMAIN_PAN /* @@ -457,6 +461,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) */ mov \tmp, #DACR_UACCESS_DISABLE mcr p15, 0, \tmp, c3, c0, 0 @ Set domain register + dacr_sync \tmp .if \isb instr_sync .endif @@ -471,6 +476,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) */ mov \tmp, #DACR_UACCESS_ENABLE mcr p15, 0, \tmp, c3, c0, 0 + dacr_sync \tmp .if \isb instr_sync .endif @@ -488,6 +494,7 @@ THUMB( orr \reg , \reg , #PSR_T_BIT) #ifdef CONFIG_CPU_SW_DOMAIN_PAN ldr r0, [sp, #S_FRAME_SIZE] mcr p15, 0, r0, c3, c0, 0 + dacr_sync r0 #endif .endm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: >> Moreover, this is consistent with the fact that this commit is in linux-next >> but >> not in v4.1 : >> a5e090acbf54 ("ARM: software-based priviledged-no-access support") >> >> So the issue is around this SW_DOMAIN_PAN, at least on PXA. > > Is it only PXA which seems to be affected? Sorry I don't know, I only own pxa platforms. > If so, you may need to add: > > mrc p15, 0, \rd, c2, c0, 0 > mov \rd, \rd > sub pc, pc, #4 > > to the places we update the domain access register to ensure that the > Xscale pipeline stalls to allow the CP15 DACR update to hit. Okay, I'll try that. By the way, the ARMv5 manual states in chapter "B4.5.1 MMU Fault" that for a DACR update, a "PrefetchFlush" operation has to be done (chapter B2.6.3 PrefetchFlush CP15 register 7), quoting : Changes to the Domain Access Control register are synchronized by performing a PrefetchFlush operation (or as result of an exception or exception return). See Changes to CP15 registers and the memory order model on page B2-24 for details. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Sat, Sep 05, 2015 at 07:10:49PM +0200, Robert Jarzmik wrote: > Russell King - ARM Linux writes: > > > On Sat, Sep 05, 2015 at 03:48:38PM +0200, Robert Jarzmik wrote: > >> This time I took my JTAG to have a look at the flow, in > >> arch/arm/mm/alignment.c, > >> where I added the small chunk in [2], which gave in my case : > >> RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 > >> tinstr=0x > > > > Right, so as fault is nonzero, this means that we were unable to read the > > instruction. That seems mad though - the instruction pointer is certainly > > valid, and as we're using probe_kernel_address(), that switches to the > > kernel "segment" before trying to read kernel addresses. That should > > mean that __copy_from_user_inatomic() is able to read the instruction. > > > > I think this is the root cause of the issue. > > And there is more madness to come : I tried to "reread" the instruction [1] a > second time if the first result was 4 : > RJK: fault=4 instr=0x(@c385d72c) instrptr=0xc02b39e8 thumb_mode=0 > tinstr=0x > RJK: reread instruction: [0xc02b39e8] = 0x10c650b2: 0 > > Guess what, the second probe_kernel_address() with the same parameters returns > 0, and everything works. It's insane. > > >> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > > It seems you have SW_DOMAIN_PAN enabled. > That's the default arch/arm/Kconfig implies. > And ... this is what also _is_ the cause of this behavior : removing > SW_DOMAIN_PAN makes all my pxa boards work again !!! > > Moreover, this is consistent with the fact that this commit is in linux-next > but > not in v4.1 : > a5e090acbf54 ("ARM: software-based priviledged-no-access support") > > So the issue is around this SW_DOMAIN_PAN, at least on PXA. Is it only PXA which seems to be affected? If so, you may need to add: mrc p15, 0, \rd, c2, c0, 0 mov \rd, \rd sub pc, pc, #4 to the places we update the domain access register to ensure that the Xscale pipeline stalls to allow the CP15 DACR update to hit. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Russell King - ARM Linux writes: > On Sat, Sep 05, 2015 at 03:48:38PM +0200, Robert Jarzmik wrote: >> This time I took my JTAG to have a look at the flow, in >> arch/arm/mm/alignment.c, >> where I added the small chunk in [2], which gave in my case : >> RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 >> tinstr=0x > > Right, so as fault is nonzero, this means that we were unable to read the > instruction. That seems mad though - the instruction pointer is certainly > valid, and as we're using probe_kernel_address(), that switches to the > kernel "segment" before trying to read kernel addresses. That should > mean that __copy_from_user_inatomic() is able to read the instruction. > > I think this is the root cause of the issue. And there is more madness to come : I tried to "reread" the instruction [1] a second time if the first result was 4 : RJK: fault=4 instr=0x(@c385d72c) instrptr=0xc02b39e8 thumb_mode=0 tinstr=0x RJK: reread instruction: [0xc02b39e8] = 0x10c650b2: 0 Guess what, the second probe_kernel_address() with the same parameters returns 0, and everything works. It's insane. >> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > It seems you have SW_DOMAIN_PAN enabled. That's the default arch/arm/Kconfig implies. And ... this is what also _is_ the cause of this behavior : removing SW_DOMAIN_PAN makes all my pxa boards work again !!! Moreover, this is consistent with the fact that this commit is in linux-next but not in v4.1 : a5e090acbf54 ("ARM: software-based priviledged-no-access support") So the issue is around this SW_DOMAIN_PAN, at least on PXA. -- Robert [1] @@ -787,6 +798,15 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) instr = __mem_to_opcode_arm(instr); } + pr_info("RJK: fault=%d instr=0x%08lx(@%p) instrptr=0x%08lx thumb_mode=%lu tinstr=0x%04x\n", + fault, instr, &instr, instrptr, thumb_mode(regs), tinstr); + if (fault == 4 && !thumb_mode(regs)) { + fault = probe_kernel_address(instrptr, instr); + pr_info("RJK: reread instruction: [0x%08lx] = 0x%08lx: %u\n", + instrptr, instr, fault); + rjk_debug_point(instrptr); + } + if (fault) { type = TYPE_FAULT; goto bad_or_fault; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Sat, Sep 05, 2015 at 03:48:38PM +0200, Robert Jarzmik wrote: > Dave Martin writes: > > > On Wed, Sep 02, 2015 at 08:23:29AM +0200, Robert Jarzmik wrote: > >> On old ARM chips, unaligned accesses to memory are not trapped and > >> fixed. On module load, symbols are relocated, and the relocation of > >> __bug_table symbols is done on a u32 basis. Yet the section is not > >> aligned to a multiple of 4 address, but to a multiple of 2. > > Hi Russell, > > I dug deeper, and got another stack, unrelated to modules insertion but > related > to alignement fault (see [1] for reference). As before, this didn't happen on > v4.1, but happens on linux-next. > I'm wondering if alignement fixup does work in my case and if I understand it. > > This time I took my JTAG to have a look at the flow, in > arch/arm/mm/alignment.c, > where I added the small chunk in [2], which gave in my case : > RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 > tinstr=0x Right, so as fault is nonzero, this means that we were unable to read the instruction. That seems mad though - the instruction pointer is certainly valid, and as we're using probe_kernel_address(), that switches to the kernel "segment" before trying to read kernel addresses. That should mean that __copy_from_user_inatomic() is able to read the instruction. I think this is the root cause of the issue. > The instruction (as seen with vmlinux disassembly or JTAG memory dump) is : > 0xc02b37c8 <+372>:b2 50 c6 10 strhne r5, [r6], #2 > > I must admit I fail to see how the following "fixup:" label can be reached > with > a "missed" copy_from_user() (fault == 4). We can't fix up the fault if we failed to read the instruction causing the fault - because we've no idea what register(s) will need updating. > [1] New stack > = > #0: pxa2xx-ac97 (Wolfson WM9713,WM9714) > RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 tinstr=0x > &Unable to handle kernel paging request at virtual address c3057661 > &pgd = c0004000 > "[c3057661] *pgd=a300040e(bad) > Internal error: Oops: 803 [#1] PREEMPT ARM > Modules linked in: > CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc8-next-20150828+ #900 > Hardware name: MIO A701 > task: c3858bc0 ti: c385c000 task.ti: c385c000 > PC is at doc_read_data_area+0x174/0x370 > LR is at doc_read_page_getbytes+0x58/0x78 > pc : []lr : []psr: a813 > sp : c385d8e0 ip : c07142b8 fp : c385d91c > r10: c3aac540 r9 : c070ffc0 r8 : c385c000 > @QGi > r7 : 0002 r6 : c3057661 r5 : c1e5 r4 : 000b > r3 : 000a r2 : 000b r1 : c3057661 r0 : > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none It seems you have SW_DOMAIN_PAN enabled. > Control: 397f Table: a0004000 DAC: 0053 And the DACR for the parent context shows that user no-access, kernel manager-access (iow, in the doc_read_data_area() function). I have to wonder why that would be the case - I can't find anything that would set the kernel to manager-access. There's no get_ds() or KERNEL_DS reference in fs/ubifs or drivers/mtd. > [3] Abort stack > === > #0 panic (fmt=0xc385d6c4 ) at kernel/panic.c:72 > #1 0xc000e788 in oops_end (regs=, signr=, > flags=) at arch/arm/kernel/traps.c:311 > #2 die (str=0x6813 "", regs=, err=-1014638958) at > arch/arm/kernel/traps.c:333 > #3 0xc00137c0 in __do_kernel_fault (mm=0xc06e8c04 , > addr=3271915105, fsr=2051, regs=0xc385d890) at arch/arm/mm/fault.c:150 > #4 0xc0013b10 in do_bad_area (addr=, fsr=, > regs=) at arch/arm/mm/fault.c:198 > #5 0xc00166c8 in do_alignment (addr=3271915105, fsr=2051, regs=0xc385d890) > at arch/arm/mm/alignment.c:900 > #6 0xc0009264 in do_DataAbort (addr=3271915105, fsr=2051, regs=0xc385d890) > at arch/arm/mm/fault.c:550 > #7 0xc000f024 in __dabt_svc () at arch/arm/kernel/entry-armv.S:204 I'm afraid this isn't useful, and I think (as seems to be typical with gdb) some of those values are completely wrong. There's no way "str" would be 0x6813 in frame 2 for example. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: fix alignement of __bug_table section entries
Dave Martin writes: > On Wed, Sep 02, 2015 at 08:23:29AM +0200, Robert Jarzmik wrote: >> On old ARM chips, unaligned accesses to memory are not trapped and >> fixed. On module load, symbols are relocated, and the relocation of >> __bug_table symbols is done on a u32 basis. Yet the section is not >> aligned to a multiple of 4 address, but to a multiple of 2. Hi Russell, I dug deeper, and got another stack, unrelated to modules insertion but related to alignement fault (see [1] for reference). As before, this didn't happen on v4.1, but happens on linux-next. I'm wondering if alignement fixup does work in my case and if I understand it. This time I took my JTAG to have a look at the flow, in arch/arm/mm/alignment.c, where I added the small chunk in [2], which gave in my case : RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 tinstr=0x The instruction (as seen with vmlinux disassembly or JTAG memory dump) is : 0xc02b37c8 <+372>: b2 50 c6 10 strhne r5, [r6], #2 I must admit I fail to see how the following "fixup:" label can be reached with a "missed" copy_from_user() (fault == 4). This is probably what happened to me with the modules __bug_table section, and it will continue to happen until I understand why this copy fails. It's really odd nobody but me faces this issue. Cheers. -- Robert [1] New stack = #0: pxa2xx-ac97 (Wolfson WM9713,WM9714) RJK: fault=4 instr=0x instrptr=0xc02b37c8 thumb_mode=0 tinstr=0x &Unable to handle kernel paging request at virtual address c3057661 &pgd = c0004000 "[c3057661] *pgd=a300040e(bad) Internal error: Oops: 803 [#1] PREEMPT ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc8-next-20150828+ #900 Hardware name: MIO A701 task: c3858bc0 ti: c385c000 task.ti: c385c000 PC is at doc_read_data_area+0x174/0x370 LR is at doc_read_page_getbytes+0x58/0x78 pc : []lr : []psr: a813 sp : c385d8e0 ip : c07142b8 fp : c385d91c r10: c3aac540 r9 : c070ffc0 r8 : c385c000 @QGi r7 : 0002 r6 : c3057661 r5 : c1e5 r4 : 000b r3 : 000a r2 : 000b r1 : c3057661 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 397f Table: a0004000 DAC: 0053 Process swapper (pid: 1, stack limit = 0xc385c198) Stack: (0xc385d8e0 to 0xc385e000) d8e0: c02b3a34 0001 000a c385d914 c3057660 000c c3aac540 ... chop chop ... dfe0: 0013 f3ff8cdd 4cf3d76f [] (doc_read_data_area) from [] (doc_read_page_getbytes+0x58/0x78) [] (doc_read_page_getbytes) from [] (doc_read_oob+0x22c/0x75c) [] (doc_read_oob) from [] (doc_read+0x64/0x74) [] (doc_read) from [] (part_read+0x58/0x9c) [] (part_read) from [] (mtd_read+0x88/0xbc) [] (mtd_read) from [] (ubi_io_read+0x16c/0x358) [] (ubi_io_read) from [] (ubi_eba_read_leb+0x384/0x4d4) [] (ubi_eba_read_leb) from [] (ubi_leb_read+0xd4/0x134) [] (ubi_leb_read) from [] (ubifs_leb_read+0x3c/0xa8)s [] (ubifs_leb_read) from [] (ubifs_read_nnode+0xec/0x200) [] (ubifs_read_nnode) from [] (ubifs_lpt_lookup_dirty+0x38/0x330) [] (ubifs_lpt_lookup_dirty) from [] (ubifs_replay_journal+0x3c/0x1b38) [] (ubifs_replay_journal) from [] (ubifs_mount+0x1444/0x2410) [] (ubifs_mount) from [] (mount_fs+0x24/0xb0) [] (mount_fs) from [] (vfs_kern_mount+0x58/0x124) [] (vfs_kern_mount) from [] (do_mount+0xb40/0xd10) [] (do_mount) from [] (SyS_mount+0x84/0xb0) [] (SyS_mount) from [] (mount_block_root+0x12c/0x2dc) [] (mount_block_root) from [] (prepare_namespace+0x98/0x1bc) [] (prepare_namespace) from [] (kernel_init_freeable+0x188/0x1d4) [] (kernel_init_freeable) from [] (kernel_init+0x18/0xfc) [] (kernel_init) from [] (ret_from_fork+0x14/0x28) Code: 1a60 e51b3030 e356 e2877002 (10c650b2) ---[ end trace a0bcca195299a22d ]--- [2] Debug patch === diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c index 2c0c541c60ca..b0897da5456c 100644 --- a/arch/arm/mm/alignment.c +++ b/arch/arm/mm/alignment.c @@ -787,6 +787,8 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs) instr = __mem_to_opcode_arm(instr); } + pr_info("RJK: fault=%d instr=0x%08lx instrptr=0x%08lx thumb_mode=%lu tinstr=0x%04x\n", + fault, instr, instrptr, thumb_mode(regs), tinstr); if (fault) { type = TYPE_FAULT; goto bad_or_fault; [3] Abort stack === #0 panic (fmt=0xc385d6c4 ) at kernel/panic.c:72 #1 0xc000e788 in oops_end (regs=, signr=, flags=) at arch/arm/kernel/traps.c:311 #2 die (str=0x6813 "", regs=, err=-1014638958) at arch/arm/kernel/traps.c:333 #3 0xc00137c0 in __do_kernel_fault (mm=0xc06e8c04 , addr=3271915105, fsr=2051, regs=0xc385d890) at arch/arm/mm/fault.c:150 #4 0xc0013b10 in do_bad_area (addr=, fsr=, regs=) at arch/arm/mm/fault.c:198 #5 0xc00166c8 in do_alignment (addr=3271915105, fsr=2051, regs=0xc385d890) at arch/arm/mm/alignment.c:9
Re: [PATCH] ARM: fix alignement of __bug_table section entries
On Wed, Sep 02, 2015 at 08:23:29AM +0200, Robert Jarzmik wrote: > On old ARM chips, unaligned accesses to memory are not trapped and > fixed. On module load, symbols are relocated, and the relocation of > __bug_table symbols is done on a u32 basis. Yet the section is not > aligned to a multiple of 4 address, but to a multiple of 2. > > This triggers an Oops on pxa architecture, where address 0xbf0021ea > is the first relocation in the __bug_table section : > apply_relocate(): pxa3xx_nand: section 13 reloc 0 sym '' > Unable to handle kernel paging request at virtual address bf0021ea > pgd = e1cd > [bf0021ea] *pgd=c1cce851, *pte=c1cde04f, *ppte=c1cde01f > Internal error: Oops: 23 [#1] ARM > Modules linked in: > CPU: 0 PID: 606 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ > #887 > Hardware name: CM-X300 module > task: e1c68700 ti: e1c3e000 task.ti: e1c3e000 > PC is at apply_relocate+0x2f4/0x3d4 > LR is at 0xbf0021ea > pc : []lr : []psr: 8013 > sp : e1c3fe30 ip : 6013 fp : e49e8c60 > r10: e49e8fa8 r9 : r8 : e49e7c58 > r7 : e49e8c38 r6 : e49e8a58 r5 : e49e8920 r4 : e49e8918 > r3 : bf0021ea r2 : bf007034 r1 : r0 : bf00 > Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 397f Table: c1cd0018 DAC: 0051 > Process insmod (pid: 606, stack limit = 0xe1c3e198) > [] (apply_relocate) from [] (load_module+0x1248/0x1f5c) > [] (load_module) from [] (SyS_init_module+0xe4/0x170) > [] (SyS_init_module) from [] (ret_fast_syscall+0x0/0x38) > > Fix this by ensuring entries in __bug_table are all aligned to at least > of multiple of 4. This transforms a module section __bug_table as : > - [12] __bug_table PROGBITS 002232 18 00 A 0 > 0 1 > + [12] __bug_table PROGBITS 002232 18 00 A 0 > 0 4 > > Signed-off-by: Robert Jarzmik > --- > arch/arm/include/asm/bug.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h > index b274bde24905..e7335a92144e 100644 > --- a/arch/arm/include/asm/bug.h > +++ b/arch/arm/include/asm/bug.h > @@ -40,6 +40,7 @@ do { > \ > "2:\t.asciz " #__file "\n" \ > ".popsection\n" \ > ".pushsection __bug_table,\"a\"\n" \ > + ".align 2\n"\ > "3:\t.word 1b, 2b\n"\ > "\t.hword " #__line ", 0\n" \ > ".popsection"); \ Reviewed-by: Dave Martin I added the .align in my recent patches implementing BUG for arm64, but didn't touch arch/arm. When referring to the arm code I did notice that there was no .align. I'd concluded that the linker script layout and lack of bug reports meant the arm code was alignment-safe in practice, but I guess I was mistaken... Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: fix alignement of __bug_table section entries
On old ARM chips, unaligned accesses to memory are not trapped and fixed. On module load, symbols are relocated, and the relocation of __bug_table symbols is done on a u32 basis. Yet the section is not aligned to a multiple of 4 address, but to a multiple of 2. This triggers an Oops on pxa architecture, where address 0xbf0021ea is the first relocation in the __bug_table section : apply_relocate(): pxa3xx_nand: section 13 reloc 0 sym '' Unable to handle kernel paging request at virtual address bf0021ea pgd = e1cd [bf0021ea] *pgd=c1cce851, *pte=c1cde04f, *ppte=c1cde01f Internal error: Oops: 23 [#1] ARM Modules linked in: CPU: 0 PID: 606 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #887 Hardware name: CM-X300 module task: e1c68700 ti: e1c3e000 task.ti: e1c3e000 PC is at apply_relocate+0x2f4/0x3d4 LR is at 0xbf0021ea pc : []lr : []psr: 8013 sp : e1c3fe30 ip : 6013 fp : e49e8c60 r10: e49e8fa8 r9 : r8 : e49e7c58 r7 : e49e8c38 r6 : e49e8a58 r5 : e49e8920 r4 : e49e8918 r3 : bf0021ea r2 : bf007034 r1 : r0 : bf00 Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 397f Table: c1cd0018 DAC: 0051 Process insmod (pid: 606, stack limit = 0xe1c3e198) [] (apply_relocate) from [] (load_module+0x1248/0x1f5c) [] (load_module) from [] (SyS_init_module+0xe4/0x170) [] (SyS_init_module) from [] (ret_fast_syscall+0x0/0x38) Fix this by ensuring entries in __bug_table are all aligned to at least of multiple of 4. This transforms a module section __bug_table as : - [12] __bug_table PROGBITS 002232 18 00 A 0 0 1 + [12] __bug_table PROGBITS 002232 18 00 A 0 0 4 Signed-off-by: Robert Jarzmik --- arch/arm/include/asm/bug.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h index b274bde24905..e7335a92144e 100644 --- a/arch/arm/include/asm/bug.h +++ b/arch/arm/include/asm/bug.h @@ -40,6 +40,7 @@ do { \ "2:\t.asciz " #__file "\n" \ ".popsection\n" \ ".pushsection __bug_table,\"a\"\n" \ + ".align 2\n"\ "3:\t.word 1b, 2b\n"\ "\t.hword " #__line ", 0\n" \ ".popsection"); \ -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/