diff mbox

ARM: poison initmem when it is freed

Message ID 20110705184254.GH8286@n2100.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King - ARM Linux July 5, 2011, 6:42 p.m. UTC
When the initmem is freed, we can no longer rely on its contents.  In
lightly loaded systems, this memory may persist for some time, making
it harder discover run-time issues (caused by the build warnings being
ignored.)

Poison the initmem at the point where it is freed to encourage run-time
problems when initmem is dereferenced as an aid to finding such problems.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/init.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Nicolas Pitre July 5, 2011, 7:17 p.m. UTC | #1
On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:

> When the initmem is freed, we can no longer rely on its contents.  In
> lightly loaded systems, this memory may persist for some time, making
> it harder discover run-time issues (caused by the build warnings being
> ignored.)
> 
> Poison the initmem at the point where it is freed to encourage run-time
> problems when initmem is dereferenced as an aid to finding such problems.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

The default poison doesn't appear to be a judicious choice for ARM.

include/linux/poison.h:#define POISON_FREE_INITMEM      0xcc

   0:   cccccccc        stclgt  12, cr12, [ip], {204}   ; 0xcc

So if the gt condition is false this will execute nops until it falls 
out of the initmem section.  Would be nicer if a fault could be 
generated right at the accessed address which could be looked up.


Nicolas
Russell King - ARM Linux July 5, 2011, 7:26 p.m. UTC | #2
On Tue, Jul 05, 2011 at 03:17:33PM -0400, Nicolas Pitre wrote:
> On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:
> 
> > When the initmem is freed, we can no longer rely on its contents.  In
> > lightly loaded systems, this memory may persist for some time, making
> > it harder discover run-time issues (caused by the build warnings being
> > ignored.)
> > 
> > Poison the initmem at the point where it is freed to encourage run-time
> > problems when initmem is dereferenced as an aid to finding such problems.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> The default poison doesn't appear to be a judicious choice for ARM.
> 
> include/linux/poison.h:#define POISON_FREE_INITMEM      0xcc
> 
>    0:   cccccccc        stclgt  12, cr12, [ip], {204}   ; 0xcc
> 
> So if the gt condition is false this will execute nops until it falls 
> out of the initmem section.  Would be nicer if a fault could be 
> generated right at the accessed address which could be looked up.

Have you tried to find a byte-based poison value which would fault
yet still cause a pointer dereference?  You're limited to 0xeN on
ARM, of which there's almost nothing to chose from:

   0:   e0e0e0e0        rsc     lr, r0, r0, ror #1
   4:   e1e1e1e1        mvn     lr, r1, ror #3
   8:   e2e2e2e2        rsc     lr, r2, #536870926      ; 0x2000000e
   c:   e3e3e3e3        mvn     lr, #-1946157053        ; 0x8c000003
  10:   e4e4e4e4        strbt   lr, [r4], #1252
  14:   e5e5e5e5        strb    lr, [r5, #1509]!
  18:   e6e6e6e6        strbt   lr, [r6], r6, ror #13
  1c:   e7e7e7e7        strb    lr, [r7, r7, ror #15]!
  20:   e8e8e8e8        stmia   r8!, {r3, r5, r6, r7, fp, sp, lr, pc}^
  24:   e9e9e9e9        stmib   r9!, {r0, r3, r5, r6, r7, r8, fp, sp, lr, pc}^
  28:   eaeaeaea        b       0xffababd8
  2c:   ebebebeb        bl      0xffafafe0
  30:   ecececec        stcl    12, cr14, [ip], #944
  34:   edededed        stcl    13, cr14, [sp, #948]!
  38:   eeeeeeee        cdp     14, 14, cr14, cr14, cr14, {7}
  3c:   efefefef        svc     0x00efefef

0xefefefef looks to be about the best alternative.

It then brings up whether POISON_FREE_INITMEM should be changed or not,
as 0xcc is the expected value for this at the moment.
Nicolas Pitre July 5, 2011, 7:48 p.m. UTC | #3
On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:

> On Tue, Jul 05, 2011 at 03:17:33PM -0400, Nicolas Pitre wrote:
> > On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:
> > 
> > > When the initmem is freed, we can no longer rely on its contents.  In
> > > lightly loaded systems, this memory may persist for some time, making
> > > it harder discover run-time issues (caused by the build warnings being
> > > ignored.)
> > > 
> > > Poison the initmem at the point where it is freed to encourage run-time
> > > problems when initmem is dereferenced as an aid to finding such problems.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > The default poison doesn't appear to be a judicious choice for ARM.
> > 
> > include/linux/poison.h:#define POISON_FREE_INITMEM      0xcc
> > 
> >    0:   cccccccc        stclgt  12, cr12, [ip], {204}   ; 0xcc
> > 
> > So if the gt condition is false this will execute nops until it falls 
> > out of the initmem section.  Would be nicer if a fault could be 
> > generated right at the accessed address which could be looked up.
> 
> Have you tried to find a byte-based poison value which would fault
> yet still cause a pointer dereference?  You're limited to 0xeN on
> ARM, of which there's almost nothing to chose from:
> 
>    0:   e0e0e0e0        rsc     lr, r0, r0, ror #1
>    4:   e1e1e1e1        mvn     lr, r1, ror #3
>    8:   e2e2e2e2        rsc     lr, r2, #536870926      ; 0x2000000e
>    c:   e3e3e3e3        mvn     lr, #-1946157053        ; 0x8c000003
>   10:   e4e4e4e4        strbt   lr, [r4], #1252
>   14:   e5e5e5e5        strb    lr, [r5, #1509]!
>   18:   e6e6e6e6        strbt   lr, [r6], r6, ror #13
>   1c:   e7e7e7e7        strb    lr, [r7, r7, ror #15]!
>   20:   e8e8e8e8        stmia   r8!, {r3, r5, r6, r7, fp, sp, lr, pc}^
>   24:   e9e9e9e9        stmib   r9!, {r0, r3, r5, r6, r7, r8, fp, sp, lr, pc}^
>   28:   eaeaeaea        b       0xffababd8
>   2c:   ebebebeb        bl      0xffafafe0
>   30:   ecececec        stcl    12, cr14, [ip], #944
>   34:   edededed        stcl    13, cr14, [sp, #948]!
>   38:   eeeeeeee        cdp     14, 14, cr14, cr14, cr14, {7}
>   3c:   efefefef        svc     0x00efefef
> 
> 0xefefefef looks to be about the best alternative.

Right.  Does it have to be a byte?  Having a word (or half-word if 
Thumb2) would be much more convenient.

> It then brings up whether POISON_FREE_INITMEM should be changed or not,
> as 0xcc is the expected value for this at the moment.

I would think that this should be a per architecture value to actually 
be useful.


Nicolas
Tixy July 6, 2011, 9:08 a.m. UTC | #4
On Tue, 2011-07-05 at 15:48 -0400, Nicolas Pitre wrote:
> On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:
> > Have you tried to find a byte-based poison value which would fault
> > yet still cause a pointer dereference?  You're limited to 0xeN on
> > ARM, of which there's almost nothing to chose from:
> > 
> >    0:   e0e0e0e0        rsc     lr, r0, r0, ror #1
> >    4:   e1e1e1e1        mvn     lr, r1, ror #3
> >    8:   e2e2e2e2        rsc     lr, r2, #536870926      ; 0x2000000e
> >    c:   e3e3e3e3        mvn     lr, #-1946157053        ; 0x8c000003
> >   10:   e4e4e4e4        strbt   lr, [r4], #1252
> >   14:   e5e5e5e5        strb    lr, [r5, #1509]!
> >   18:   e6e6e6e6        strbt   lr, [r6], r6, ror #13
> >   1c:   e7e7e7e7        strb    lr, [r7, r7, ror #15]!
> >   20:   e8e8e8e8        stmia   r8!, {r3, r5, r6, r7, fp, sp, lr, pc}^
> >   24:   e9e9e9e9        stmib   r9!, {r0, r3, r5, r6, r7, r8, fp, sp, lr, pc}^
> >   28:   eaeaeaea        b       0xffababd8
> >   2c:   ebebebeb        bl      0xffafafe0
> >   30:   ecececec        stcl    12, cr14, [ip], #944
> >   34:   edededed        stcl    13, cr14, [sp, #948]!
> >   38:   eeeeeeee        cdp     14, 14, cr14, cr14, cr14, {7}
> >   3c:   efefefef        svc     0x00efefef
> > 
> > 0xefefefef looks to be about the best alternative.
> 
> Right.  Does it have to be a byte?  Having a word (or half-word if 
> Thumb2) would be much more convenient.

For Thumb, 0xde?? is Permanently UNDEFINED, so we could have 0xdede for
a single byte pattern or an even more descriptive 0xdead if we don't
have that restriction.
Russell King - ARM Linux July 6, 2011, 8:35 p.m. UTC | #5
On Wed, Jul 06, 2011 at 10:08:20AM +0100, Tixy wrote:
> On Tue, 2011-07-05 at 15:48 -0400, Nicolas Pitre wrote:
> > On Tue, 5 Jul 2011, Russell King - ARM Linux wrote:
> > > Have you tried to find a byte-based poison value which would fault
> > > yet still cause a pointer dereference?  You're limited to 0xeN on
> > > ARM, of which there's almost nothing to chose from:
> > > 
> > >    0:   e0e0e0e0        rsc     lr, r0, r0, ror #1
> > >    4:   e1e1e1e1        mvn     lr, r1, ror #3
> > >    8:   e2e2e2e2        rsc     lr, r2, #536870926      ; 0x2000000e
> > >    c:   e3e3e3e3        mvn     lr, #-1946157053        ; 0x8c000003
> > >   10:   e4e4e4e4        strbt   lr, [r4], #1252
> > >   14:   e5e5e5e5        strb    lr, [r5, #1509]!
> > >   18:   e6e6e6e6        strbt   lr, [r6], r6, ror #13
> > >   1c:   e7e7e7e7        strb    lr, [r7, r7, ror #15]!
> > >   20:   e8e8e8e8        stmia   r8!, {r3, r5, r6, r7, fp, sp, lr, pc}^
> > >   24:   e9e9e9e9        stmib   r9!, {r0, r3, r5, r6, r7, r8, fp, sp, lr, pc}^
> > >   28:   eaeaeaea        b       0xffababd8
> > >   2c:   ebebebeb        bl      0xffafafe0
> > >   30:   ecececec        stcl    12, cr14, [ip], #944
> > >   34:   edededed        stcl    13, cr14, [sp, #948]!
> > >   38:   eeeeeeee        cdp     14, 14, cr14, cr14, cr14, {7}
> > >   3c:   efefefef        svc     0x00efefef
> > > 
> > > 0xefefefef looks to be about the best alternative.
> > 
> > Right.  Does it have to be a byte?  Having a word (or half-word if 
> > Thumb2) would be much more convenient.
> 
> For Thumb, 0xde?? is Permanently UNDEFINED, so we could have 0xdede for
> a single byte pattern or an even more descriptive 0xdead if we don't
> have that restriction.

Works for Thumb but not ARM.  For ARM it needs to be 0xeN.

Stephen Boyd's patch results in a 32-bit value which will fault as
an instruction in both ARM and Thumb modes, so that sounds like the
best solution.
diff mbox

Patch

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 2c2cce9..46c39bd 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -19,6 +19,7 @@ 
 #include <linux/highmem.h>
 #include <linux/gfp.h>
 #include <linux/memblock.h>
+#include <linux/poison.h>
 #include <linux/sort.h>
 
 #include <asm/mach-types.h>
@@ -701,6 +702,8 @@  void free_initmem(void)
 				    "TCM link");
 #endif
 
+	memset(__init_begin, POISON_FREE_INITMEM, __init_end - __init_begin);
+
 	if (!machine_is_integrator() && !machine_is_cintegrator())
 		totalram_pages += free_area(__phys_to_pfn(__pa(__init_begin)),
 					    __phys_to_pfn(__pa(__init_end)),