Skip to contentSkip to conversation

ARM: aout-core: zero thread debug registers in a.out core dump

PATCH New Will Deacon 2 participants 5 messages Mar 8, 2011 – Mar 10, 2011 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore

Will Deacon

author
#0 lore raw

PATCH 1/1 ARM: aout-core: zero thread debug registers in a.out core dump

The removal of the single-step emulation from ptrace on ARM means that thread_struct no longer has software breakpoint fields in its debug member.

This patch fixes the a.out core dump code so that the debug registers are zeroed rather than trying to copy from non-existent fields.

Cc Nicolas Pitre <[email protected]>
Reported-by Bryan Wu <[email protected]>
Signed-off-by Will Deacon <[email protected]>

---

Hi Bryan,

Feel free to upgrade your tag on this (signed-off / acked) since this is clearly based on the patch you posted yesterday. I'd like to get this to Russell ASAP so that we don't get a build-breaker in the near future.

Cheers,

Will

 arch/arm/include/asm/a.out-core.h |    6 +-----
 arch/arm/include/asm/user.h       |    2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)
arch/arm/include/asm/a.out-core.h +1 -5
index 93d04ac..92f10cb 100644
@@ -32,11 +32,7 @@ static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump)
 	dump->u_dsize = (tsk->mm->brk - tsk->mm->start_data + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	dump->u_ssize = 0;
 
-	dump->u_debugreg[0] = tsk->thread.debug.bp[0].address;
-	dump->u_debugreg[1] = tsk->thread.debug.bp[1].address;
-	dump->u_debugreg[2] = tsk->thread.debug.bp[0].insn.arm;
-	dump->u_debugreg[3] = tsk->thread.debug.bp[1].insn.arm;
-	dump->u_debugreg[4] = tsk->thread.debug.nsaved;
+	memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg));
 
 	if (dump->start_stack < 0x04000000)
 		dump->u_ssize = (0x04000000 - dump->start_stack) >> PAGE_SHIFT;
arch/arm/include/asm/user.h +1 -1
index 05ac4b0..35917b3 100644
@@ -71,7 +71,7 @@ struct user{
 				/* the registers. */
   unsigned long magic;		/* To uniquely identify a core file */
   char u_comm[32];		/* User command that was responsible */
-  int u_debugreg[8];
+  int u_debugreg[8];		/* No longer used */
   struct user_fp u_fp;		/* FP state */
   struct user_fp_struct * u_fp0;/* Used by gdb to help find the values for */
   				/* the FP registers. */
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/1 ARM: aout-core: zero thread debug registers in a.out core dump

Quoted 10 lines · Will Deacon

The removal of the single-step emulation from ptrace on ARM means that thread_struct no longer has software breakpoint fields in its debug member.

This patch fixes the a.out core dump code so that the debug registers are zeroed rather than trying to copy from non-existent fields.

Cc Nicolas Pitre <[email protected]>
Reported-by Bryan Wu <[email protected]>
Signed-off-by Will Deacon <[email protected]>
Quoted 8 lines

---

Hi Bryan,

Feel free to upgrade your tag on this (signed-off / acked) since this is clearly based on the patch you posted yesterday. I'd like to get this to Russell ASAP so that we don't get a build-breaker in the near future.

Oh, I missed this email. Just send out an similar patch.

Quoted 13 lines

Cheers,

Will

?arch/arm/include/asm/a.out-core.h | ? ?6 +-----
?arch/arm/include/asm/user.h ? ? ? | ? ?2 +-
?2 files changed, 2 insertions(+), 6 deletions(-)

arch/arm/include/asm/a.out-core.h +0 -0
index 93d04ac..92f10cb 100644

arch/arm/include/asm/a.out-core.h static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump) · line 36

Quoted 10 lines
arch/arm/include/asm/a.out-core.h +1 -5
@@ -32,11 +32,7 @@ static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump)
 ? ? ? ?dump->u_dsize = (tsk->mm->brk - tsk->mm->start_data + PAGE_SIZE - 1) >> PAGE_SHIFT;
 ? ? ? ?dump->u_ssize = 0;
 
- ? ? ? dump->u_debugreg[0] = tsk->thread.debug.bp[0].address;
- ? ? ? dump->u_debugreg[1] = tsk->thread.debug.bp[1].address;
- ? ? ? dump->u_debugreg[2] = tsk->thread.debug.bp[0].insn.arm;
- ? ? ? dump->u_debugreg[3] = tsk->thread.debug.bp[1].insn.arm;
- ? ? ? dump->u_debugreg[4] = tsk->thread.debug.nsaved;
+ ? ? ? memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg));

I think this should be
memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg) * 8);

u_debugreg is a pointer to a int, the array contains 8 of them as we found in the user.h

Quoted 9 lines
arch/arm/include/asm/a.out-core.h +0 -0
 ? ? ? ?if (dump->start_stack < 0x04000000)
 ? ? ? ? ? ? ? ?dump->u_ssize = (0x04000000 - dump->start_stack) >> PAGE_SHIFT;
arch/arm/include/asm/user.h +0 -0
index 05ac4b0..35917b3 100644
@@ -71,7 +71,7 @@ struct user{

? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?/* the registers. */
? unsigned long magic; ? ? ? ? /* To uniquely identify a core file */

Quoted 10 lines

? char u_comm[32]; ? ? ? ? ? ? /* User command that was responsible */
- ?int u_debugreg[8];
+ ?int u_debugreg[8]; ? ? ? ? ? /* No longer used */
? struct user_fp u_fp; ? ? ? ? /* FP state */
? struct user_fp_struct * u_fp0;/* Used by gdb to help find the values for */ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?/* the FP registers. */
--
1.7.0.4

-Bryan

author
#2 lore raw

PATCH 1/1 ARM: aout-core: zero thread debug registers in a.out core dump

Hi Bryan,

Quoted 8 lines
Quoted 6 lines

Hi Bryan,

Feel free to upgrade your tag on this (signed-off / acked) since this is clearly based on the patch you posted yesterday. I'd like to get this to Russell ASAP so that we don't get a build-breaker in the near future.

Oh, I missed this email. Just send out an similar patch.

No problem, I'd just like to get this in the pipeline asap!

Quoted 18 lines
Quoted 5 lines
arch/arm/include/asm/a.out-core.h +0 -0
index 93d04ac..92f10cb 100644

arch/arm/include/asm/a.out-core.h static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump) · line 36

Quoted 10 lines
arch/arm/include/asm/a.out-core.h +1 -5
@@ -32,11 +32,7 @@ static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump)
 ? ? ? ?dump->u_dsize = (tsk->mm->brk - tsk->mm->start_data + PAGE_SIZE - 1) >> PAGE_SHIFT;
 ? ? ? ?dump->u_ssize = 0;
 
- ? ? ? dump->u_debugreg[0] = tsk->thread.debug.bp[0].address;
- ? ? ? dump->u_debugreg[1] = tsk->thread.debug.bp[1].address;
- ? ? ? dump->u_debugreg[2] = tsk->thread.debug.bp[0].insn.arm;
- ? ? ? dump->u_debugreg[3] = tsk->thread.debug.bp[1].insn.arm;
- ? ? ? dump->u_debugreg[4] = tsk->thread.debug.nsaved;
+ ? ? ? memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg));

I think this should be
memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg) * 8);

Quoted 2 lines

u_debugreg is a pointer to a int, the array contains 8 of them as we found in the user.h

dump_udebugreg is an integer array of fixed size. If you look at the disassembly of fs/binfmt_aout.o (removed some inlining, recompiled with -01):

00000c7c <aout_dump_thread>:

 c7c:   e92d4038        push    {r3, r4, r5, lr}
 c80:   e1a05000        mov     r5, r0
 c84:   e1a04001        mov     r4, r1
 c88:   e1a0200d        mov     r2, sp

[...]

 cfc:   e2840090        add     r0, r4, #144    ; 0x90
 d00:   e3a01020        mov     r1, #32
 d04:   ebfffffe        bl      0 <__memzero>

So the size from sizeof is correct, multiplying it by 8 is asking for trouble! If we used the ARRAY_SIZE macro, then the multiplication would be necessary because we would have performed the division inside the macro.

Please can you submit another patch? I think the one I posted the other day was alright, so if you re-post that you can add my S-o-B.

Thanks,

Will

A message that is not in the archive (sent off-list or not yet received): -4405681450588494999@unknownmsgid

#4 lore raw

PATCH 1/1 ARM: aout-core: zero thread debug registers in a.out core dump

Quoted 60 lines · Will Deacon

Hi Bryan,

Quoted 3 lines
Quoted 3 lines

Hi Bryan,

Feel free to upgrade your tag on this (signed-off / acked) since this

is

Quoted 1 line
Quoted 1 line

clearly based on the patch you posted yesterday. I'd like to get this

to

Quoted 4 lines
Quoted 2 lines

Russell ASAP so that we don't get a build-breaker in the near future.

Oh, I missed this email. Just send out an similar patch.

No problem, I'd just like to get this in the pipeline asap!

Quoted 1 line

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/arm/include/asm/a.out-core.h

Quoted 4 lines

arch/arm/include/asm/a.out-core.h static inline void aout_dump_thread(struct pt_regs

Quoted 4 lines

index 93d04ac..92f10cb 100644

arch/arm/include/asm/a.out-core.h +0 -0
@@ -32,11 +32,7 @@ static inline void aout_dump_thread(struct pt_regs

*regs, struct user *dump)

Quoted 1 line

arch/arm/include/asm/a.out-core.h static inline void aout_dump_thread(struct pt_regs · line 32

Quoted 1 line
arch/arm/include/asm/a.out-core.h +0 -0
       dump->u_dsize = (tsk->mm->brk - tsk->mm->start_data + PAGE_SIZE

- 1) >> PAGE_SHIFT;

Quoted 9 lines

arch/arm/include/asm/a.out-core.h static inline void aout_dump_thread(struct pt_regs *regs, struct user *dump) · line 36

Quoted 9 lines
arch/arm/include/asm/a.out-core.h +1 -5
       dump->u_ssize = 0;
 
-       dump->u_debugreg[0] = tsk->thread.debug.bp[0].address;
-       dump->u_debugreg[1] = tsk->thread.debug.bp[1].address;
-       dump->u_debugreg[2] = tsk->thread.debug.bp[0].insn.arm;
-       dump->u_debugreg[3] = tsk->thread.debug.bp[1].insn.arm;
-       dump->u_debugreg[4] = tsk->thread.debug.nsaved;
+       memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg));
Quoted 5 lines

I think this should be
memset(dump->u_debugreg, 0, sizeof(dump->u_debugreg) * 8);

u_debugreg is a pointer to a int, the array contains 8 of them as we found in the user.h

dump_udebugreg is an integer array of fixed size. If you look at the disassembly of fs/binfmt_aout.o (removed some inlining, recompiled with -01):

00000c7c <aout_dump_thread>:

 c7c:   e92d4038        push    {r3, r4, r5, lr}
 c80:   e1a05000        mov     r5, r0
 c84:   e1a04001        mov     r4, r1
 c88:   e1a0200d        mov     r2, sp

[...]

 cfc:   e2840090        add     r0, r4, #144    ; 0x90
 d00:   e3a01020        mov     r1, #32
 d04:   ebfffffe        bl      0 <__memzero>

So the size from sizeof is correct, multiplying it by 8 is asking for trouble! If we used the ARRAY_SIZE macro, then the multiplication would be necessary because we would have performed the division inside the macro.

Quoted 8 lines

Please can you submit another patch? I think the one I posted the other day was alright, so if you re-post that you can add my S-o-B.

Thanks,

Will

Ah, yeah, I got it.

I think your patch is right, no need for me to repost. Please add my SOB

Signed-off-by Bryan Wu <[email protected]>

Thanks,
-Bryan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110310/f4e66fb0/attachment.html>

author
#5 lore raw

PATCH 1/1 ARM: aout-core: zero thread debug registers in a.out core dump

Hi Bryan,

Quoted 5 lines

Ah, yeah, I got it.

I think your patch is right, no need for me to repost. Please add my SOB

Signed-off-by Bryan Wu <[email protected]>

Thanks, submitted as 6798/1. Note that the patch system didn't like the `re:' substring of ARM: aout-core: zero ... so the patch name has been slightly mangled.

Will