arm64: increase stack size for KASAN_EXTRA
diff mbox series

Message ID 20181207223449.38808-1-cai@lca.pw
State New
Headers show
Series
  • arm64: increase stack size for KASAN_EXTRA
Related show

Commit Message

Qian Cai Dec. 7, 2018, 10:34 p.m. UTC
If the kernel is configured with KASAN_EXTRA, the stack size is
increasted significantly due to enable this option will set
-fstack-reuse to "none" in GCC [1]. As the results, it could trigger
stack overrun quite often with 32k stack size compiled using GCC 8. For
example, this reproducer

https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/\
syscalls/madvise/madvise06.c

could trigger a "corrupted stack end detected inside scheduler" very
reliably with CONFIG_SCHED_STACK_END_CHECK enabled. Also, See other bug
reports,

https://lore.kernel.org/lkml/1542144497.12945.29.camel@gmx.us/
https://lore.kernel.org/lkml/721E7B42-2D55-4866-9C1A-3E8D64F33F9C@gmx.us/

There are just too many functions that could have a large stack with
KASAN_EXTRA due to large local variables that have been called over and
over again without being able to reuse the stacks. Some noticiable ones
are,

size
7536 shrink_inactive_list
7440 shrink_page_list
6560 fscache_stats_show
3920 jbd2_journal_commit_transaction
3216 try_to_unmap_one
3072 migrate_page_move_mapping
3584 migrate_misplaced_transhuge_page
3920 ip_vs_lblcr_schedule
4304 lpfc_nvme_info_show
3888 lpfc_debugfs_nvmestat_data.constprop

There are other 49 functions are over 2k in size while compiling kernel
with "-Wframe-larger-than=" on this machine. Hence, it is too much work
to change Makefiles for each object to compile without
-fsanitize-address-use-after-scope individually.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c23

Signed-off-by: Qian Cai <cai@lca.pw>
---
 arch/arm64/include/asm/memory.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann Dec. 11, 2018, 12:42 p.m. UTC | #1
On Fri, Dec 7, 2018 at 11:35 PM Qian Cai <cai@lca.pw> wrote:
>
> If the kernel is configured with KASAN_EXTRA, the stack size is
> increasted significantly due to enable this option will set
> -fstack-reuse to "none" in GCC [1]. As the results, it could trigger
> stack overrun quite often with 32k stack size compiled using GCC 8. For
> example, this reproducer

>
> size
> 7536 shrink_inactive_list
> 7440 shrink_page_list
> 6560 fscache_stats_show
> 3920 jbd2_journal_commit_transaction
> 3216 try_to_unmap_one
> 3072 migrate_page_move_mapping
> 3584 migrate_misplaced_transhuge_page
> 3920 ip_vs_lblcr_schedule
> 4304 lpfc_nvme_info_show
> 3888 lpfc_debugfs_nvmestat_data.constprop
>
> There are other 49 functions are over 2k in size while compiling kernel
> with "-Wframe-larger-than=" on this machine. Hence, it is too much work
> to change Makefiles for each object to compile without
> -fsanitize-address-use-after-scope individually.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c23

Could you clarify: are the numbers you see with or without the bugfix
from that bugzilla?

       Arnd
Qian Cai Dec. 11, 2018, 5:18 p.m. UTC | #2
On Tue, 2018-12-11 at 13:42 +0100, Arnd Bergmann wrote:
> On Fri, Dec 7, 2018 at 11:35 PM Qian Cai <cai@lca.pw> wrote:
> > 
> > If the kernel is configured with KASAN_EXTRA, the stack size is
> > increasted significantly due to enable this option will set
> > -fstack-reuse to "none" in GCC [1]. As the results, it could trigger
> > stack overrun quite often with 32k stack size compiled using GCC 8. For
> > example, this reproducer
> > 
> > size
> > 7536 shrink_inactive_list
> > 7440 shrink_page_list
> > 6560 fscache_stats_show
> > 3920 jbd2_journal_commit_transaction
> > 3216 try_to_unmap_one
> > 3072 migrate_page_move_mapping
> > 3584 migrate_misplaced_transhuge_page
> > 3920 ip_vs_lblcr_schedule
> > 4304 lpfc_nvme_info_show
> > 3888 lpfc_debugfs_nvmestat_data.constprop
> > 
> > There are other 49 functions are over 2k in size while compiling kernel
> > with "-Wframe-larger-than=" on this machine. Hence, it is too much work
> > to change Makefiles for each object to compile without
> > -fsanitize-address-use-after-scope individually.
> > 
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c23
> 
> Could you clarify: are the numbers you see with or without the bugfix
> from that bugzilla?
> 

The numbers were from GCC8 which does NOT contain this patch [1].

GCC9 is awesome which reduced the numbers in half even for KASAN_EXTRA. Only
thing is that GCC9 has not been officially released yet, so it is a bit
inconvenient for users need to compile the compiler by themselves first.

I am fine either way to drop this patch or keep it until GCC9 is GA.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c35
Arnd Bergmann Dec. 11, 2018, 9:43 p.m. UTC | #3
On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
>
> On Tue, 2018-12-11 at 13:42 +0100, Arnd Bergmann wrote:
> > On Fri, Dec 7, 2018 at 11:35 PM Qian Cai <cai@lca.pw> wrote:
> > >
> > > If the kernel is configured with KASAN_EXTRA, the stack size is
> > > increasted significantly due to enable this option will set
> > > -fstack-reuse to "none" in GCC [1]. As the results, it could trigger
> > > stack overrun quite often with 32k stack size compiled using GCC 8. For
> > > example, this reproducer
> > >
> > > size
> > > 7536 shrink_inactive_list
> > > 7440 shrink_page_list
> > > 6560 fscache_stats_show
> > > 3920 jbd2_journal_commit_transaction
> > > 3216 try_to_unmap_one
> > > 3072 migrate_page_move_mapping
> > > 3584 migrate_misplaced_transhuge_page
> > > 3920 ip_vs_lblcr_schedule
> > > 4304 lpfc_nvme_info_show
> > > 3888 lpfc_debugfs_nvmestat_data.constprop
> > >
> > > There are other 49 functions are over 2k in size while compiling kernel
> > > with "-Wframe-larger-than=" on this machine. Hence, it is too much work
> > > to change Makefiles for each object to compile without
> > > -fsanitize-address-use-after-scope individually.
> > >
> > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c23
> >
> > Could you clarify: are the numbers you see with or without the bugfix
> > from that bugzilla?
> >
>
> The numbers were from GCC8 which does NOT contain this patch [1].
>
> GCC9 is awesome which reduced the numbers in half even for KASAN_EXTRA. Only
> thing is that GCC9 has not been officially released yet, so it is a bit
> inconvenient for users need to compile the compiler by themselves first.
>
> I am fine either way to drop this patch or keep it until GCC9 is GA.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c35

Maybe we can make the constant depend on the compiler version?
It may also be possible to reduce the KASAN_THREAD_SHIFT
constant for the normal case with gcc-9 and go back to the
default frame size then.

       Arnd
Qian Cai Dec. 11, 2018, 9:52 p.m. UTC | #4
On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
> > 
> > On Tue, 2018-12-11 at 13:42 +0100, Arnd Bergmann wrote:
> > > On Fri, Dec 7, 2018 at 11:35 PM Qian Cai <cai@lca.pw> wrote:
> > > > 
> > > > If the kernel is configured with KASAN_EXTRA, the stack size is
> > > > increasted significantly due to enable this option will set
> > > > -fstack-reuse to "none" in GCC [1]. As the results, it could trigger
> > > > stack overrun quite often with 32k stack size compiled using GCC 8. For
> > > > example, this reproducer
> > > > 
> > > > size
> > > > 7536 shrink_inactive_list
> > > > 7440 shrink_page_list
> > > > 6560 fscache_stats_show
> > > > 3920 jbd2_journal_commit_transaction
> > > > 3216 try_to_unmap_one
> > > > 3072 migrate_page_move_mapping
> > > > 3584 migrate_misplaced_transhuge_page
> > > > 3920 ip_vs_lblcr_schedule
> > > > 4304 lpfc_nvme_info_show
> > > > 3888 lpfc_debugfs_nvmestat_data.constprop
> > > > 
> > > > There are other 49 functions are over 2k in size while compiling kernel
> > > > with "-Wframe-larger-than=" on this machine. Hence, it is too much work
> > > > to change Makefiles for each object to compile without
> > > > -fsanitize-address-use-after-scope individually.
> > > > 
> > > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c23
> > > 
> > > Could you clarify: are the numbers you see with or without the bugfix
> > > from that bugzilla?
> > > 
> > 
> > The numbers were from GCC8 which does NOT contain this patch [1].
> > 
> > GCC9 is awesome which reduced the numbers in half even for KASAN_EXTRA. Only
> > thing is that GCC9 has not been officially released yet, so it is a bit
> > inconvenient for users need to compile the compiler by themselves first.
> > 
> > I am fine either way to drop this patch or keep it until GCC9 is GA.
> > 
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c35
> 
> Maybe we can make the constant depend on the compiler version?

I am not too keen to do the version-check considering some LTS versions could
just back-port those patches and the render the version-check incorrectly.

> It may also be possible to reduce the KASAN_THREAD_SHIFT
> constant for the normal case with gcc-9 and go back to the
> default frame size then.
Arnd Bergmann Dec. 11, 2018, 9:56 p.m. UTC | #5
On Tue, Dec 11, 2018 at 10:52 PM Qian Cai <cai@lca.pw> wrote:
> On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> > On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:

> I am not too keen to do the version-check considering some LTS versions could
> just back-port those patches and the render the version-check incorrectly.

I'm not following what the problem is. Do you mean distro versions gcc
with the compiler bugfix, or LTS kernel versions?

For backported kernel fixes, this doesn't seem to be any different
from other kernel changes that might be incorrect, but a straight
backport of that kernel patch should still do the right thing in older
kernels, and set the frame size higher for old compilers but not
new ones.

    Arnd
Qian Cai Dec. 11, 2018, 9:59 p.m. UTC | #6
On Tue, 2018-12-11 at 22:56 +0100, Arnd Bergmann wrote:
> On Tue, Dec 11, 2018 at 10:52 PM Qian Cai <cai@lca.pw> wrote:
> > On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> > > On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
> > I am not too keen to do the version-check considering some LTS versions
> > could
> > just back-port those patches and the render the version-check incorrectly.
> 
> I'm not following what the problem is. Do you mean distro versions gcc
> with the compiler bugfix, or LTS kernel versions?
> 

I mean distro versions of GCC where the version is still 8 but keep back-porting 
tons of patches.
Arnd Bergmann Dec. 11, 2018, 10:12 p.m. UTC | #7
On Tue, Dec 11, 2018 at 10:59 PM Qian Cai <cai@lca.pw> wrote:
>
> On Tue, 2018-12-11 at 22:56 +0100, Arnd Bergmann wrote:
> > On Tue, Dec 11, 2018 at 10:52 PM Qian Cai <cai@lca.pw> wrote:
> > > On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> > > > On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
> > > I am not too keen to do the version-check considering some LTS versions
> > > could
> > > just back-port those patches and the render the version-check incorrectly.
> >
> > I'm not following what the problem is. Do you mean distro versions gcc
> > with the compiler bugfix, or LTS kernel versions?
> >
>
> I mean distro versions of GCC where the version is still 8 but keep back-porting
> tons of patches.

Ok, but in that case, checking the version would still be no worse
than your current patch, the only difference is that for users of a
fixed older gcc, the kernel would use more stack than it needs.

        Arnd
Qian Cai Dec. 11, 2018, 10:22 p.m. UTC | #8
On Tue, 2018-12-11 at 23:12 +0100, Arnd Bergmann wrote:
> On Tue, Dec 11, 2018 at 10:59 PM Qian Cai <cai@lca.pw> wrote:
> > 
> > On Tue, 2018-12-11 at 22:56 +0100, Arnd Bergmann wrote:
> > > On Tue, Dec 11, 2018 at 10:52 PM Qian Cai <cai@lca.pw> wrote:
> > > > On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> > > > > On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
> > > > 
> > > > I am not too keen to do the version-check considering some LTS versions
> > > > could
> > > > just back-port those patches and the render the version-check
> > > > incorrectly.
> > > 
> > > I'm not following what the problem is. Do you mean distro versions gcc
> > > with the compiler bugfix, or LTS kernel versions?
> > > 
> > 
> > I mean distro versions of GCC where the version is still 8 but keep back-
> > porting
> > tons of patches.
> 
> Ok, but in that case, checking the version would still be no worse
> than your current patch, the only difference is that for users of a
> fixed older gcc, the kernel would use more stack than it needs.
> 

I am thinking about something it is probably best just waiting for those major
distors to complete upgrading to GCC9 or back-porting those stack reduction
patches first. Then, it is good time to tie up loose ends for those default
stack sizes in all combinations.
Arnd Bergmann Dec. 11, 2018, 11:06 p.m. UTC | #9
On Tue, Dec 11, 2018 at 11:22 PM Qian Cai <cai@lca.pw> wrote:
>
> On Tue, 2018-12-11 at 23:12 +0100, Arnd Bergmann wrote:
> > On Tue, Dec 11, 2018 at 10:59 PM Qian Cai <cai@lca.pw> wrote:
> > >
> > > On Tue, 2018-12-11 at 22:56 +0100, Arnd Bergmann wrote:
> > > > On Tue, Dec 11, 2018 at 10:52 PM Qian Cai <cai@lca.pw> wrote:
> > > > > On Tue, 2018-12-11 at 22:43 +0100, Arnd Bergmann wrote:
> > > > > > On Tue, Dec 11, 2018 at 6:18 PM Qian Cai <cai@lca.pw> wrote:
> > > > >
> > > > > I am not too keen to do the version-check considering some LTS versions
> > > > > could
> > > > > just back-port those patches and the render the version-check
> > > > > incorrectly.
> > > >
> > > > I'm not following what the problem is. Do you mean distro versions gcc
> > > > with the compiler bugfix, or LTS kernel versions?
> > > >
> > >
> > > I mean distro versions of GCC where the version is still 8 but keep back-
> > > porting
> > > tons of patches.
> >
> > Ok, but in that case, checking the version would still be no worse
> > than your current patch, the only difference is that for users of a
> > fixed older gcc, the kernel would use more stack than it needs.
> >
>
> I am thinking about something it is probably best just waiting for those major
> distors to complete upgrading to GCC9 or back-porting those stack reduction
> patches first. Then, it is good time to tie up loose ends for those default
> stack sizes in all combinations.

I was basically trying to make sure we don't forget it when it gets to that.

Another alternative would be to just disable KASAN_EXTRA now
for gcc versions before 9, which essentially means for everyone,
but then we get it back once a working version gets released. As
I understand, this kasan option is actually fairly useless given its
cost, so very few people would miss it.

On a related note, I think we have to turn off asan-stack entirely
on all released clang versions. asan-stack in general is much more
useful than the use-after-scope check, but we clang produces some
very large stack frames with it and we probably can't even work
around it with KASAN_THREAD_SHIFT=2 but would need even
more than that otherwise.

         Arnd
Qian Cai Dec. 12, 2018, 3:54 a.m. UTC | #10
On 12/11/18 6:06 PM, Arnd Bergmann wrote:
>> I am thinking about something it is probably best just waiting for those major
>> distors to complete upgrading to GCC9 or back-porting those stack reduction
>> patches first. Then, it is good time to tie up loose ends for those default
>> stack sizes in all combinations.
> 
> I was basically trying to make sure we don't forget it when it gets to that.

I added a reminder in my calendar to check the GCC9 adoption in Q2 FY19.

> 
> Another alternative would be to just disable KASAN_EXTRA now
> for gcc versions before 9, which essentially means for everyone,
> but then we get it back once a working version gets released. As
> I understand, this kasan option is actually fairly useless given its
> cost, so very few people would miss it.
> 
> On a related note, I think we have to turn off asan-stack entirely
> on all released clang versions. asan-stack in general is much more
> useful than the use-after-scope check, but we clang produces some
> very large stack frames with it and we probably can't even work
> around it with KASAN_THREAD_SHIFT=2 but would need even
> more than that otherwise.
> 
>          Arnd
>

Patch
diff mbox series

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..56562ff01076 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -76,12 +76,17 @@ 
 /*
  * KASAN requires 1/8th of the kernel virtual address space for the shadow
  * region. KASAN can bloat the stack significantly, so double the (minimum)
- * stack size when KASAN is in use.
+ * stack size when KASAN is in use, and then double it again if KASAN_EXTRA is
+ * on.
  */
 #ifdef CONFIG_KASAN
 #define KASAN_SHADOW_SCALE_SHIFT 3
 #define KASAN_SHADOW_SIZE	(UL(1) << (VA_BITS - KASAN_SHADOW_SCALE_SHIFT))
+#ifdef CONFIG_KASAN_EXTRA
+#define KASAN_THREAD_SHIFT	2
+#else
 #define KASAN_THREAD_SHIFT	1
+#endif /* CONFIG_KASAN_EXTRA */
 #else
 #define KASAN_SHADOW_SIZE	(0)
 #define KASAN_THREAD_SHIFT	0