Messages in this thread | | | Date | Sun, 12 Nov 2023 18:37:24 -0500 | From | matoro <> | Subject | Re: Bisected stability regression in 6.6 |
| |
On 2023-11-12 15:22, Helge Deller wrote: > * matoro <matoro_mailinglist_kernel@matoro.tk>: >> On 2023-11-11 16:27, Sam James wrote: >> > Helge Deller <deller@gmx.de> writes: >> > >> > > On 11/11/23 07:31, matoro wrote: >> > > > Hi Helge, I have bisected a regression in 6.6 which is causing >> > > > userspace segfaults at a significantly increased rate in kernel 6.6. >> > > > There seems to be a pathological case triggered by the ninja build >> > > > tool. The test case I have been using is cmake with ninja backend to >> > > > attempt to build the nghttp2 package. In 6.6, this segfaults, not at >> > > > the same location every time, but with enough reliability that I was >> > > > able to use it as a bisection regression case, including immediately >> > > > after a reboot. In the kernel log, these show up as "trap #15: Data >> > > > TLB miss fault" messages. Now these messages can and do show up in >> > > > 6.5 causing segfaults, but never immediately after a reboot and >> > > > infrequently enough that the system is stable. With kernel 6.6 I am >> > > > completely unable to build nghttp2 under any circumstances. >> > > > >> > > > I have bisected this down to the following commit: >> > > > >> > > > $ git bisect good >> > > > 3033cd4307681c60db6d08f398a64484b36e0b0f is the first bad commit >> > > > commit 3033cd4307681c60db6d08f398a64484b36e0b0f >> > > > Author: Helge Deller <deller@gmx.de> >> > > > Date: Sat Aug 19 00:53:28 2023 +0200 >> > > > >> > > > parisc: Use generic mmap top-down layout and brk randomization >> > > > >> > > > parisc uses a top-down layout by default that exactly fits >> > > > the generic >> > > > functions, so get rid of arch specific code and use the >> > > > generic version >> > > > by selecting ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT. >> > > > >> > > > Note that on parisc the stack always grows up and a "unlimited stack" >> > > > simply means that the value as defined in >> > > > CONFIG_STACK_MAX_DEFAULT_SIZE_MB >> > > > should be used. So RLIM_INFINITY is not an indicator to use >> > > > the legacy >> > > > memory layout. >> > > > >> > > > Signed-off-by: Helge Deller <deller@gmx.de> >> > > > >> > > > arch/parisc/Kconfig | 17 +++++++++++++ >> > > > arch/parisc/kernel/process.c | 14 ----------- >> > > > arch/parisc/kernel/sys_parisc.c | 54 >> > > > +---------------------------------------- >> > > > mm/util.c | 5 +++- >> > > > 4 files changed, 22 insertions(+), 68 deletions(-) >> > > >> > > Thanks for your report! >> > > I think it's quite unlikely that this patch introduces such a bad >> > > regression. > > I was wrong. > Indeed, by switching to the generic implementation with this patch > the calculation of mmap_base is wrong for parisc (because parisc > is the only architecture left where the stack grows upwards). > > Could you please test the patch below. It did fixed the crashes > when building nghttp2 for me. > > Helge > > --- > > From: Helge Deller <deller@gmx.de> > Subject: [PATCH] parisc: Adjust ARCH_MMAP_RND_BITS* to previous values > > Matoro reported various userspace crashes in kernel 6.6 and bisected it to > commit 3033cd430768 ("parisc: Use generic mmap top-down layout and brk > randomization"). > > The problem is, that mmap_base is calculated wrongly for the > stack-grows-upwards case (as on parisc). On parisc, mmap_base is simply just > below the stack start. > > Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk> > Signed-off-by: Helge Deller <deller@gmx.de> > Fixes: 3033cd430768 ("parisc: Use generic mmap top-down layout and brk > randomization") > Cc: <stable@vger.kernel.org> # v6.6+ > > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > index a15ab147af2e..68cbe666510a 100644 > --- a/arch/parisc/Kconfig > +++ b/arch/parisc/Kconfig > @@ -138,11 +138,11 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN > default 8 > > config ARCH_MMAP_RND_BITS_MAX > - default 24 if 64BIT > - default 17 > + default 18 if 64BIT > + default 13 > > config ARCH_MMAP_RND_COMPAT_BITS_MAX > - default 17 > + default 13 > > # unless you want to implement ACPI on PA-RISC ... ;-) > config PM > diff --git a/arch/parisc/include/asm/elf.h b/arch/parisc/include/asm/elf.h > index 140eaa97bf21..2d73d3c3cd37 100644 > --- a/arch/parisc/include/asm/elf.h > +++ b/arch/parisc/include/asm/elf.h > @@ -349,15 +349,7 @@ struct pt_regs; /* forward declaration... */ > > #define ELF_HWCAP 0 > > -/* Masks for stack and mmap randomization */ > -#define BRK_RND_MASK (is_32bit_task() ? 0x07ffUL : 0x3ffffUL) > -#define MMAP_RND_MASK (is_32bit_task() ? 0x1fffUL : 0x3ffffUL) > -#define STACK_RND_MASK MMAP_RND_MASK > - > -struct mm_struct; > -extern unsigned long arch_randomize_brk(struct mm_struct *); > -#define arch_randomize_brk arch_randomize_brk > - > +#define STACK_RND_MASK 0x7ff /* 8MB of VA */ > > #define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1 > struct linux_binprm; > diff --git a/arch/parisc/include/asm/processor.h > b/arch/parisc/include/asm/processor.h > index ff6cbdb6903b..ece4b3046515 100644 > --- a/arch/parisc/include/asm/processor.h > +++ b/arch/parisc/include/asm/processor.h > @@ -47,6 +47,8 @@ > > #ifndef __ASSEMBLY__ > > +struct rlimit; > +unsigned long mmap_upper_limit(struct rlimit *rlim_stack); > unsigned long calc_max_stack_size(unsigned long stack_max); > > /* > diff --git a/arch/parisc/kernel/sys_parisc.c > b/arch/parisc/kernel/sys_parisc.c > index ab896eff7a1d..98af719d5f85 100644 > --- a/arch/parisc/kernel/sys_parisc.c > +++ b/arch/parisc/kernel/sys_parisc.c > @@ -77,7 +77,7 @@ unsigned long calc_max_stack_size(unsigned long stack_max) > * indicating that "current" should be used instead of a passed-in > * value from the exec bprm as done with arch_pick_mmap_layout(). > */ > -static unsigned long mmap_upper_limit(struct rlimit *rlim_stack) > +unsigned long mmap_upper_limit(struct rlimit *rlim_stack) > { > unsigned long stack_base; > > diff --git a/mm/util.c b/mm/util.c > index 8cbbfd3a3d59..0b7e715a71f2 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -414,6 +414,15 @@ static int mmap_is_legacy(struct rlimit *rlim_stack) > > static unsigned long mmap_base(unsigned long rnd, struct rlimit > *rlim_stack) > { > +#ifdef CONFIG_STACK_GROWSUP > + /* > + * For an upwards growing stack the calculation is much simpler. > + * Memory for the maximum stack size is reserved at the top of the > + * task. mmap_base starts directly below the stack and grows > + * downwards. > + */ > + return PAGE_ALIGN(mmap_upper_limit(rlim_stack) - rnd); > +#else > unsigned long gap = rlim_stack->rlim_cur; > unsigned long pad = stack_guard_gap; > > @@ -431,6 +440,7 @@ static unsigned long mmap_base(unsigned long rnd, struct > rlimit *rlim_stack) > gap = MAX_GAP; > > return PAGE_ALIGN(STACK_TOP - gap - rnd); > +#endif > } > > void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
Works here! Thanks Helge!!
| |