Linux-mediatek Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] arm: kasan: clear stale stack poison
@ 2024-04-10  7:30 boy.wu
  2024-04-12  8:37 ` Linus Walleij
  0 siblings, 1 reply; 7+ messages in thread
From: boy.wu @ 2024-04-10  7:30 UTC (permalink / raw
  To: Mark Rutland, Andrey Ryabinin, Linus Walleij
  Cc: Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov,
	Vincenzo Frascino, kasan-dev, Russell King, Matthias Brugger,
	AngeloGioacchino Del Regno, Boy Wu, linux-arm-kernel,
	linux-kernel, linux-mediatek, Iverlin Wang, Light Chen

From: Boy Wu <boy.wu@mediatek.com>

We found below OOB crash:

[   33.452494] ==================================================================
[   33.453513] BUG: KASAN: stack-out-of-bounds in refresh_cpu_vm_stats.constprop.0+0xcc/0x2ec
[   33.454660] Write of size 164 at addr c1d03d30 by task swapper/0/0
[   33.455515]
[   33.455767] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G           O       6.1.25-mainline #1
[   33.456880] Hardware name: Generic DT based system
[   33.457555]  unwind_backtrace from show_stack+0x18/0x1c
[   33.458326]  show_stack from dump_stack_lvl+0x40/0x4c
[   33.459072]  dump_stack_lvl from print_report+0x158/0x4a4
[   33.459863]  print_report from kasan_report+0x9c/0x148
[   33.460616]  kasan_report from kasan_check_range+0x94/0x1a0
[   33.461424]  kasan_check_range from memset+0x20/0x3c
[   33.462157]  memset from refresh_cpu_vm_stats.constprop.0+0xcc/0x2ec
[   33.463064]  refresh_cpu_vm_stats.constprop.0 from tick_nohz_idle_stop_tick+0x180/0x53c
[   33.464181]  tick_nohz_idle_stop_tick from do_idle+0x264/0x354
[   33.465029]  do_idle from cpu_startup_entry+0x20/0x24
[   33.465769]  cpu_startup_entry from rest_init+0xf0/0xf4
[   33.466528]  rest_init from arch_post_acpi_subsys_init+0x0/0x18
[   33.467397]
[   33.467644] The buggy address belongs to stack of task swapper/0/0
[   33.468493]  and is located at offset 112 in frame:
[   33.469172]  refresh_cpu_vm_stats.constprop.0+0x0/0x2ec
[   33.469917]
[   33.470165] This frame has 2 objects:
[   33.470696]  [32, 76) 'global_zone_diff'
[   33.470729]  [112, 276) 'global_node_diff'
[   33.471294]
[   33.472095] The buggy address belongs to the physical page:
[   33.472862] page:3cd72da8 refcount:1 mapcount:0 mapping:00000000 index:0x0 pfn:0x41d03
[   33.473944] flags: 0x1000(reserved|zone=0)
[   33.474565] raw: 00001000 ed741470 ed741470 00000000 00000000 00000000 ffffffff 00000001
[   33.475656] raw: 00000000
[   33.476050] page dumped because: kasan: bad access detected
[   33.476816]
[   33.477061] Memory state around the buggy address:
[   33.477732]  c1d03c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   33.478630]  c1d03c80: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
[   33.479526] >c1d03d00: 00 04 f2 f2 f2 f2 00 00 00 00 00 00 f1 f1 f1 f1
[   33.480415]                                                ^
[   33.481195]  c1d03d80: 00 00 00 00 00 00 00 00 00 00 04 f3 f3 f3 f3 f3
[   33.482088]  c1d03e00: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
[   33.482978] ==================================================================

We find the root cause of this OOB is that arm does not clear stale stack
poison in the case of cpuidle.

This patch refer to arch/arm64/kernel/sleep.S to resolve this issue.

From cited commit [1] that explain the problem

Functions which the compiler has instrumented for KASAN place poison on
the stack shadow upon entry and remove this poison prior to returning.

In the case of cpuidle, CPUs exit the kernel a number of levels deep in
C code.  Any instrumented functions on this critical path will leave
portions of the stack shadow poisoned.

If CPUs lose context and return to the kernel via a cold path, we
restore a prior context saved in __cpu_suspend_enter are forgotten, and
we never remove the poison they placed in the stack shadow area by
functions calls between this and the actual exit of the kernel.

Thus, (depending on stackframe layout) subsequent calls to instrumented
functions may hit this stale poison, resulting in (spurious) KASAN
splats to the console.

To avoid this, clear any stale poison from the idle thread for a CPU
prior to bringing a CPU online.

From cited commit [2]

Extend to check for CONFIG_KASAN_STACK

[1] commit 0d97e6d8024c ("arm64: kasan: clear stale stack poison")
[2] commit d56a9ef84bd0 ("kasan, arm64: unpoison stack only with CONFIG_KASAN_STACK")

Signed-off-by: Boy Wu <boy.wu@mediatek.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
v2 - Add commit message that reviewer mention
---
 arch/arm/kernel/sleep.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
index a86a1d4f3461..93afd1005b43 100644
--- a/arch/arm/kernel/sleep.S
+++ b/arch/arm/kernel/sleep.S
@@ -127,6 +127,10 @@ cpu_resume_after_mmu:
 	instr_sync
 #endif
 	bl	cpu_init		@ restore the und/abt/irq banked regs
+#if defined(CONFIG_KASAN) && defined(CONFIG_KASAN_STACK)
+	mov	r0, sp
+	bl	kasan_unpoison_task_stack_below
+#endif
 	mov	r0, #0			@ return zero on success
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(cpu_resume_after_mmu)
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-10  7:30 [PATCH v2] arm: kasan: clear stale stack poison boy.wu
@ 2024-04-12  8:37 ` Linus Walleij
  2024-04-28 14:45   ` Russell King (Oracle)
  0 siblings, 1 reply; 7+ messages in thread
From: Linus Walleij @ 2024-04-12  8:37 UTC (permalink / raw
  To: boy.wu
  Cc: Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
	Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, kasan-dev,
	Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
	linux-arm-kernel, linux-kernel, linux-mediatek, Iverlin Wang,
	Light Chen

On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:

> From: Boy Wu <boy.wu@mediatek.com>
>
> We found below OOB crash:

Thanks for digging in!

Pleas put this patch into Russell's patch tracker so he can apply it:
https://www.armlinux.org.uk/developer/patches/

Yours,
Linus Walleij


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-12  8:37 ` Linus Walleij
@ 2024-04-28 14:45   ` Russell King (Oracle)
  2024-04-29  7:51     ` Boy Wu (吳勃誼)
  0 siblings, 1 reply; 7+ messages in thread
From: Russell King (Oracle) @ 2024-04-28 14:45 UTC (permalink / raw
  To: Linus Walleij
  Cc: boy.wu, Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
	Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, kasan-dev,
	Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-kernel, linux-mediatek, Iverlin Wang, Light Chen

On Fri, Apr 12, 2024 at 10:37:06AM +0200, Linus Walleij wrote:
> On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:
> 
> > From: Boy Wu <boy.wu@mediatek.com>
> >
> > We found below OOB crash:
> 
> Thanks for digging in!
> 
> Pleas put this patch into Russell's patch tracker so he can apply it:
> https://www.armlinux.org.uk/developer/patches/

Is this a bug fix? If so, having a Fixes: tag would be nice...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-28 14:45   ` Russell King (Oracle)
@ 2024-04-29  7:51     ` Boy Wu (吳勃誼)
  2024-04-29 12:42       ` Russell King (Oracle)
  0 siblings, 1 reply; 7+ messages in thread
From: Boy Wu (吳勃誼) @ 2024-04-29  7:51 UTC (permalink / raw
  To: linux@armlinux.org.uk, linus.walleij@linaro.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	andreyknvl@gmail.com, dvyukov@google.com,
	kasan-dev@googlegroups.com,
	Iverlin Wang (王苳霖), mark.rutland@arm.com,
	Light Chen (陳昱光), ryabinin.a.a@gmail.com,
	linux-arm-kernel@lists.infradead.org, glider@google.com,
	matthias.bgg@gmail.com, vincenzo.frascino@arm.com,
	angelogioacchino.delregno@collabora.com

On Sun, 2024-04-28 at 15:45 +0100, Russell King (Oracle) wrote:
>  On Fri, Apr 12, 2024 at 10:37:06AM +0200, Linus Walleij wrote:
> > On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:
> > 
> > > From: Boy Wu <boy.wu@mediatek.com>
> > >
> > > We found below OOB crash:
> > 
> > Thanks for digging in!
> > 
> > Pleas put this patch into Russell's patch tracker so he can apply
> it:
> > https://www.armlinux.org.uk/developer/patches/
> 
> Is this a bug fix? If so, having a Fixes: tag would be nice...
> 

This is a patch for cpuidle flow when KASAN enable, that is in ARM64
but not in ARM, so add to ARM.

The reference commits did not mention fix any commits.
[1] commit 0d97e6d8024c ("arm64: kasan: clear stale stack poison")
[2] commit d56a9ef84bd0 ("kasan, arm64: unpoison stack only with
CONFIG_KASAN_STACK")


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-29  7:51     ` Boy Wu (吳勃誼)
@ 2024-04-29 12:42       ` Russell King (Oracle)
  2024-04-29 12:57         ` Andrey Ryabinin
  0 siblings, 1 reply; 7+ messages in thread
From: Russell King (Oracle) @ 2024-04-29 12:42 UTC (permalink / raw
  To: Boy Wu (吳勃誼)
  Cc: linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	linux-mediatek@lists.infradead.org, andreyknvl@gmail.com,
	dvyukov@google.com, kasan-dev@googlegroups.com,
	Iverlin Wang (王苳霖), mark.rutland@arm.com,
	Light Chen (陳昱光), ryabinin.a.a@gmail.com,
	linux-arm-kernel@lists.infradead.org, glider@google.com,
	matthias.bgg@gmail.com, vincenzo.frascino@arm.com,
	angelogioacchino.delregno@collabora.com

On Mon, Apr 29, 2024 at 07:51:49AM +0000, Boy Wu (吳勃誼) wrote:
> On Sun, 2024-04-28 at 15:45 +0100, Russell King (Oracle) wrote:
> >  On Fri, Apr 12, 2024 at 10:37:06AM +0200, Linus Walleij wrote:
> > > On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:
> > > 
> > > > From: Boy Wu <boy.wu@mediatek.com>
> > > >
> > > > We found below OOB crash:
> > > 
> > > Thanks for digging in!
> > > 
> > > Pleas put this patch into Russell's patch tracker so he can apply
> > it:
> > > https://www.armlinux.org.uk/developer/patches/
> > 
> > Is this a bug fix? If so, having a Fixes: tag would be nice...
> > 
> 
> This is a patch for cpuidle flow when KASAN enable, that is in ARM64
> but not in ARM, so add to ARM.
> 
> The reference commits did not mention fix any commits.
> [1] commit 0d97e6d8024c ("arm64: kasan: clear stale stack poison")
> [2] commit d56a9ef84bd0 ("kasan, arm64: unpoison stack only with
> CONFIG_KASAN_STACK")

These are not suitable for use as a Fixes: tag because these commits
refer to code in another part of the tree that has nothing to do with
the BUG() dump that is contained within your commit message.

I ask again... Is this a bug fix?

Is it a regression?

Is it something that used to work that no longer works?

When did it break?

Has it always been broken?

Has it been broken since KASAN was introduced on 32-bit ARM?

I'm not applying this commit until I get *proper* answers to these
questions so that I can work out whether this needs to go in -rc
or whether it waits until the next merge window... and whether it
needs a Fixes: tag or not.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-29 12:42       ` Russell King (Oracle)
@ 2024-04-29 12:57         ` Andrey Ryabinin
  2024-04-29 13:11           ` Russell King (Oracle)
  0 siblings, 1 reply; 7+ messages in thread
From: Andrey Ryabinin @ 2024-04-29 12:57 UTC (permalink / raw
  To: Russell King (Oracle), Boy Wu (吳勃誼)
  Cc: linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	linux-mediatek@lists.infradead.org, andreyknvl@gmail.com,
	dvyukov@google.com, kasan-dev@googlegroups.com,
	Iverlin Wang (王苳霖), mark.rutland@arm.com,
	Light Chen (陳昱光),
	linux-arm-kernel@lists.infradead.org, glider@google.com,
	matthias.bgg@gmail.com, vincenzo.frascino@arm.com,
	angelogioacchino.delregno@collabora.com



On 4/29/24 14:42, Russell King (Oracle) wrote:
> On Mon, Apr 29, 2024 at 07:51:49AM +0000, Boy Wu (吳勃誼) wrote:
>> On Sun, 2024-04-28 at 15:45 +0100, Russell King (Oracle) wrote:
>>>  On Fri, Apr 12, 2024 at 10:37:06AM +0200, Linus Walleij wrote:
>>>> On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:
>>>>
>>>>> From: Boy Wu <boy.wu@mediatek.com>
>>>>>
>>>>> We found below OOB crash:
>>>>
>>>> Thanks for digging in!
>>>>
>>>> Pleas put this patch into Russell's patch tracker so he can apply
>>> it:
>>>> https://www.armlinux.org.uk/developer/patches/
>>>
>>> Is this a bug fix? If so, having a Fixes: tag would be nice...
>>>
>>
>> This is a patch for cpuidle flow when KASAN enable, that is in ARM64
>> but not in ARM, so add to ARM.
>>
>> The reference commits did not mention fix any commits.
>> [1] commit 0d97e6d8024c ("arm64: kasan: clear stale stack poison")
>> [2] commit d56a9ef84bd0 ("kasan, arm64: unpoison stack only with
>> CONFIG_KASAN_STACK")
> 
> These are not suitable for use as a Fixes: tag because these commits
> refer to code in another part of the tree that has nothing to do with
> the BUG() dump that is contained within your commit message.
> 
> I ask again... Is this a bug fix?
> 
> Is it a regression?
> 
> Is it something that used to work that no longer works?
> 
> When did it break?
> 
> Has it always been broken?
> 
> Has it been broken since KASAN was introduced on 32-bit ARM?
> 

Yes, this is a bug fix and it has been broken since KASAN was introduced on 32-bit ARM.
So, I think this should be
	Fixes: 5615f69bc209 ("ARM: 9016/2: Initialize the mapping of KASan shadow memory")


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arm: kasan: clear stale stack poison
  2024-04-29 12:57         ` Andrey Ryabinin
@ 2024-04-29 13:11           ` Russell King (Oracle)
  0 siblings, 0 replies; 7+ messages in thread
From: Russell King (Oracle) @ 2024-04-29 13:11 UTC (permalink / raw
  To: Andrey Ryabinin
  Cc: Boy Wu (吳勃誼), linus.walleij@linaro.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	andreyknvl@gmail.com, dvyukov@google.com,
	kasan-dev@googlegroups.com,
	Iverlin Wang (王苳霖), mark.rutland@arm.com,
	Light Chen (陳昱光),
	linux-arm-kernel@lists.infradead.org, glider@google.com,
	matthias.bgg@gmail.com, vincenzo.frascino@arm.com,
	angelogioacchino.delregno@collabora.com

On Mon, Apr 29, 2024 at 02:57:35PM +0200, Andrey Ryabinin wrote:
> On 4/29/24 14:42, Russell King (Oracle) wrote:
> > On Mon, Apr 29, 2024 at 07:51:49AM +0000, Boy Wu (吳勃誼) wrote:
> >> On Sun, 2024-04-28 at 15:45 +0100, Russell King (Oracle) wrote:
> >>>  On Fri, Apr 12, 2024 at 10:37:06AM +0200, Linus Walleij wrote:
> >>>> On Wed, Apr 10, 2024 at 9:31 AM boy.wu <boy.wu@mediatek.com> wrote:
> >>>>
> >>>>> From: Boy Wu <boy.wu@mediatek.com>
> >>>>>
> >>>>> We found below OOB crash:
> >>>>
> >>>> Thanks for digging in!
> >>>>
> >>>> Pleas put this patch into Russell's patch tracker so he can apply
> >>> it:
> >>>> https://www.armlinux.org.uk/developer/patches/
> >>>
> >>> Is this a bug fix? If so, having a Fixes: tag would be nice...
> >>>
> >>
> >> This is a patch for cpuidle flow when KASAN enable, that is in ARM64
> >> but not in ARM, so add to ARM.
> >>
> >> The reference commits did not mention fix any commits.
> >> [1] commit 0d97e6d8024c ("arm64: kasan: clear stale stack poison")
> >> [2] commit d56a9ef84bd0 ("kasan, arm64: unpoison stack only with
> >> CONFIG_KASAN_STACK")
> > 
> > These are not suitable for use as a Fixes: tag because these commits
> > refer to code in another part of the tree that has nothing to do with
> > the BUG() dump that is contained within your commit message.
> > 
> > I ask again... Is this a bug fix?
> > 
> > Is it a regression?
> > 
> > Is it something that used to work that no longer works?
> > 
> > When did it break?
> > 
> > Has it always been broken?
> > 
> > Has it been broken since KASAN was introduced on 32-bit ARM?
> > 
> 
> Yes, this is a bug fix and it has been broken since KASAN was introduced on 32-bit ARM.
> So, I think this should be
> 	Fixes: 5615f69bc209 ("ARM: 9016/2: Initialize the mapping of KASan shadow memory")

Brilliant, thanks! Now merged.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-29 13:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10  7:30 [PATCH v2] arm: kasan: clear stale stack poison boy.wu
2024-04-12  8:37 ` Linus Walleij
2024-04-28 14:45   ` Russell King (Oracle)
2024-04-29  7:51     ` Boy Wu (吳勃誼)
2024-04-29 12:42       ` Russell King (Oracle)
2024-04-29 12:57         ` Andrey Ryabinin
2024-04-29 13:11           ` Russell King (Oracle)

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 read-only IMAP folder(s) and NNTP newsgroup(s).