Exception handling keeps additional state (whether exception was Trap
and if it was due to a breakpoint) in pt_regs->event, a bitfield member
unsigned long orig_r8:16, event:16;
A bitfield esentially has an "offset" and a "length". What I wasn't
aware of was that, bitfields in a union loose the "offset" attribute
and all of them are laid out at offset 0.
This obviously means that both @event and @orig_r8 will be incorrectly
referenced to at same "0" offset by "C" generated code which is
certainly wrong, not because both members are accessed, but because asm
code updates it at different address.
In Little Endian config, @event is at offset 0 and @orig_r8 (not
actively used at all) clashing with it is OK. However in Big Endian
config,
ST 0xNNNN_EEEE, [addr]
writes 0xEEEE to @event (offset 2 in memory) while "C" code references
it from 0.
Needless to say, this causes ptrace machinery to not detect the breakpoint
scenario (and incorrect stop_pc returned to gdbserver).
------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"
However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------
Reported-by: Noam Camus <[email protected]>
Cc: <[email protected]> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
---
arch/arc/include/asm/ptrace.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
/*to distinguish bet excp, syscall, irq */
union {
+ struct {
#ifdef CONFIG_CPU_BIG_ENDIAN
/* so that assembly code is same for LE/BE */
unsigned long orig_r8:16, event:16;
#else
unsigned long event:16, orig_r8:16;
#endif
+ };
long orig_r8_word;
};
};
--
1.8.1.2
This is related to how how different asm instructions write to a
bitfield member.
In BE config, @orig_r8 is at offset 0 while @event is at offset 2.
"ST 0xNNNN_EEEE, [addr]" correctly writes 0xEEEE to @event, however
a half word write, "STW 0xEEEE, [addr]" does not in Big Endian while
works perfectly file for LE config (since @event is at offset 0).
------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"
However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------
Reported-by: Noam Camus <[email protected]>
Cc: <[email protected]> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
---
arch/arc/kernel/entry.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
index 7b5f3ec..724de08 100644
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
trap_with_param:
; stop_pc info by gdb needs this info
- stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+ st orig_r8_IS_BRKPT, [sp, PT_orig_r8]
mov r0, r12
lr r1, [efa]
@@ -727,7 +727,7 @@ not_exception:
; things to what they were, before returning from L2 context
;----------------------------------------------------------------
- ldw r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
+ ld r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
ld r9, [sp, PT_status32] ; get statu32_l2 (saved in pt_regs)
--
1.8.1.2
Hi Greg,
I'd posted these patches for stable backport. Do I need to do anything more to get them included.
https://patchwork.kernel.org/patch/2841153/ [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
https://patchwork.kernel.org/patch/2841158/ [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
Thx,
-Vineet
On 08/08/2013 07:23 PM, Vineet Gupta wrote:
Exception handling keeps additional state (whether exception was Trap
and if it was due to a breakpoint) in pt_regs->event, a bitfield member
unsigned long orig_r8:16, event:16;
A bitfield esentially has an "offset" and a "length". What I wasn't
aware of was that, bitfields in a union loose the "offset" attribute
and all of them are laid out at offset 0.
This obviously means that both @event and @orig_r8 will be incorrectly
referenced to at same "0" offset by "C" generated code which is
certainly wrong, not because both members are accessed, but because asm
code updates it at different address.
In Little Endian config, @event is at offset 0 and @orig_r8 (not
actively used at all) clashing with it is OK. However in Big Endian
config,
ST 0xNNNN_EEEE, [addr]
writes 0xEEEE to @event (offset 2 in memory) while "C" code references
it from 0.
Needless to say, this causes ptrace machinery to not detect the breakpoint
scenario (and incorrect stop_pc returned to gdbserver).
------>8---------------
Thi issue is already fixed in mainline 3.11 kernel as part of commit:
502a0c775c7f0a "ARC: pt_regs update #5"
However that patch has lot more changes than I would like backporting,
hence this seperate change.
------>8---------------
Reported-by: Noam Camus <[email protected]><mailto:[email protected]>
Cc: <[email protected]><mailto:[email protected]> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <[email protected]><mailto:[email protected]>
Signed-off-by: Vineet Gupta <[email protected]><mailto:[email protected]>
---
arch/arc/include/asm/ptrace.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
/*to distinguish bet excp, syscall, irq */
union {
+ struct {
#ifdef CONFIG_CPU_BIG_ENDIAN
/* so that assembly code is same for LE/BE */
unsigned long orig_r8:16, event:16;
#else
unsigned long event:16, orig_r8:16;
#endif
+ };
long orig_r8_word;
};
};
On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
> Hi Greg,
>
> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
>
> https://patchwork.kernel.org/patch/2841153/ [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
> https://patchwork.kernel.org/patch/2841158/ [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
I ignored them as I thought you were submitting them for upstream
inclusion. If they are already in Linus's tree, then take a look at the
file, Documentation/stable_kernel_rules.txt for how to send a patch for
inclusion into a stable release (hint, I need to know the git commit id
that the patch has in Linus's tree, which I didn't see anywhere here.)
thanks,
greg k-h
Hi Greg,
On 08/19/2013 08:27 PM, greg Kroah-Hartman wrote:
> On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
>> Hi Greg,
>>
>> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
>>
>> https://patchwork.kernel.org/patch/2841153/ [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
>> https://patchwork.kernel.org/patch/2841158/ [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
>
> I ignored them as I thought you were submitting them for upstream
> inclusion. If they are already in Linus's tree, then take a look at the
> file, Documentation/stable_kernel_rules.txt for how to send a patch for
> inclusion into a stable release (hint, I need to know the git commit id
> that the patch has in Linus's tree, which I didn't see anywhere here.)
I'm sure I mentioned the commit-id in the patch. Hint, please look at annotation
within --->8--- blocks in changelogs.
The caveat however is we can't apply those exact commits as it is as that warrants
more changes to be pulled in. However I'm going by the last stable kernel rule "It
or an equivalent fix must already exist in Linus' tree (upstream)."
Anyhow, since both these patches are extracted from same commit, I'll respin a
single patch (with a small addition - again part of the same commit) and send it
over. OK !
Thx,
-Vineet
On Tue, Aug 20, 2013 at 10:50:40AM +0530, Vineet Gupta wrote:
> Hi Greg,
>
> On 08/19/2013 08:27 PM, greg Kroah-Hartman wrote:
> > On Mon, Aug 19, 2013 at 09:08:49AM +0000, Vineet Gupta wrote:
> >> Hi Greg,
> >>
> >> I'd posted these patches for stable backport. Do I need to do anything more to get them included.
> >>
> >> https://patchwork.kernel.org/patch/2841153/ [1/2] ARC: gdbserver breakage in Big-Endian configuration #1<https://patchwork.kernel.org/patch/2841153/>]
> >> https://patchwork.kernel.org/patch/2841158/ [2/2] ARC: gdbserver breakage in Big-Endian configuration #2<https://patchwork.kernel.org/patch/2841158/>
> >
> > I ignored them as I thought you were submitting them for upstream
> > inclusion. If they are already in Linus's tree, then take a look at the
> > file, Documentation/stable_kernel_rules.txt for how to send a patch for
> > inclusion into a stable release (hint, I need to know the git commit id
> > that the patch has in Linus's tree, which I didn't see anywhere here.)
>
>
> I'm sure I mentioned the commit-id in the patch. Hint, please look at annotation
> within --->8--- blocks in changelogs.
You're right, I missed that, sorry.
> The caveat however is we can't apply those exact commits as it is as that warrants
> more changes to be pulled in. However I'm going by the last stable kernel rule "It
> or an equivalent fix must already exist in Linus' tree (upstream)."
>
> Anyhow, since both these patches are extracted from same commit, I'll respin a
> single patch (with a small addition - again part of the same commit) and send it
> over. OK !
That would be great, thanks.
greg k-h
Hi,
Please apply the following 2 patches to 3.9/3.10 series.
They have been extracted out of mainline patches which can't be individually
backported due to other dependencies (please see respective changelogs for
references).
Thx,
-Vineet
Vineet Gupta (2):
ARC: gdbserver breakage in Big-Endian configuration #1
ARC: gdbserver breakage in Big-Endian configuration #2
arch/arc/include/asm/ptrace.h | 2 ++
arch/arc/include/asm/syscall.h | 5 ++---
arch/arc/kernel/entry.S | 4 ++--
3 files changed, 6 insertions(+), 5 deletions(-)
--
1.8.1.2
[Based on mainline commit 502a0c775c7f0a: "ARC: pt_regs update #5"]
gdbserver needs @stop_pc, served by ptrace, but fetched from pt_regs
differently, based on in_brkpt_traps(), which in turn relies on
additional machine state in pt_regs->event bitfield.
unsigned long orig_r8:16, event:16;
For big endian config, this macro was returning false, despite being in
breakpoint Trap exception, causing wrong @stop_pc to be returned to gdb.
Issue #1: In BE, @event above is at offset 2 in word, while a STW insn
at offset 0 was used to update it. Resort to using ST insn
which updates the half-word at right location.
Issue #2: The union involving bitfields causes all the members to be
laid out at offset 0. So with fix #1 above, ASM was now
updating at offset 2, "C" code was still referencing at
offset 0. Fixed by wrapping bitfield in a struct.
Reported-by: Noam Camus <[email protected]>
Cc: <[email protected]> # [3.9 and 3.10 only]
Tested-by: Anton Kolesov <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
---
arch/arc/include/asm/ptrace.h | 2 ++
arch/arc/kernel/entry.S | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 6179de7..2046a89 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
/*to distinguish bet excp, syscall, irq */
union {
+ struct {
#ifdef CONFIG_CPU_BIG_ENDIAN
/* so that assembly code is same for LE/BE */
unsigned long orig_r8:16, event:16;
#else
unsigned long event:16, orig_r8:16;
#endif
+ };
long orig_r8_word;
};
};
diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
index 7b5f3ec..724de08 100644
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
trap_with_param:
; stop_pc info by gdb needs this info
- stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+ st orig_r8_IS_BRKPT, [sp, PT_orig_r8]
mov r0, r12
lr r1, [efa]
@@ -727,7 +727,7 @@ not_exception:
; things to what they were, before returning from L2 context
;----------------------------------------------------------------
- ldw r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
+ ld r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
ld r9, [sp, PT_status32] ; get statu32_l2 (saved in pt_regs)
--
1.8.1.2
[Based on mainline commit 352c1d95e3220d0: "ARC: stop using
pt_regs->orig_r8"]
Stop using orig_r8 as it could get clobbered by ST in trap_with_param,
and further it is semantically not needed either.
Signed-off-by: Vineet Gupta <[email protected]>
---
arch/arc/include/asm/syscall.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
index 33ab304..29de098 100644
--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -18,7 +18,7 @@ static inline long
syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
{
if (user_mode(regs) && in_syscall(regs))
- return regs->orig_r8;
+ return regs->r8;
else
return -1;
}
@@ -26,8 +26,7 @@ syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
static inline void
syscall_rollback(struct task_struct *task, struct pt_regs *regs)
{
- /* XXX: I can't fathom how pt_regs->r8 will be clobbered ? */
- regs->r8 = regs->orig_r8;
+ regs->r0 = regs->orig_r0;
}
static inline long
--
1.8.1.2
This is a note to let you know that I've just added the patch titled
ARC: gdbserver breakage in Big-Endian configuration #1
to the 3.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
arc-gdbserver-breakage-in-big-endian-configuration-1.patch
and it can be found in the queue-3.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.
>From [email protected] Thu Aug 22 15:32:58 2013
From: Vineet Gupta <[email protected]>
Date: Tue, 20 Aug 2013 13:38:10 +0530
Subject: ARC: gdbserver breakage in Big-Endian configuration #1
To: <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, Vineet Gupta <[email protected]>
Message-ID: <[email protected]>
From: Vineet Gupta <[email protected]>
[Based on mainline commit 502a0c775c7f0a: "ARC: pt_regs update #5"]
gdbserver needs @stop_pc, served by ptrace, but fetched from pt_regs
differently, based on in_brkpt_traps(), which in turn relies on
additional machine state in pt_regs->event bitfield.
unsigned long orig_r8:16, event:16;
For big endian config, this macro was returning false, despite being in
breakpoint Trap exception, causing wrong @stop_pc to be returned to gdb.
Issue #1: In BE, @event above is at offset 2 in word, while a STW insn
at offset 0 was used to update it. Resort to using ST insn
which updates the half-word at right location.
Issue #2: The union involving bitfields causes all the members to be
laid out at offset 0. So with fix #1 above, ASM was now
updating at offset 2, "C" code was still referencing at
offset 0. Fixed by wrapping bitfield in a struct.
Reported-by: Noam Camus <[email protected]>
Tested-by: Anton Kolesov <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/arc/include/asm/ptrace.h | 2 ++
arch/arc/kernel/entry.S | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -52,12 +52,14 @@ struct pt_regs {
/*to distinguish bet excp, syscall, irq */
union {
+ struct {
#ifdef CONFIG_CPU_BIG_ENDIAN
/* so that assembly code is same for LE/BE */
unsigned long orig_r8:16, event:16;
#else
unsigned long event:16, orig_r8:16;
#endif
+ };
long orig_r8_word;
};
};
--- a/arch/arc/kernel/entry.S
+++ b/arch/arc/kernel/entry.S
@@ -498,7 +498,7 @@ tracesys_exit:
trap_with_param:
; stop_pc info by gdb needs this info
- stw orig_r8_IS_BRKPT, [sp, PT_orig_r8]
+ st orig_r8_IS_BRKPT, [sp, PT_orig_r8]
mov r0, r12
lr r1, [efa]
@@ -723,7 +723,7 @@ not_exception:
; things to what they were, before returning from L2 context
;----------------------------------------------------------------
- ldw r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
+ ld r9, [sp, PT_orig_r8] ; get orig_r8 to make sure it is
brne r9, orig_r8_IS_IRQ2, 149f ; infact a L2 ISR ret path
ld r9, [sp, PT_status32] ; get statu32_l2 (saved in pt_regs)
Patches currently in stable-queue which might be from [email protected] are
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-1.patch
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-2.patch
This is a note to let you know that I've just added the patch titled
ARC: gdbserver breakage in Big-Endian configuration #2
to the 3.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
arc-gdbserver-breakage-in-big-endian-configuration-2.patch
and it can be found in the queue-3.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.
>From [email protected] Thu Aug 22 15:33:34 2013
From: Vineet Gupta <[email protected]>
Date: Tue, 20 Aug 2013 13:38:11 +0530
Subject: ARC: gdbserver breakage in Big-Endian configuration #2
To: <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, Vineet Gupta <[email protected]>
Message-ID: <[email protected]>
From: Vineet Gupta <[email protected]>
[Based on mainline commit 352c1d95e3220d0: "ARC: stop using
pt_regs->orig_r8"]
Stop using orig_r8 as it could get clobbered by ST in trap_with_param,
and further it is semantically not needed either.
Signed-off-by: Vineet Gupta <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/arc/include/asm/syscall.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- a/arch/arc/include/asm/syscall.h
+++ b/arch/arc/include/asm/syscall.h
@@ -18,7 +18,7 @@ static inline long
syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
{
if (user_mode(regs) && in_syscall(regs))
- return regs->orig_r8;
+ return regs->r8;
else
return -1;
}
@@ -26,8 +26,7 @@ syscall_get_nr(struct task_struct *task,
static inline void
syscall_rollback(struct task_struct *task, struct pt_regs *regs)
{
- /* XXX: I can't fathom how pt_regs->r8 will be clobbered ? */
- regs->r8 = regs->orig_r8;
+ regs->r0 = regs->orig_r0;
}
static inline long
Patches currently in stable-queue which might be from [email protected] are
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-1.patch
queue-3.10/arc-gdbserver-breakage-in-big-endian-configuration-2.patch