* [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump @ 2011-03-08 10:57 Will Deacon 2011-03-10 3:43 ` Bryan Wu 0 siblings, 1 reply; 5+ messages in thread From: Will Deacon @ 2011-03-08 10:57 UTC (permalink / raw) To: linux-arm-kernel 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 <nicolas.pitre@linaro.org> Reported-by: Bryan Wu <bryan.wu@canonical.com> Signed-off-by: Will Deacon <will.deacon@arm.com> --- 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(-) diff --git a/arch/arm/include/asm/a.out-core.h b/arch/arm/include/asm/a.out-core.h index 93d04ac..92f10cb 100644 --- a/arch/arm/include/asm/a.out-core.h +++ b/arch/arm/include/asm/a.out-core.h @@ -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; diff --git a/arch/arm/include/asm/user.h b/arch/arm/include/asm/user.h index 05ac4b0..35917b3 100644 --- a/arch/arm/include/asm/user.h +++ b/arch/arm/include/asm/user.h @@ -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. */ -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump 2011-03-08 10:57 [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump Will Deacon @ 2011-03-10 3:43 ` Bryan Wu 2011-03-10 10:15 ` Will Deacon [not found] ` <-4405681450588494999@unknownmsgid> 0 siblings, 2 replies; 5+ messages in thread From: Bryan Wu @ 2011-03-10 3:43 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 8, 2011 at 6:57 PM, Will Deacon <will.deacon@arm.com> wrote: > 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 <nicolas.pitre@linaro.org> > Reported-by: Bryan Wu <bryan.wu@canonical.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> > --- > > 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. > 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(-) > > diff --git a/arch/arm/include/asm/a.out-core.h b/arch/arm/include/asm/a.out-core.h > index 93d04ac..92f10cb 100644 > --- a/arch/arm/include/asm/a.out-core.h > +++ b/arch/arm/include/asm/a.out-core.h > @@ -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 > ? ? ? ?if (dump->start_stack < 0x04000000) > ? ? ? ? ? ? ? ?dump->u_ssize = (0x04000000 - dump->start_stack) >> PAGE_SHIFT; > diff --git a/arch/arm/include/asm/user.h b/arch/arm/include/asm/user.h > index 05ac4b0..35917b3 100644 > --- a/arch/arm/include/asm/user.h > +++ b/arch/arm/include/asm/user.h > @@ -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. */ > -- > 1.7.0.4 > > -Bryan ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump 2011-03-10 3:43 ` Bryan Wu @ 2011-03-10 10:15 ` Will Deacon [not found] ` <-4405681450588494999@unknownmsgid> 1 sibling, 0 replies; 5+ messages in thread From: Will Deacon @ 2011-03-10 10:15 UTC (permalink / raw) To: linux-arm-kernel Hi Bryan, > > 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! > > diff --git a/arch/arm/include/asm/a.out-core.h b/arch/arm/include/asm/a.out-core.h > > index 93d04ac..92f10cb 100644 > > --- a/arch/arm/include/asm/a.out-core.h > > +++ b/arch/arm/include/asm/a.out-core.h > > @@ -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 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <-4405681450588494999@unknownmsgid>]
* [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump [not found] ` <-4405681450588494999@unknownmsgid> @ 2011-03-10 11:06 ` Bryan Wu 2011-03-10 13:08 ` Will Deacon 0 siblings, 1 reply; 5+ messages in thread From: Bryan Wu @ 2011-03-10 11:06 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 10, 2011 at 6:15 PM, Will Deacon <will.deacon@arm.com> wrote: > Hi Bryan, > > > > 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! > > > > diff --git a/arch/arm/include/asm/a.out-core.h > b/arch/arm/include/asm/a.out-core.h > > > index 93d04ac..92f10cb 100644 > > > --- a/arch/arm/include/asm/a.out-core.h > > > +++ b/arch/arm/include/asm/a.out-core.h > > > @@ -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 > > 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 > > 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 <bryan.wu@canonical.com> Thanks, -Bryan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110310/f4e66fb0/attachment.html> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump 2011-03-10 11:06 ` Bryan Wu @ 2011-03-10 13:08 ` Will Deacon 0 siblings, 0 replies; 5+ messages in thread From: Will Deacon @ 2011-03-10 13:08 UTC (permalink / raw) To: linux-arm-kernel Hi Bryan, > 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 <bryan.wu@canonical.com> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-10 13:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-08 10:57 [PATCH] ARM: aout-core: zero thread debug registers in a.out core dump Will Deacon
2011-03-10 3:43 ` Bryan Wu
2011-03-10 10:15 ` Will Deacon
[not found] ` <-4405681450588494999@unknownmsgid>
2011-03-10 11:06 ` Bryan Wu
2011-03-10 13:08 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).