ARM: poison initmem when it is freed

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 <[email protected]>

Quoted 9 lines · Russell King - ARM Linux

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 <[email protected]>

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.

Quoted 14 lines · Nicolas Pitre

Quoted 9 lines · Russell King - ARM Linux

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 <[email protected]>

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

Quoted 7 lines

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.

Quoted 42 lines · Russell King - ARM Linux

Quoted 13 lines · Nicolas Pitre

Quoted 9 lines · Russell King - ARM Linux

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 <[email protected]>

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

Quoted 7 lines

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

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 44 lines · Nicolas Pitre

Quoted 42 lines · Russell King - ARM Linux

Quoted 13 lines · Nicolas Pitre

Quoted 9 lines · Russell King - ARM Linux

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 <[email protected]>

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

Quoted 7 lines

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

Quoted 9 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Didn't I already post this patch about 6 months ago?

https://lkml.org/lkml/2011/1/11/1

Here it is, the only downside I see is the memset isn't really efficient as the assembler optimized one.

Quoted 59 lines · Stephen Boyd

Quoted 44 lines · Nicolas Pitre

Quoted 42 lines · Russell King - ARM Linux

Quoted 13 lines · Nicolas Pitre

Quoted 9 lines · Russell King - ARM Linux

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 <[email protected]>

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

Quoted 7 lines

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

Quoted 9 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Didn't I already post this patch about 6 months ago?

https://lkml.org/lkml/2011/1/11/1

Quoted 2 lines

Here it is, the only downside I see is the memset isn't really efficient as the assembler optimized one.

Ok, let's do it your way...

But, do we need to do it page by page? Can we not have a function which does the poisioning, and we just pass the __init_begin/__init_end and tcm start/end stuff to?

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

How about a free_init_area() function which calls free_area() after poisoning the memory?

Quoted 2 lines · Stephen Boyd

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

It could do - I don't see any harm in not doing so. The only issue is that people may want to disable this stuff if they're after squeezing every last ms out of the boot time.

Quoted 2 lines · Stephen Boyd

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

It could do - I don't see any harm in not doing so. The only issue is that people may want to disable this stuff if they're after squeezing every last ms out of the boot time.

Quoted 3 lines

How about a free_init_area() function which calls free_area() after poisoning the memory?

I need to go back and look at the Integrator etc situation with regard to reorganizing the vmlinux layout - it may be that the omission of freeing .init memory can now be removed (it was there to stop the memory being used as the first K of memory wasn't DMA-able.)

Assuming it has to stay though, we still should arrange for the initrd memory to be poisoned even if it isn't freed.

Poisoning __init marked memory can be useful when tracking down obscure memory corruption bugs. Therefore, poison init memory with 0xe7fddef0 to catch bugs earlier. The poison value is an undefined instruction in ARM mode and branch to an undefined instruction in Thumb mode.

Signed-off-by Stephen Boyd <[email protected]>

---

Quoted 7 lines · Russell King - ARM Linux

Quoted 2 lines · Stephen Boyd

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

It could do - I don't see any harm in not doing so. The only issue is that people may want to disable this stuff if they're after squeezing every last ms out of the boot time.

I haven't done this. I hope a follow up patch will suffice.

Quoted 9 lines

Quoted 3 lines

How about a free_init_area() function which calls free_area() after poisoning the memory?

I need to go back and look at the Integrator etc situation with regard to reorganizing the vmlinux layout - it may be that the omission of freeing .init memory can now be removed (it was there to stop the memory being used as the first K of memory wasn't DMA-able.)

Quoted 2 lines

Assuming it has to stay though, we still should arrange for the initrd memory to be poisoned even if it isn't freed.

Is this is patch what you're saying? I would have liked to do a free_init_area() wrapper, but until the Integrator situation can be sorted it doesn't look worthwhile.

Quoted 25 lines · Stephen Boyd

Poisoning __init marked memory can be useful when tracking down obscure memory corruption bugs. Therefore, poison init memory with 0xe7fddef0 to catch bugs earlier. The poison value is an undefined instruction in ARM mode and branch to an undefined instruction in Thumb mode.

Signed-off-by Stephen Boyd <[email protected]>

---

Quoted 6 lines · Russell King - ARM Linux

Quoted 2 lines · Stephen Boyd

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

It could do - I don't see any harm in not doing so. The only issue is that people may want to disable this stuff if they're after squeezing every last ms out of the boot time.

I haven't done this. I hope a follow up patch will suffice.

Quoted 7 lines

Quoted 2 lines

How about a free_init_area() function which calls free_area() after poisoning the memory?

I need to go back and look at the Integrator etc situation with regard to reorganizing the vmlinux layout - it may be that the omission of freeing .init memory can now be removed (it was there to stop the memory being used as the first K of memory wasn't DMA-able.)

Quoted 5 lines

Quoted 2 lines

Assuming it has to stay though, we still should arrange for the initrd memory to be poisoned even if it isn't freed.

Is this is patch what you're saying? I would have liked to do a free_init_area() wrapper, but until the Integrator situation can be sorted it doesn't look worthwhile.

Yes, thanks. This looks fine for the time being. Have you been able to test it? If yes, then please put it in the patch system and I'll see about giving it a test too.

Poisoning __init marked memory can be useful when tracking down obscure memory corruption bugs. Therefore, poison init memory with 0xe7fddef0 to catch bugs earlier. The poison value is an undefined instruction in ARM mode and branch to an undefined instruction in Thumb mode.

Signed-off-by Stephen Boyd <[email protected]>

---

Quoted 7 lines · Russell King - ARM Linux

Quoted 2 lines · Stephen Boyd

Should it include the initrd too? At least x86 poisons that memory but I don't know who would be using that incorrectly.

It could do - I don't see any harm in not doing so. The only issue is that people may want to disable this stuff if they're after squeezing every last ms out of the boot time.

I haven't done this. I hope a follow up patch will suffice.

Quoted 11 lines

Quoted 3 lines

How about a free_init_area() function which calls free_area() after poisoning the memory?

I need to go back and look at the Integrator etc situation with regard to reorganizing the vmlinux layout - it may be that the omission of freeing .init memory can now be removed (it was there to stop the memory being used as the first K of memory wasn't DMA-able.)

Assuming it has to stay though, we still should arrange for the initrd memory to be poisoned even if it isn't freed.

Is this is patch what you're saying? I would have liked to do a free_init_area() wrapper, but until the Integrator situation can be sorted it doesn't look worthwhile.

 arch/arm/mm/init.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

arch/arm/mm/init.c +16 -1

index c19571c..d6360b1 100644
@@ -422,6 +422,17 @@ static inline int free_area(unsigned long pfn, unsigned long end, char *s)
 	return pages;
 }
 
+/*
+ * Poison init memory with an undefined instruction (ARM) or a branch to an
+ * undefined instruction (Thumb).
+ */
+static inline void poison_init_mem(void *s, size_t count)
+{
+	u32 *p = (u32 *)s;
+	while ((count = count - 4))
+		*p++ = 0xe7fddef0;
+}
+
 static inline void
 free_memmap(unsigned long start_pfn, unsigned long end_pfn)
 {

@@ -704,11 +715,13 @@ void free_initmem(void)
 #ifdef CONFIG_HAVE_TCM
 	extern char __tcm_start, __tcm_end;
 
+	poison_init_mem(&__tcm_start, &__tcm_end - &__tcm_start);
 	totalram_pages += free_area(__phys_to_pfn(__pa(&__tcm_start)),
 				    __phys_to_pfn(__pa(&__tcm_end)),
 				    "TCM link");
 #endif
 
+	poison_init_mem(__init_begin, __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)),

@@ -721,10 +734,12 @@ static int keep_initrd;
 
 void free_initrd_mem(unsigned long start, unsigned long end)
 {
-	if (!keep_initrd)
+	if (!keep_initrd) {
+		poison_init_mem((void *)start, PAGE_ALIGN(end) - start);
 		totalram_pages += free_area(__phys_to_pfn(__pa(start)),
 					    __phys_to_pfn(__pa(end)),
 					    "initrd");
+	}
 }
 
 static int __init keepinitrd_setup(char *__unused)

Signature · 8 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Quoted 21 lines · Russell King - ARM Linux

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

Quoted 1 line

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.

Quoted 22 lines · Nicolas Pitre

Quoted 21 lines · Russell King - ARM Linux

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

Quoted 4 lines

Quoted 1 line

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.