| Message ID | 20240410073044.23294-1-boy.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] arm: kasan: clear stale stack poison | expand |
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
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...
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")
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.
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")
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.
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)