Re: [PATCH] ARM: fix alignement of __bug_table section entries

2015-09-11 Thread Russell King - ARM Linux
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

2015-09-10 Thread Robert Jarzmik
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

2015-09-10 Thread Russell King - ARM Linux
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

2015-09-10 Thread Robert Jarzmik
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

2015-09-09 Thread Robert Jarzmik
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

2015-09-08 Thread Robert Jarzmik
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

2015-09-08 Thread Russell King - ARM Linux
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

2015-09-08 Thread Robert Jarzmik
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

2015-09-06 Thread Russell King - ARM Linux
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

2015-09-06 Thread Robert Jarzmik
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

2015-09-06 Thread Russell King - ARM Linux
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

2015-09-06 Thread Robert Jarzmik
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

2015-09-05 Thread Robert Jarzmik
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

2015-09-05 Thread Russell King - ARM Linux
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

2015-09-05 Thread Robert Jarzmik
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

2015-09-05 Thread Russell King - ARM Linux
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

2015-09-05 Thread Robert Jarzmik
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

2015-09-02 Thread Dave Martin
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

2015-09-01 Thread Robert Jarzmik
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/