* [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-07 11:14 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-07 11:14 UTC (permalink / raw)
To: Ralf Baechle; +Cc: James Hogan, Gregory Fong, linux-mips, stable
Commit 1400eb6 (MIPS: r4k,octeon,r2300: stack protector: change canary
per task) was merged in v3.11 and introduced assembly in the MIPS resume
functions to update the value of the current canary in
__stack_chk_guard. However it used PTR_L resulting in a load of the
canary value, instead of PTR_LA to construct its address. The value is
intended to be random but is then treated as an address in the
subsequent LONG_S (store).
This was observed to cause a fault and panic:
CPU 0 Unable to handle kernel paging request at virtual address 139fea20, epc == 8000cc0c, ra == 8034f2a4
Oops[#1]:
...
$24 : 139fea20 1e1f7cb6
...
Call Trace:
[<8000cc0c>] resume+0xac/0x118
[<8034f2a4>] __schedule+0x5f8/0x78c
[<8034f4e0>] schedule_preempt_disabled+0x20/0x2c
[<80348eec>] rest_init+0x74/0x84
[<804dc990>] start_kernel+0x43c/0x454
Code: 3c18804b 8f184030 8cb901f8 <af190000> 00c0e021 8cb002f0 8cb102f4 8cb202f8 8cb302fc
This can also be forced by modifying
arch/mips/include/asm/stackprotector.h so that the default
__stack_chk_guard value is more likely to be a bad (or unaligned)
pointer.
Fix it to use PTR_LA instead, to load the address of the canary value,
which the LONG_S can then use to write into it.
Reported-by: bobjones (via #mipslinux on IRC)
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> #3.11
---
Ralf: This is a regression in v3.11, so please consider for v3.12.
(Sorry, resent to Cc stable correctly)
arch/mips/kernel/octeon_switch.S | 2 +-
arch/mips/kernel/r2300_switch.S | 2 +-
arch/mips/kernel/r4k_switch.S | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/kernel/octeon_switch.S b/arch/mips/kernel/octeon_switch.S
index 4204d76..029e002 100644
--- a/arch/mips/kernel/octeon_switch.S
+++ b/arch/mips/kernel/octeon_switch.S
@@ -73,7 +73,7 @@
3:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r2300_switch.S b/arch/mips/kernel/r2300_switch.S
index 38af83f..20b7b04 100644
--- a/arch/mips/kernel/r2300_switch.S
+++ b/arch/mips/kernel/r2300_switch.S
@@ -67,7 +67,7 @@ LEAF(resume)
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r4k_switch.S b/arch/mips/kernel/r4k_switch.S
index 921238a..078de5e 100644
--- a/arch/mips/kernel/r4k_switch.S
+++ b/arch/mips/kernel/r4k_switch.S
@@ -69,7 +69,7 @@
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-07 11:14 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-07 11:14 UTC (permalink / raw)
To: Ralf Baechle; +Cc: James Hogan, Gregory Fong, linux-mips, stable
Commit 1400eb6 (MIPS: r4k,octeon,r2300: stack protector: change canary
per task) was merged in v3.11 and introduced assembly in the MIPS resume
functions to update the value of the current canary in
__stack_chk_guard. However it used PTR_L resulting in a load of the
canary value, instead of PTR_LA to construct its address. The value is
intended to be random but is then treated as an address in the
subsequent LONG_S (store).
This was observed to cause a fault and panic:
CPU 0 Unable to handle kernel paging request at virtual address 139fea20, epc == 8000cc0c, ra == 8034f2a4
Oops[#1]:
...
$24 : 139fea20 1e1f7cb6
...
Call Trace:
[<8000cc0c>] resume+0xac/0x118
[<8034f2a4>] __schedule+0x5f8/0x78c
[<8034f4e0>] schedule_preempt_disabled+0x20/0x2c
[<80348eec>] rest_init+0x74/0x84
[<804dc990>] start_kernel+0x43c/0x454
Code: 3c18804b 8f184030 8cb901f8 <af190000> 00c0e021 8cb002f0 8cb102f4 8cb202f8 8cb302fc
This can also be forced by modifying
arch/mips/include/asm/stackprotector.h so that the default
__stack_chk_guard value is more likely to be a bad (or unaligned)
pointer.
Fix it to use PTR_LA instead, to load the address of the canary value,
which the LONG_S can then use to write into it.
Reported-by: bobjones (via #mipslinux on IRC)
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: <stable@vger.kernel.org> #3.11
---
Ralf: This is a regression in v3.11, so please consider for v3.12.
(Sorry, resent to Cc stable correctly)
arch/mips/kernel/octeon_switch.S | 2 +-
arch/mips/kernel/r2300_switch.S | 2 +-
arch/mips/kernel/r4k_switch.S | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/kernel/octeon_switch.S b/arch/mips/kernel/octeon_switch.S
index 4204d76..029e002 100644
--- a/arch/mips/kernel/octeon_switch.S
+++ b/arch/mips/kernel/octeon_switch.S
@@ -73,7 +73,7 @@
3:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r2300_switch.S b/arch/mips/kernel/r2300_switch.S
index 38af83f..20b7b04 100644
--- a/arch/mips/kernel/r2300_switch.S
+++ b/arch/mips/kernel/r2300_switch.S
@@ -67,7 +67,7 @@ LEAF(resume)
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r4k_switch.S b/arch/mips/kernel/r4k_switch.S
index 921238a..078de5e 100644
--- a/arch/mips/kernel/r4k_switch.S
+++ b/arch/mips/kernel/r4k_switch.S
@@ -69,7 +69,7 @@
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: stack protector: Fix per-task canary switch
2013-10-07 11:14 ` James Hogan
(?)
@ 2013-10-07 12:48 ` Ralf Baechle
2013-10-10 23:16 ` Greg KH
-1 siblings, 1 reply; 9+ messages in thread
From: Ralf Baechle @ 2013-10-07 12:48 UTC (permalink / raw)
To: James Hogan; +Cc: Gregory Fong, linux-mips, stable
On Mon, Oct 07, 2013 at 12:14:26PM +0100, James Hogan wrote:
> Ralf: This is a regression in v3.11, so please consider for v3.12.
Applied, will send to Linus with the next pull request.
stable folks - please apply to 3.12-stable.
Thanks!
Ralf
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: stack protector: Fix per-task canary switch
2013-10-07 12:48 ` Ralf Baechle
@ 2013-10-10 23:16 ` Greg KH
2013-10-14 8:40 ` James Hogan
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2013-10-10 23:16 UTC (permalink / raw)
To: Ralf Baechle; +Cc: James Hogan, Gregory Fong, linux-mips, stable
On Mon, Oct 07, 2013 at 02:48:59PM +0200, Ralf Baechle wrote:
> On Mon, Oct 07, 2013 at 12:14:26PM +0100, James Hogan wrote:
>
> > Ralf: This is a regression in v3.11, so please consider for v3.12.
>
> Applied, will send to Linus with the next pull request.
Which would be in time for 3.12-final, right?
> stable folks - please apply to 3.12-stable.
There is no 3.12-stable yet, as 3.12-final isn't out yet.
Confused,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-14 8:40 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-14 8:40 UTC (permalink / raw)
To: Greg KH; +Cc: Ralf Baechle, Gregory Fong, linux-mips, stable
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
Hi Greg,
On 11/10/13 00:16, Greg KH wrote:
> On Mon, Oct 07, 2013 at 02:48:59PM +0200, Ralf Baechle wrote:
>> On Mon, Oct 07, 2013 at 12:14:26PM +0100, James Hogan wrote:
>>
>>> Ralf: This is a regression in v3.11, so please consider for v3.12.
>>
>> Applied, will send to Linus with the next pull request.
>
> Which would be in time for 3.12-final, right?
Yes, it's included in v3.12-rc5 as:
8b3c569a3999a8fd5a819f892525ab5520777c92
>> stable folks - please apply to 3.12-stable.
>
> There is no 3.12-stable yet, as 3.12-final isn't out yet.
>
> Confused,
I think Ralf meant v3.11-stable. It's only needed there as the
regression was introduced in v3.11-rc1.
Thanks
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-14 8:40 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-14 8:40 UTC (permalink / raw)
To: Greg KH; +Cc: Ralf Baechle, Gregory Fong, linux-mips, stable
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
Hi Greg,
On 11/10/13 00:16, Greg KH wrote:
> On Mon, Oct 07, 2013 at 02:48:59PM +0200, Ralf Baechle wrote:
>> On Mon, Oct 07, 2013 at 12:14:26PM +0100, James Hogan wrote:
>>
>>> Ralf: This is a regression in v3.11, so please consider for v3.12.
>>
>> Applied, will send to Linus with the next pull request.
>
> Which would be in time for 3.12-final, right?
Yes, it's included in v3.12-rc5 as:
8b3c569a3999a8fd5a819f892525ab5520777c92
>> stable folks - please apply to 3.12-stable.
>
> There is no 3.12-stable yet, as 3.12-final isn't out yet.
>
> Confused,
I think Ralf meant v3.11-stable. It's only needed there as the
regression was introduced in v3.11-rc1.
Thanks
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MIPS: stack protector: Fix per-task canary switch
2013-10-14 8:40 ` James Hogan
(?)
@ 2013-10-14 10:27 ` Ralf Baechle
-1 siblings, 0 replies; 9+ messages in thread
From: Ralf Baechle @ 2013-10-14 10:27 UTC (permalink / raw)
To: James Hogan; +Cc: Greg KH, Gregory Fong, linux-mips, stable
On Mon, Oct 14, 2013 at 09:40:27AM +0100, James Hogan wrote:
> On 11/10/13 00:16, Greg KH wrote:
> > On Mon, Oct 07, 2013 at 02:48:59PM +0200, Ralf Baechle wrote:
> >> On Mon, Oct 07, 2013 at 12:14:26PM +0100, James Hogan wrote:
> >>
> >>> Ralf: This is a regression in v3.11, so please consider for v3.12.
> >>
> >> Applied, will send to Linus with the next pull request.
> >
> > Which would be in time for 3.12-final, right?
>
> Yes, it's included in v3.12-rc5 as:
> 8b3c569a3999a8fd5a819f892525ab5520777c92
>
> >> stable folks - please apply to 3.12-stable.
> >
> > There is no 3.12-stable yet, as 3.12-final isn't out yet.
> >
> > Confused,
>
> I think Ralf meant v3.11-stable. It's only needed there as the
> regression was introduced in v3.11-rc1.
Yes, indeed. Greg, sorry for the confusion.
Thanks,
Ralf
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-07 11:08 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-07 11:08 UTC (permalink / raw)
To: Ralf Baechle; +Cc: James Hogan, Gregory Fong, linux-mips, stable
Commit 1400eb6 (MIPS: r4k,octeon,r2300: stack protector: change canary
per task) was merged in v3.11 and introduced assembly in the MIPS resume
functions to update the value of the current canary in
__stack_chk_guard. However it used PTR_L resulting in a load of the
canary value, instead of PTR_LA to construct its address. The value is
intended to be random but is then treated as an address in the
subsequent LONG_S (store).
This was observed to cause a fault and panic:
CPU 0 Unable to handle kernel paging request at virtual address 139fea20, epc == 8000cc0c, ra == 8034f2a4
Oops[#1]:
...
$24 : 139fea20 1e1f7cb6
...
Call Trace:
[<8000cc0c>] resume+0xac/0x118
[<8034f2a4>] __schedule+0x5f8/0x78c
[<8034f4e0>] schedule_preempt_disabled+0x20/0x2c
[<80348eec>] rest_init+0x74/0x84
[<804dc990>] start_kernel+0x43c/0x454
Code: 3c18804b 8f184030 8cb901f8 <af190000> 00c0e021 8cb002f0 8cb102f4 8cb202f8 8cb302fc
This can also be forced by modifying
arch/mips/include/asm/stackprotector.h so that the default
__stack_chk_guard value is more likely to be a bad (or unaligned)
pointer.
Fix it to use PTR_LA instead, to load the address of the canary value,
which the LONG_S can then use to write into it.
Reported-by: bobjones (via #mipslinux on IRC)
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: stable@vger.kernel.org #3.11
---
Ralf: This is a regression in v3.11, so please consider for v3.12.
arch/mips/kernel/octeon_switch.S | 2 +-
arch/mips/kernel/r2300_switch.S | 2 +-
arch/mips/kernel/r4k_switch.S | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/kernel/octeon_switch.S b/arch/mips/kernel/octeon_switch.S
index 4204d76..029e002 100644
--- a/arch/mips/kernel/octeon_switch.S
+++ b/arch/mips/kernel/octeon_switch.S
@@ -73,7 +73,7 @@
3:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r2300_switch.S b/arch/mips/kernel/r2300_switch.S
index 38af83f..20b7b04 100644
--- a/arch/mips/kernel/r2300_switch.S
+++ b/arch/mips/kernel/r2300_switch.S
@@ -67,7 +67,7 @@ LEAF(resume)
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r4k_switch.S b/arch/mips/kernel/r4k_switch.S
index 921238a..078de5e 100644
--- a/arch/mips/kernel/r4k_switch.S
+++ b/arch/mips/kernel/r4k_switch.S
@@ -69,7 +69,7 @@
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] MIPS: stack protector: Fix per-task canary switch
@ 2013-10-07 11:08 ` James Hogan
0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2013-10-07 11:08 UTC (permalink / raw)
To: Ralf Baechle; +Cc: James Hogan, Gregory Fong, linux-mips, stable
Commit 1400eb6 (MIPS: r4k,octeon,r2300: stack protector: change canary
per task) was merged in v3.11 and introduced assembly in the MIPS resume
functions to update the value of the current canary in
__stack_chk_guard. However it used PTR_L resulting in a load of the
canary value, instead of PTR_LA to construct its address. The value is
intended to be random but is then treated as an address in the
subsequent LONG_S (store).
This was observed to cause a fault and panic:
CPU 0 Unable to handle kernel paging request at virtual address 139fea20, epc == 8000cc0c, ra == 8034f2a4
Oops[#1]:
...
$24 : 139fea20 1e1f7cb6
...
Call Trace:
[<8000cc0c>] resume+0xac/0x118
[<8034f2a4>] __schedule+0x5f8/0x78c
[<8034f4e0>] schedule_preempt_disabled+0x20/0x2c
[<80348eec>] rest_init+0x74/0x84
[<804dc990>] start_kernel+0x43c/0x454
Code: 3c18804b 8f184030 8cb901f8 <af190000> 00c0e021 8cb002f0 8cb102f4 8cb202f8 8cb302fc
This can also be forced by modifying
arch/mips/include/asm/stackprotector.h so that the default
__stack_chk_guard value is more likely to be a bad (or unaligned)
pointer.
Fix it to use PTR_LA instead, to load the address of the canary value,
which the LONG_S can then use to write into it.
Reported-by: bobjones (via #mipslinux on IRC)
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Gregory Fong <gregory.0xf0@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: stable@vger.kernel.org #3.11
---
Ralf: This is a regression in v3.11, so please consider for v3.12.
arch/mips/kernel/octeon_switch.S | 2 +-
arch/mips/kernel/r2300_switch.S | 2 +-
arch/mips/kernel/r4k_switch.S | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/mips/kernel/octeon_switch.S b/arch/mips/kernel/octeon_switch.S
index 4204d76..029e002 100644
--- a/arch/mips/kernel/octeon_switch.S
+++ b/arch/mips/kernel/octeon_switch.S
@@ -73,7 +73,7 @@
3:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r2300_switch.S b/arch/mips/kernel/r2300_switch.S
index 38af83f..20b7b04 100644
--- a/arch/mips/kernel/r2300_switch.S
+++ b/arch/mips/kernel/r2300_switch.S
@@ -67,7 +67,7 @@ LEAF(resume)
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
diff --git a/arch/mips/kernel/r4k_switch.S b/arch/mips/kernel/r4k_switch.S
index 921238a..078de5e 100644
--- a/arch/mips/kernel/r4k_switch.S
+++ b/arch/mips/kernel/r4k_switch.S
@@ -69,7 +69,7 @@
1:
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
- PTR_L t8, __stack_chk_guard
+ PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
#endif
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-10-14 10:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-07 11:14 [PATCH] MIPS: stack protector: Fix per-task canary switch James Hogan
2013-10-07 11:14 ` James Hogan
2013-10-07 12:48 ` Ralf Baechle
2013-10-10 23:16 ` Greg KH
2013-10-14 8:40 ` James Hogan
2013-10-14 8:40 ` James Hogan
2013-10-14 10:27 ` Ralf Baechle
-- strict thread matches above, loose matches on Subject: below --
2013-10-07 11:08 James Hogan
2013-10-07 11:08 ` James Hogan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.