2023-06-29 18:51:40

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 6.4 00/28] 6.4.1-rc1 review

This is the start of the stable review cycle for the 6.4.1 release.
There are 28 patches in this series, all will be posted as a response
to this one. If anyone has any issues with these being applied, please
let me know.

Responses should be made by Sat, 01 Jul 2023 18:41:39 +0000.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.4.1-rc1.gz
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.4.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <[email protected]>
Linux 6.4.1-rc1

Ricardo Cañuelo <[email protected]>
Revert "thermal/drivers/mediatek: Use devm_of_iomap to avoid resource leak in mtk_thermal_probe"

Mike Hommey <[email protected]>
HID: logitech-hidpp: add HIDPP_QUIRK_DELAYED_INIT for the T651.

Ludvig Michaelsson <[email protected]>
HID: hidraw: fix data race on device refcount

Zhang Shurong <[email protected]>
fbdev: fix potential OOB read in fast_imageblit()

Hugh Dickins <[email protected]>
mm/khugepaged: fix regression in collapse_file()

Linus Torvalds <[email protected]>
gup: add warning if some caller would seem to want stack expansion

Jason Gerecke <[email protected]>
HID: wacom: Use ktime_t rather than int when dealing with timestamps

Linus Torvalds <[email protected]>
mm: always expand the stack with the mmap write lock held

Linus Torvalds <[email protected]>
execve: expand new process stack manually ahead of time

Liam R. Howlett <[email protected]>
mm: make find_extend_vma() fail if write lock not held

Linus Torvalds <[email protected]>
powerpc/mm: convert coprocessor fault to lock_mm_and_find_vma()

Linus Torvalds <[email protected]>
mm/fault: convert remaining simple cases to lock_mm_and_find_vma()

Ben Hutchings <[email protected]>
arm/mm: Convert to using lock_mm_and_find_vma()

Ben Hutchings <[email protected]>
riscv/mm: Convert to using lock_mm_and_find_vma()

Ben Hutchings <[email protected]>
mips/mm: Convert to using lock_mm_and_find_vma()

Michael Ellerman <[email protected]>
powerpc/mm: Convert to using lock_mm_and_find_vma()

Linus Torvalds <[email protected]>
arm64/mm: Convert to using lock_mm_and_find_vma()

Linus Torvalds <[email protected]>
mm: make the page fault mmap locking killable

Linus Torvalds <[email protected]>
mm: introduce new 'lock_mm_and_find_vma()' page fault helper

Peng Zhang <[email protected]>
maple_tree: fix potential out-of-bounds access in mas_wr_end_piv()

Oliver Hartkopp <[email protected]>
can: isotp: isotp_sendmsg(): fix return error fix on TX path

Wyes Karny <[email protected]>
cpufreq: amd-pstate: Make amd-pstate EPP driver name hyphenated

Thomas Gleixner <[email protected]>
x86/smp: Cure kexec() vs. mwait_play_dead() breakage

Thomas Gleixner <[email protected]>
x86/smp: Use dedicated cache-line for mwait_play_dead()

Thomas Gleixner <[email protected]>
x86/smp: Remove pointless wmb()s from native_stop_other_cpus()

Tony Battersby <[email protected]>
x86/smp: Dont access non-existing CPUID leaf

Thomas Gleixner <[email protected]>
x86/smp: Make stop_other_cpus() more robust

Borislav Petkov (AMD) <[email protected]>
x86/microcode/AMD: Load late on both threads too


-------------

Diffstat:

Makefile | 4 +-
arch/alpha/Kconfig | 1 +
arch/alpha/mm/fault.c | 13 +--
arch/arc/Kconfig | 1 +
arch/arc/mm/fault.c | 11 +--
arch/arm/Kconfig | 1 +
arch/arm/mm/fault.c | 63 ++++-----------
arch/arm64/Kconfig | 1 +
arch/arm64/mm/fault.c | 47 ++---------
arch/csky/Kconfig | 1 +
arch/csky/mm/fault.c | 22 ++----
arch/hexagon/Kconfig | 1 +
arch/hexagon/mm/vm_fault.c | 18 +----
arch/ia64/mm/fault.c | 36 ++-------
arch/loongarch/Kconfig | 1 +
arch/loongarch/mm/fault.c | 16 ++--
arch/m68k/mm/fault.c | 9 ++-
arch/microblaze/mm/fault.c | 5 +-
arch/mips/Kconfig | 1 +
arch/mips/mm/fault.c | 12 +--
arch/nios2/Kconfig | 1 +
arch/nios2/mm/fault.c | 17 +---
arch/openrisc/mm/fault.c | 5 +-
arch/parisc/mm/fault.c | 23 +++---
arch/powerpc/Kconfig | 1 +
arch/powerpc/mm/copro_fault.c | 14 +---
arch/powerpc/mm/fault.c | 39 +--------
arch/riscv/Kconfig | 1 +
arch/riscv/mm/fault.c | 31 +++-----
arch/s390/mm/fault.c | 5 +-
arch/sh/Kconfig | 1 +
arch/sh/mm/fault.c | 17 +---
arch/sparc/Kconfig | 1 +
arch/sparc/mm/fault_32.c | 32 ++------
arch/sparc/mm/fault_64.c | 8 +-
arch/um/kernel/trap.c | 11 +--
arch/x86/Kconfig | 1 +
arch/x86/include/asm/cpu.h | 2 +
arch/x86/include/asm/smp.h | 2 +
arch/x86/kernel/cpu/microcode/amd.c | 2 +-
arch/x86/kernel/process.c | 28 ++++++-
arch/x86/kernel/smp.c | 73 ++++++++++-------
arch/x86/kernel/smpboot.c | 81 ++++++++++++++++---
arch/x86/mm/fault.c | 52 +-----------
arch/xtensa/Kconfig | 1 +
arch/xtensa/mm/fault.c | 14 +---
drivers/cpufreq/amd-pstate.c | 2 +-
drivers/hid/hid-logitech-hidpp.c | 2 +-
drivers/hid/hidraw.c | 9 ++-
drivers/hid/wacom_wac.c | 6 +-
drivers/hid/wacom_wac.h | 2 +-
drivers/iommu/amd/iommu_v2.c | 4 +-
drivers/iommu/iommu-sva.c | 2 +-
drivers/thermal/mediatek/auxadc_thermal.c | 14 +---
drivers/video/fbdev/core/sysimgblt.c | 2 +-
fs/binfmt_elf.c | 6 +-
fs/exec.c | 38 +++++----
include/linux/mm.h | 16 ++--
lib/maple_tree.c | 11 +--
mm/Kconfig | 4 +
mm/gup.c | 14 +++-
mm/khugepaged.c | 7 +-
mm/memory.c | 127 ++++++++++++++++++++++++++++++
mm/mmap.c | 121 ++++++++++++++++++++++++----
mm/nommu.c | 17 ++--
net/can/isotp.c | 5 +-
66 files changed, 605 insertions(+), 531 deletions(-)




2023-06-29 20:39:22

by Ronald Warsow

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

Hi Greg

6.4.1-rc1

compiles, boots and runs here on x86_64
(Intel Rocket Lake, i5-11400)

Thanks

Tested-by: Ronald Warsow <[email protected]>


2023-06-30 05:41:44

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, 30 Jun 2023 at 00:18, Greg Kroah-Hartman
<[email protected]> wrote:
>
> This is the start of the stable review cycle for the 6.4.1 release.
> There are 28 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat, 01 Jul 2023 18:41:39 +0000.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.4.1-rc1.gz
> or in the git tree and branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.4.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h

Results from Linaro’s test farm.

Following build regression noticed on Linux stable-rc 6.4 and also noticed on
Linux mainline master.

Regressions found on Parisc and Sparc build failed:
- build/gcc-11-defconfig

Reported-by: Linux Kernel Functional Testing <[email protected]>

Parisc Build log:
=============
arch/parisc/mm/fault.c: In function 'do_page_fault':
arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in
this function)
292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
| ^~~~
arch/parisc/mm/fault.c:292:22: note: each undeclared identifier is
reported only once for each function it appears in


sparc Build log:
===========
<stdin>:1519:2: warning: #warning syscall clone3 not implemented [-Wcpp]
arch/sparc/mm/fault_32.c: In function 'force_user_fault':
arch/sparc/mm/fault_32.c:315:49: error: 'regs' undeclared (first use
in this function)
315 | vma = lock_mm_and_find_vma(mm, address, regs);
| ^~~~
arch/sparc/mm/fault_32.c:315:49: note: each undeclared identifier is
reported only once for each function it appears in


Links:
- https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959811/suite/build/test/gcc-11-defconfig/details/
- https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959811/suite/build/test/gcc-11-defconfig/log

- https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959890/suite/build/test/gcc-11-defconfig/details/
- https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959890/suite/build/test/gcc-11-defconfig/log


Both build failures noticed on mainline and sparc build have been
fixed yesterday.
- https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.4-8542-g82a2a5105589/testrun/17963192/suite/build/test/gcc-11-defconfig/history/


Following patch that got fixed
---
From 0b26eadbf200abf6c97c6d870286c73219cdac65 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <[email protected]>
Date: Thu, 29 Jun 2023 20:41:24 -0700
Subject: sparc32: fix lock_mm_and_find_vma() conversion

The sparc32 conversion to lock_mm_and_find_vma() in commit a050ba1e7422
("mm/fault: convert remaining simple cases to lock_mm_and_find_vma()")
missed the fact that we didn't actually have a 'regs' pointer available
in the 'force_user_fault()' case.

It's there in the regular page fault path ("do_sparc_fault()"), but not
the window underflow/overflow paths.

Which is all fine - we can just pass in a NULL pointer. The register
state is only used to avoid deadlock with kernel faults, which is not
the case for any of these register window faults.

Reported-by: Stephen Rothwell <[email protected]>
Fixes: a050ba1e7422 ("mm/fault: convert remaining simple cases to
lock_mm_and_find_vma()")
Signed-off-by: Linus Torvalds <[email protected]>

--
Linaro LKFT
https://lkft.linaro.org

2023-06-30 06:28:37

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
>
> arch/parisc/mm/fault.c: In function 'do_page_fault':
> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
> 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))

Bah. "prev" should be "prev_vma" here.

I've pushed out the fix. Greg, apologies. It's

ea3f8272876f parisc: fix expand_stack() conversion

and Naresh already pointed to the similarly silly sparc32 fix.

Linus

2023-06-30 06:28:46

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, Jun 30, 2023 at 11:00:51AM +0530, Naresh Kamboju wrote:
> On Fri, 30 Jun 2023 at 00:18, Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > This is the start of the stable review cycle for the 6.4.1 release.
> > There are 28 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Sat, 01 Jul 2023 18:41:39 +0000.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.4.1-rc1.gz
> > or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.4.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
>
> Results from Linaro’s test farm.
>
> Following build regression noticed on Linux stable-rc 6.4 and also noticed on
> Linux mainline master.
>
> Regressions found on Parisc and Sparc build failed:
> - build/gcc-11-defconfig
>
> Reported-by: Linux Kernel Functional Testing <[email protected]>
>
> Parisc Build log:
> =============
> arch/parisc/mm/fault.c: In function 'do_page_fault':
> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in
> this function)
> 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
> | ^~~~
> arch/parisc/mm/fault.c:292:22: note: each undeclared identifier is
> reported only once for each function it appears in
>
>
> sparc Build log:
> ===========
> <stdin>:1519:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> arch/sparc/mm/fault_32.c: In function 'force_user_fault':
> arch/sparc/mm/fault_32.c:315:49: error: 'regs' undeclared (first use
> in this function)
> 315 | vma = lock_mm_and_find_vma(mm, address, regs);
> | ^~~~
> arch/sparc/mm/fault_32.c:315:49: note: each undeclared identifier is
> reported only once for each function it appears in
>
>
> Links:
> - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959811/suite/build/test/gcc-11-defconfig/details/
> - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959811/suite/build/test/gcc-11-defconfig/log
>
> - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959890/suite/build/test/gcc-11-defconfig/details/
> - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.4.y/build/v6.4-29-g8e5ddb853f08/testrun/17959890/suite/build/test/gcc-11-defconfig/log
>
>
> Both build failures noticed on mainline and sparc build have been
> fixed yesterday.
> - https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.4-8542-g82a2a5105589/testrun/17963192/suite/build/test/gcc-11-defconfig/history/
>
>
> Following patch that got fixed
> ---
> >From 0b26eadbf200abf6c97c6d870286c73219cdac65 Mon Sep 17 00:00:00 2001
> From: Linus Torvalds <[email protected]>
> Date: Thu, 29 Jun 2023 20:41:24 -0700
> Subject: sparc32: fix lock_mm_and_find_vma() conversion
>
> The sparc32 conversion to lock_mm_and_find_vma() in commit a050ba1e7422
> ("mm/fault: convert remaining simple cases to lock_mm_and_find_vma()")
> missed the fact that we didn't actually have a 'regs' pointer available
> in the 'force_user_fault()' case.
>
> It's there in the regular page fault path ("do_sparc_fault()"), but not
> the window underflow/overflow paths.
>
> Which is all fine - we can just pass in a NULL pointer. The register
> state is only used to avoid deadlock with kernel faults, which is not
> the case for any of these register window faults.
>
> Reported-by: Stephen Rothwell <[email protected]>
> Fixes: a050ba1e7422 ("mm/fault: convert remaining simple cases to
> lock_mm_and_find_vma()")
> Signed-off-by: Linus Torvalds <[email protected]>

Thanks! That saves me having to dig. I'll go push out updates with
this in them...

greg k-h

2023-06-30 06:30:08

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, Jun 30, 2023 at 11:00:51AM +0530, Naresh Kamboju wrote:
> On Fri, 30 Jun 2023 at 00:18, Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > This is the start of the stable review cycle for the 6.4.1 release.
> > There are 28 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Sat, 01 Jul 2023 18:41:39 +0000.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.4.1-rc1.gz
> > or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.4.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
>
> Results from Linaro’s test farm.
>
> Following build regression noticed on Linux stable-rc 6.4 and also noticed on
> Linux mainline master.
>
> Regressions found on Parisc and Sparc build failed:
> - build/gcc-11-defconfig
>
> Reported-by: Linux Kernel Functional Testing <[email protected]>
>
> Parisc Build log:
> =============
> arch/parisc/mm/fault.c: In function 'do_page_fault':
> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in
> this function)
> 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
> | ^~~~
> arch/parisc/mm/fault.c:292:22: note: each undeclared identifier is
> reported only once for each function it appears in

This is now fixed in Linus's tree with ea3f8272876f ("parisc: fix
expand_stack() conversion"), so I'll queue it up and push out
yet-another-rc...

thanks,

greg k-h

2023-06-30 06:32:49

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Thu, Jun 29, 2023 at 11:16:21PM -0700, Linus Torvalds wrote:
> On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
> >
> > arch/parisc/mm/fault.c: In function 'do_page_fault':
> > arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
> > 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
>
> Bah. "prev" should be "prev_vma" here.
>
> I've pushed out the fix. Greg, apologies. It's
>
> ea3f8272876f parisc: fix expand_stack() conversion
>
> and Naresh already pointed to the similarly silly sparc32 fix.

Ah, I saw it hit your repo before your email here, sorry about that.
Now picked up.

greg k-h

2023-06-30 06:47:55

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On 6/29/23 23:29, Guenter Roeck wrote:
> On 6/29/23 23:16, Linus Torvalds wrote:
>> On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
>>>
>>> arch/parisc/mm/fault.c: In function 'do_page_fault':
>>> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
>>>    292 |                 if (!prev || !(prev->vm_flags & VM_GROWSUP))
>>
>> Bah. "prev" should be "prev_vma" here.
>>
>> I've pushed out the fix. Greg, apologies. It's
>>
>>     ea3f8272876f parisc: fix expand_stack() conversion
>>
>> and Naresh already pointed to the similarly silly sparc32 fix.
>>
>>               Linus
>
> Did you see that one (in mainline) ?
>
> Building csky:defconfig ... failed
> --------------
> Error log:
> arch/csky/mm/fault.c: In function 'do_page_fault':
> arch/csky/mm/fault.c:240:40: error: 'address' undeclared (first use in this function); did you mean 'addr'?
>   240 |         vma = lock_mm_and_find_vma(mm, address, regs);
>

This is also in {6.1,6.3,6.4}-rc unless I am missing something.

Guenter


2023-06-30 06:49:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Thu, 29 Jun 2023 at 23:29, Guenter Roeck <[email protected]> wrote:
>
> Did you see that one (in mainline) ?
>
> Building csky:defconfig ... failed

Nope. Thanks. Obvious fix: 'address' is called 'addr' here.

I knew we had all these tiny little mazes that looked the same but
were just _subtly_ different, but I still ended up doing too much
cut-and-paste.

And I only ended up cross-compiling the fairly small set that I had
existing cross-build environments for. Which was less than half our
~24 different architectures.

Oh well. We'll get them all. Eventually. Let me go fix up that csky case.

Linus

2023-06-30 06:57:37

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On 6/29/23 23:16, Linus Torvalds wrote:
> On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
>>
>> arch/parisc/mm/fault.c: In function 'do_page_fault':
>> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
>> 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
>
> Bah. "prev" should be "prev_vma" here.
>
> I've pushed out the fix. Greg, apologies. It's
>
> ea3f8272876f parisc: fix expand_stack() conversion
>
> and Naresh already pointed to the similarly silly sparc32 fix.
>
> Linus

Did you see that one (in mainline) ?

Building csky:defconfig ... failed
--------------
Error log:
arch/csky/mm/fault.c: In function 'do_page_fault':
arch/csky/mm/fault.c:240:40: error: 'address' undeclared (first use in this function); did you mean 'addr'?
240 | vma = lock_mm_and_find_vma(mm, address, regs);

Guenter


2023-06-30 07:07:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Thu, Jun 29, 2023 at 11:33:45PM -0700, Linus Torvalds wrote:
> On Thu, 29 Jun 2023 at 23:29, Guenter Roeck <[email protected]> wrote:
> >
> > Did you see that one (in mainline) ?
> >
> > Building csky:defconfig ... failed
>
> Nope. Thanks. Obvious fix: 'address' is called 'addr' here.
>
> I knew we had all these tiny little mazes that looked the same but
> were just _subtly_ different, but I still ended up doing too much
> cut-and-paste.
>
> And I only ended up cross-compiling the fairly small set that I had
> existing cross-build environments for. Which was less than half our
> ~24 different architectures.
>
> Oh well. We'll get them all. Eventually. Let me go fix up that csky case.

Thanks, I've picked that up now as well.

greg k-h

2023-06-30 07:33:59

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Thu, 29 Jun 2023 at 23:33, Linus Torvalds
<[email protected]> wrote:
>
> Oh well. We'll get them all. Eventually. Let me go fix up that csky case.

It's commit e55e5df193d2 ("csky: fix up lock_mm_and_find_vma() conversion").

Let's hope all the problems are these kinds of silly - but obvious -
naming differences between different architectures.

Because as long as they cause build errors, they may be embarrassing,
but easy to find and notice.

I may not have cared enough about some of these architectures, and it
shows. sparc32. parisc. csky...

Linus

2023-06-30 07:34:16

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On 6/30/23 08:29, Greg Kroah-Hartman wrote:
> On Thu, Jun 29, 2023 at 11:16:21PM -0700, Linus Torvalds wrote:
>> On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
>>>
>>> arch/parisc/mm/fault.c: In function 'do_page_fault':
>>> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
>>> 292 | if (!prev || !(prev->vm_flags & VM_GROWSUP))
>>
>> Bah. "prev" should be "prev_vma" here.
>>
>> I've pushed out the fix. Greg, apologies. It's
>>
>> ea3f8272876f parisc: fix expand_stack() conversion
>>
>> and Naresh already pointed to the similarly silly sparc32 fix.
>
> Ah, I saw it hit your repo before your email here, sorry about that.
> Now picked up.

I've just cherry-picked ea3f8272876f on top of -rc2, built and run-tested it,
and everything is OK on parisc.

Thanks!
Helge

2023-06-30 23:18:25

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On 6/29/23 23:47, Linus Torvalds wrote:
> On Thu, 29 Jun 2023 at 23:33, Linus Torvalds
> <[email protected]> wrote:
>>
>> Oh well. We'll get them all. Eventually. Let me go fix up that csky case.
>
> It's commit e55e5df193d2 ("csky: fix up lock_mm_and_find_vma() conversion").
>
> Let's hope all the problems are these kinds of silly - but obvious -
> naming differences between different architectures.
>
> Because as long as they cause build errors, they may be embarrassing,
> but easy to find and notice.
>
> I may not have cared enough about some of these architectures, and it
> shows. sparc32. parisc. csky...
>

There is one more, unfortunately.

Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... failed
------------
Error log:
arch/xtensa/mm/fault.c: In function ‘do_page_fault’:
arch/xtensa/mm/fault.c:133:8: error: implicit declaration of function ‘lock_mm_and_find_vma’

This affects all stable release candidates as well as mainline.
mmu builds are fine, and indeed lock_mm_and_find_vma() is only declared
for CONFIG_MMU. I don't know if this needs a dummy or some other fix
for the nommu case.

Guenter


2023-07-01 01:38:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, 30 Jun 2023 at 15:51, Guenter Roeck <[email protected]> wrote:
>
> There is one more, unfortunately.
>
> Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... failed

Heh. I didn't even realize that anybody would ever do
lock_mm_and_find_vma() code on a nommu platform.

With nommu, handle_mm_fault() will just BUG(), so it's kind of
pointless to do any of this at all, and I didn't expect anybody to
have this page faulting path that just causes that BUG() for any
faults.

But it turns out xtensa has a notion of protection faults even for
NOMMU configs:

config PFAULT
bool "Handle protection faults" if EXPERT && !MMU
default y
help
Handle protection faults. MMU configurations must enable it.
noMMU configurations may disable it if used memory map never
generates protection faults or faults are always fatal.

If unsure, say Y.

which is why it violated my expectations so badly.

I'm not sure if that protection fault handling really ever gets quite
this far (it certainly should *not* make it to the BUG() in
handle_mm_fault()), but I think the attached patch is likely the right
thing to do.

Can you check if it fixes that xtensa case? It looks
ObviouslyCorrect(tm) to me, but considering that I clearly missed this
case existing AT ALL, it might be best to double-check.

Linus


Attachments:
patch.diff (1.83 kB)

2023-07-01 03:31:15

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, Jun 30, 2023 at 06:24:49PM -0700, Linus Torvalds wrote:
> On Fri, 30 Jun 2023 at 15:51, Guenter Roeck <[email protected]> wrote:
> >
> > There is one more, unfortunately.
> >
> > Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... failed
>
> Heh. I didn't even realize that anybody would ever do
> lock_mm_and_find_vma() code on a nommu platform.
>
> With nommu, handle_mm_fault() will just BUG(), so it's kind of
> pointless to do any of this at all, and I didn't expect anybody to
> have this page faulting path that just causes that BUG() for any
> faults.
>
> But it turns out xtensa has a notion of protection faults even for
> NOMMU configs:
>
> config PFAULT
> bool "Handle protection faults" if EXPERT && !MMU
> default y
> help
> Handle protection faults. MMU configurations must enable it.
> noMMU configurations may disable it if used memory map never
> generates protection faults or faults are always fatal.
>
> If unsure, say Y.
>
> which is why it violated my expectations so badly.
>
> I'm not sure if that protection fault handling really ever gets quite
> this far (it certainly should *not* make it to the BUG() in
> handle_mm_fault()), but I think the attached patch is likely the right
> thing to do.
>
> Can you check if it fixes that xtensa case? It looks
> ObviouslyCorrect(tm) to me, but considering that I clearly missed this
> case existing AT ALL, it might be best to double-check.
>
> Linus

Yes, the patch below fixes the problem.

Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... running ......... passed

Thanks,
Guenter

> include/linux/mm.h | 5 +++--
> mm/nommu.c | 11 +++++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 39aa409e84d5..4f2c33c273eb 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2323,6 +2323,9 @@ void pagecache_isize_extended(struct inode *inode, loff_t from, loff_t to);
> void truncate_pagecache_range(struct inode *inode, loff_t offset, loff_t end);
> int generic_error_remove_page(struct address_space *mapping, struct page *page);
>
> +struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> + unsigned long address, struct pt_regs *regs);
> +
> #ifdef CONFIG_MMU
> extern vm_fault_t handle_mm_fault(struct vm_area_struct *vma,
> unsigned long address, unsigned int flags,
> @@ -2334,8 +2337,6 @@ void unmap_mapping_pages(struct address_space *mapping,
> pgoff_t start, pgoff_t nr, bool even_cows);
> void unmap_mapping_range(struct address_space *mapping,
> loff_t const holebegin, loff_t const holelen, int even_cows);
> -struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> - unsigned long address, struct pt_regs *regs);
> #else
> static inline vm_fault_t handle_mm_fault(struct vm_area_struct *vma,
> unsigned long address, unsigned int flags,
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 37d0b03143f1..fdc392735ec6 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -630,6 +630,17 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> }
> EXPORT_SYMBOL(find_vma);
>
> +/*
> + * At least xtensa ends up having protection faults even with no
> + * MMU.. No stack expansion, at least.
> + */
> +struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> + unsigned long addr, struct pt_regs *regs)
> +{
> + mmap_read_lock(mm);
> + return vma_lookup(mm, addr);
> +}
> +
> /*
> * expand a stack to a given address
> * - not supported under NOMMU conditions


2023-07-01 05:05:50

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, 30 Jun 2023 at 19:50, Guenter Roeck <[email protected]> wrote:
>
> Yes, the patch below fixes the problem.
>
> Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... running ......... passed

Thanks. Committed as

d85a143b69ab ("xtensa: fix NOMMU build with lock_mm_and_find_vma()
conversion")

and pushed out.

Linus

2023-07-01 10:26:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Fri, Jun 30, 2023 at 09:22:45PM -0700, Linus Torvalds wrote:
> On Fri, 30 Jun 2023 at 19:50, Guenter Roeck <[email protected]> wrote:
> >
> > Yes, the patch below fixes the problem.
> >
> > Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... running ......... passed
>
> Thanks. Committed as
>
> d85a143b69ab ("xtensa: fix NOMMU build with lock_mm_and_find_vma()
> conversion")
>
> and pushed out.

Thanks, now queued up.

greg k-h

2023-07-01 11:01:44

by Max Filippov

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

Hi Linus,

On Fri, Jun 30, 2023 at 9:23 PM Linus Torvalds
<[email protected]> wrote:
>
> On Fri, 30 Jun 2023 at 19:50, Guenter Roeck <[email protected]> wrote:
> >
> > Yes, the patch below fixes the problem.
> >
> > Building xtensa:de212:kc705-nommu:nommu_kc705_defconfig ... running ......... passed
>
> Thanks. Committed as
>
> d85a143b69ab ("xtensa: fix NOMMU build with lock_mm_and_find_vma()
> conversion")
>
> and pushed out.

Thanks for the build fix. Unfortunately despite being obviously correct
it doesn't release the mm lock in case VMA is not found, so it results
in a runtime hang. I've posted a fix for that.

--
Thanks.
-- Max

2023-07-01 15:34:59

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review

On Sat, 1 Jul 2023 at 03:32, Max Filippov <[email protected]> wrote:
>
> Thanks for the build fix. Unfortunately despite being obviously correct
> it doesn't release the mm lock in case VMA is not found, so it results
> in a runtime hang. I've posted a fix for that.

Heh. I woke up this morning to that feeling of "Duh!" about this, and
find you already had fixed it. Patch applied.

Linus

2023-07-02 21:54:58

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

Hi Linus,

On 6/30/23 08:56, Helge Deller wrote:
> On 6/30/23 08:29, Greg Kroah-Hartman wrote:
>> On Thu, Jun 29, 2023 at 11:16:21PM -0700, Linus Torvalds wrote:
>>> On Thu, 29 Jun 2023 at 22:31, Naresh Kamboju <[email protected]> wrote:
>>>>
>>>> arch/parisc/mm/fault.c: In function 'do_page_fault':
>>>> arch/parisc/mm/fault.c:292:22: error: 'prev' undeclared (first use in this function)
>>>>    292 |                 if (!prev || !(prev->vm_flags & VM_GROWSUP))
>>>
>>> Bah. "prev" should be "prev_vma" here.
>>>
>>> I've pushed out the fix. Greg, apologies. It's
>>>
>>>     ea3f8272876f parisc: fix expand_stack() conversion
>>>
>>> and Naresh already pointed to the similarly silly sparc32 fix.
>>
>> Ah, I saw it hit your repo before your email here, sorry about that.
>> Now picked up.
>
> I've just cherry-picked ea3f8272876f on top of -rc2, built and run-tested it,
> and everything is OK on parisc.

Actually, your changes seems to trigger...:

root@debian:~# /usr/bin/ls /usr/bin/*
-bash: /usr/bin/ls: Argument list too long

or with a long gcc argument list:
gcc: fatal error: cannot execute '/usr/lib/gcc/hppa-linux-gnu/12/cc1': execv: Argument list too long

I'm trying to understand what's missing, but maybe you have some idea?

Helge

2023-07-02 23:34:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Sun, 2 Jul 2023 at 14:33, Helge Deller <[email protected]> wrote:
>
> Actually, your changes seems to trigger...:
>
> root@debian:~# /usr/bin/ls /usr/bin/*
> -bash: /usr/bin/ls: Argument list too long

So this only happens with _fairly_ long argument lists, right? Maybe
your config has a 64kB page size, and normal programs never expand
beyond a single page?

I bet it is because of f313c51d26aa ("execve: expand new process stack
manually ahead of time"), but I don't see exactly why.

But pa-risc is the only architecture with CONFIG_STACK_GROWSUP, and
while I really thought that commit should do the exact same thing as
the old

#ifdef CONFIG_STACK_GROWSUP

special case, I must clearly have been wrong.

Would you mind just verifying that yes, that commit on mainline is
broken for you, and the previous one works?

Linus

2023-07-03 00:38:14

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Sun, 2 Jul 2023 at 15:45, Linus Torvalds
<[email protected]> wrote:
>
> Would you mind just verifying that yes, that commit on mainline is
> broken for you, and the previous one works?

Also, while I looked at it again, and still didn't understand why
parisc would be different here, I *did* realize that because parisc
has a stack that grows up, the debug warning I added for GUP won't
trigger.

So if I got that execve() logic wrong for STACK_GROWSUP (which I
clearly must have), then exactly because it's grows-up, a GUP failure
wouldn't warn about not expanding the stack.

IOW, would you mind applying something like this on top of the current
kernel, and let me know if it warns?

.. and here I thought ia64 would be the pain-point. Silly me.

Linus


Attachments:
patch.diff (773.00 B)

2023-07-03 03:35:19

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/2/23 16:30, Linus Torvalds wrote:
> On Sun, 2 Jul 2023 at 15:45, Linus Torvalds
> <[email protected]> wrote:
>>
>> Would you mind just verifying that yes, that commit on mainline is
>> broken for you, and the previous one works?
>
> Also, while I looked at it again, and still didn't understand why
> parisc would be different here, I *did* realize that because parisc
> has a stack that grows up, the debug warning I added for GUP won't
> trigger.
>
> So if I got that execve() logic wrong for STACK_GROWSUP (which I
> clearly must have), then exactly because it's grows-up, a GUP failure
> wouldn't warn about not expanding the stack.
>
> IOW, would you mind applying something like this on top of the current
> kernel, and let me know if it warns?
>

I can reproduce the problem in qemu. However, I do not see a warning
after applying your patch.

Guenter


2023-07-03 04:51:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Sun, 2 Jul 2023 at 20:23, Guenter Roeck <[email protected]> wrote:
>
> I can reproduce the problem in qemu. However, I do not see a warning
> after applying your patch.

Funky, funky.

I'm assuming it's the

page = get_arg_page(bprm, pos, 1);
if (!page) {
ret = -E2BIG;
goto out;
}

in copy_strings() that causes this. Or possibly, the version in
copy_string_kernel().

Does *this* get that "pr_warn()" printout (and a stack trace once,
just for good measure)?

Linus


Attachments:
patch.diff (773.00 B)

2023-07-03 05:19:19

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/2/23 21:22, Linus Torvalds wrote:
> On Sun, 2 Jul 2023 at 20:23, Guenter Roeck <[email protected]> wrote:
>>
>> I can reproduce the problem in qemu. However, I do not see a warning
>> after applying your patch.
>
> Funky, funky.
>
> I'm assuming it's the
>
> page = get_arg_page(bprm, pos, 1);
> if (!page) {
> ret = -E2BIG;
> goto out;
> }
>
> in copy_strings() that causes this. Or possibly, the version in
> copy_string_kernel().
>
> Does *this* get that "pr_warn()" printout (and a stack trace once,
> just for good measure)?
>

Sorry, you lost me. Isn't that the same patch as before ? Or
is it just time for me to go to bed ?

Guenter


2023-07-03 05:24:04

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Sun, 2 Jul 2023 at 21:46, Guenter Roeck <[email protected]> wrote:
>
> Sorry, you lost me. Isn't that the same patch as before ? Or
> is it just time for me to go to bed ?

No, I think it's time for *me* to go to bed.

Let's get the right diff this time.

Linus


Attachments:
patch.diff (932.00 B)

2023-07-03 05:54:21

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/2/23 21:49, Linus Torvalds wrote:
> On Sun, 2 Jul 2023 at 21:46, Guenter Roeck <[email protected]> wrote:
>>
>> Sorry, you lost me. Isn't that the same patch as before ? Or
>> is it just time for me to go to bed ?
>
> No, I think it's time for *me* to go to bed.
>
> Let's get the right diff this time.
>

Here you are:

[ 31.188688] stack expand failed: ffeff000-fff00000 (ffefeff2)
[ 31.189131] ------------[ cut here ]------------
[ 31.189259] WARNING: CPU: 0 PID: 472 at fs/exec.c:217 get_arg_page+0x1e8/0x1f4
[ 31.189827] Modules linked in:
[ 31.190083] CPU: 0 PID: 472 Comm: sh Tainted: G N 6.4.0-32bit+ #1
[ 31.190213] Hardware name: 9000/778/B160L
[ 31.190347]
[ 31.190407] YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI
[ 31.190496] PSW: 00000000000001001011111100001111 Tainted: G N
[ 31.190625] r00-03 0004bf0f 11026240 1034a3ec 12bb41c0
[ 31.190741] r04-07 127ec400 00000001 12b725a4 12b72530
[ 31.190821] r08-11 129e6708 ffefeff2 2ff9d000 ffeff000
[ 31.190895] r12-15 127ec400 10e463f0 10e34348 12a4d1a0
[ 31.190962] r16-19 00000002 00001000 ffefe000 12a4d1a0
[ 31.191033] r20-23 0000000f 00001a46 013ae000 12bb4498
[ 31.191103] r24-27 11542330 00000000 115430a0 10ed98d8
[ 31.191173] r28-31 00000031 00000310 12bb4240 0000000f
[ 31.191251] sr00-03 00000000 00000000 00000000 000000a0
[ 31.191332] sr04-07 00000000 00000000 00000000 00000000
[ 31.191407]
[ 31.191443] IASQ: 00000000 00000000 IAOQ: 1034a3ec 1034a3f0
[ 31.191522] IIR: 03ffe01f ISR: 00000000 IOR: 1065d424
[ 31.191593] CPU: 0 CR30: 12a4d1a0 CR31: 00000000
[ 31.192675] ORIG_R28: 12a4d1a0
[ 31.192770] IAOQ[0]: get_arg_page+0x1e8/0x1f4
[ 31.192851] IAOQ[1]: get_arg_page+0x1ec/0x1f4
[ 31.192922] RP(r2): get_arg_page+0x1e8/0x1f4
[ 31.193007] Backtrace:
[ 31.193085] [<1034a9cc>] copy_strings+0x148/0x3d8
[ 31.193214] [<1034ad94>] do_execveat_common+0x138/0x21c
[ 31.193302] [<1034bcc4>] sys_execve+0x3c/0x54
[ 31.193400] [<101af1b4>] syscall_exit+0x0/0x10
[ 31.193562]
[ 31.193698] ---[ end trace 0000000000000000 ]---
[ 31.200551] stack expand failed: ffeff000-fff00000 (ffefefee)
/bin/sh: ls: Argument list too long

Guenter


2023-07-03 06:37:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Sun, 2 Jul 2023 at 22:33, Guenter Roeck <[email protected]> wrote:
>
> Here you are:
>
> [ 31.188688] stack expand failed: ffeff000-fff00000 (ffefeff2)

Ahhah!

I think the problem is actually ridiculously simple.

The thing is, the parisc stack expands upwards. That's obvious. I've
mentioned it several times in just this thread as being the thing that
makes parisc special.

But it's *so* obvious that I didn't even think about what it really implies.

And part of all the changes was this part in expand_downwards():

if (!(vma->vm_flags & VM_GROWSDOWN))
return -EFAULT;

and that will *always* fail on parisc, because - as said multiple
times - the parisc stack expands upwards. It doesn't have VM_GROWSDOWN
set.

What a dum-dum I am.

And I did it that way because the *normal* stack expansion obviously
wants it that way and putting the check there not only made sense, but
simplified other code.

But fs/execve.c is special - and only special for parisc - in that it
really wants to expand a normally upwards-growing stack downwards
unconditionally.

Anyway, I think that new check in expand_downwards() is the right
thing to do, and the real fix here is to simply make vm_flags reflect
reality.

Because during execve, that stack that will _eventually_ grow upwards,
does in fact grow downwards. Let's make it reflect that.

We already do magical extra setup for the stack flags during setup
(VM_STACK_INCOMPLETE_SETUP), so extending that logic to contain
VM_GROWSDOWN seems sane and the right thing to do.

IOW, I think a patch like the attached will fix the problem for real.

It needs a good commit log and maybe a code comment or two, but before
I bother to do that, let's verify that yes, it does actually fix
things.

In the meantime, I will actually go to bed, but I'm pretty sure this is it.

Linus


Attachments:
patch.diff (948.00 B)

2023-07-03 07:33:31

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

Hi Linus,

On 7/3/23 08:20, Linus Torvalds wrote:
> On Sun, 2 Jul 2023 at 22:33, Guenter Roeck <[email protected]> wrote:
>>
>> Here you are:
>>
>> [ 31.188688] stack expand failed: ffeff000-fff00000 (ffefeff2)
>
> Ahhah!
>
> I think the problem is actually ridiculously simple.
>
> The thing is, the parisc stack expands upwards. That's obvious. I've
> mentioned it several times in just this thread as being the thing that
> makes parisc special.
>
> But it's *so* obvious that I didn't even think about what it really implies.
>
> And part of all the changes was this part in expand_downwards():
>
> if (!(vma->vm_flags & VM_GROWSDOWN))
> return -EFAULT;
>
> and that will *always* fail on parisc, because - as said multiple
> times - the parisc stack expands upwards. It doesn't have VM_GROWSDOWN
> set.
>
> What a dum-dum I am.
>
> And I did it that way because the *normal* stack expansion obviously
> wants it that way and putting the check there not only made sense, but
> simplified other code.
>
> But fs/execve.c is special - and only special for parisc - in that it
> really wants to expand a normally upwards-growing stack downwards
> unconditionally.
>
> Anyway, I think that new check in expand_downwards() is the right
> thing to do, and the real fix here is to simply make vm_flags reflect
> reality.
>
> Because during execve, that stack that will _eventually_ grow upwards,
> does in fact grow downwards. Let's make it reflect that.
>
> We already do magical extra setup for the stack flags during setup
> (VM_STACK_INCOMPLETE_SETUP), so extending that logic to contain
> VM_GROWSDOWN seems sane and the right thing to do.
>
> IOW, I think a patch like the attached will fix the problem for real.
>
> It needs a good commit log and maybe a code comment or two, but before
> I bother to do that, let's verify that yes, it does actually fix
> things.
>
> In the meantime, I will actually go to bed, but I'm pretty sure this is it.

Great, that patch fixes it!

I wonder if you want to
#define VM_STACK_EARLY VM_GROWSDOWN
even for the case where the stack grows down too (instead of 0),
just to make clear that in both cases the stack goes downwards initially.

Helge

2023-07-03 13:24:08

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/2/23 23:20, Linus Torvalds wrote:
> On Sun, 2 Jul 2023 at 22:33, Guenter Roeck <[email protected]> wrote:
>>
>> Here you are:
>>
>> [ 31.188688] stack expand failed: ffeff000-fff00000 (ffefeff2)
>
> Ahhah!
>
> I think the problem is actually ridiculously simple.
>
> The thing is, the parisc stack expands upwards. That's obvious. I've
> mentioned it several times in just this thread as being the thing that
> makes parisc special.
>
> But it's *so* obvious that I didn't even think about what it really implies.
>
> And part of all the changes was this part in expand_downwards():
>
> if (!(vma->vm_flags & VM_GROWSDOWN))
> return -EFAULT;
>
> and that will *always* fail on parisc, because - as said multiple
> times - the parisc stack expands upwards. It doesn't have VM_GROWSDOWN
> set.
>
> What a dum-dum I am.
>
> And I did it that way because the *normal* stack expansion obviously
> wants it that way and putting the check there not only made sense, but
> simplified other code.
>
> But fs/execve.c is special - and only special for parisc - in that it
> really wants to expand a normally upwards-growing stack downwards
> unconditionally.
>
> Anyway, I think that new check in expand_downwards() is the right
> thing to do, and the real fix here is to simply make vm_flags reflect
> reality.
>
> Because during execve, that stack that will _eventually_ grow upwards,
> does in fact grow downwards. Let's make it reflect that.
>
> We already do magical extra setup for the stack flags during setup
> (VM_STACK_INCOMPLETE_SETUP), so extending that logic to contain
> VM_GROWSDOWN seems sane and the right thing to do.
>
> IOW, I think a patch like the attached will fix the problem for real.
>
> It needs a good commit log and maybe a code comment or two, but before
> I bother to do that, let's verify that yes, it does actually fix
> things.
>

Yes, it does. I'll run a complete qemu test with it applied to be sure
there is no impact on other architectures (yes, I know, that should not
be the case, but better safe than sorry). I'll even apply
https://lore.kernel.org/all/[email protected]/raw
to be able to test sh4.

Guenter


2023-07-03 13:54:37

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/3/23 05:59, Guenter Roeck wrote:
> On 7/2/23 23:20, Linus Torvalds wrote:
>> On Sun, 2 Jul 2023 at 22:33, Guenter Roeck <[email protected]> wrote:
>>>
>>> Here you are:
>>>
>>> [   31.188688] stack expand failed: ffeff000-fff00000 (ffefeff2)
>>
>> Ahhah!
>>
>> I think the problem is actually ridiculously simple.
>>
>> The thing is, the parisc stack expands upwards. That's obvious. I've
>> mentioned it several times in just this thread as being the thing that
>> makes parisc special.
>>
>> But it's *so* obvious that I didn't even think about what it really implies.
>>
>> And part of all the changes was this part in expand_downwards():
>>
>>          if (!(vma->vm_flags & VM_GROWSDOWN))
>>                  return -EFAULT;
>>
>> and that will *always* fail on parisc, because - as said multiple
>> times - the parisc stack expands upwards. It doesn't have VM_GROWSDOWN
>> set.
>>
>> What a dum-dum I am.
>>
>> And I did it that way because the *normal* stack expansion obviously
>> wants it that way and putting the check there not only made sense, but
>> simplified other code.
>>
>> But fs/execve.c is special - and only special for parisc - in that it
>> really wants to  expand a normally upwards-growing stack downwards
>> unconditionally.
>>
>> Anyway, I think that new check in expand_downwards() is the right
>> thing to do, and the real fix here is to simply make vm_flags reflect
>> reality.
>>
>> Because during execve, that stack that will _eventually_ grow upwards,
>> does in fact grow downwards.  Let's make it reflect that.
>>
>> We already do magical extra setup for the stack flags during setup
>> (VM_STACK_INCOMPLETE_SETUP), so extending that logic to contain
>> VM_GROWSDOWN seems sane and the right thing to do.
>>
>> IOW, I think a patch like the attached will fix the problem for real.
>>
>> It needs a good commit log and maybe a code comment or two, but before
>> I bother to do that, let's verify that yes, it does actually fix
>> things.
>>
>
> Yes, it does. I'll run a complete qemu test with it applied to be sure
> there is no impact on other architectures (yes, I know, that should not
> be the case, but better safe than sorry). I'll even apply
> https://lore.kernel.org/all/[email protected]/raw
> to be able to test sh4.
>

Meh, should have figured. That fixes one problem with sh4 builds
and creates another. Should have figured.

Guenter


2023-07-03 17:02:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Mon, 3 Jul 2023 at 00:08, Helge Deller <[email protected]> wrote:
>
> Great, that patch fixes it!

Yeah, I was pretty sure this was it, but it's good to have it
confirmed. Committed.

> I wonder if you want to
> #define VM_STACK_EARLY VM_GROWSDOWN
> even for the case where the stack grows down too (instead of 0),
> just to make clear that in both cases the stack goes downwards initially.

No, that wouldn't work for the simple reason that the special bits in
VM_STACK_INCOMPLETE_SETUP are always cleared after the stack setup is
done.

So if we added VM_GROWSDOWN to those early bits in general, the bit
would then be cleared even when that wasn't the intent.

Yes, yes, we could change the VM_STACK_INCOMPLETE_SETUP logic to only
clear some of the bits in the end, but the end result would be
practically the same: we'd still have to do different things for
grows-up vs grows-down cases, so the difference might as well be here
in the VM_STACK_EARLY bit.

Linus

2023-07-03 17:35:04

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/3/23 09:49, Linus Torvalds wrote:
> On Mon, 3 Jul 2023 at 00:08, Helge Deller <[email protected]> wrote:
>>
>> Great, that patch fixes it!
>
> Yeah, I was pretty sure this was it, but it's good to have it
> confirmed. Committed.
>

FWIW, my qemu boot tests didn't find any problems with other architectures.

Guenter

>> I wonder if you want to
>> #define VM_STACK_EARLY VM_GROWSDOWN
>> even for the case where the stack grows down too (instead of 0),
>> just to make clear that in both cases the stack goes downwards initially.
>
> No, that wouldn't work for the simple reason that the special bits in
> VM_STACK_INCOMPLETE_SETUP are always cleared after the stack setup is
> done.
>
> So if we added VM_GROWSDOWN to those early bits in general, the bit
> would then be cleared even when that wasn't the intent.
>
> Yes, yes, we could change the VM_STACK_INCOMPLETE_SETUP logic to only
> clear some of the bits in the end, but the end result would be
> practically the same: we'd still have to do different things for
> grows-up vs grows-down cases, so the difference might as well be here
> in the VM_STACK_EARLY bit.
>
> Linus


2023-07-03 17:44:32

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On Mon, 3 Jul 2023 at 10:19, Guenter Roeck <[email protected]> wrote:
>
> FWIW, my qemu boot tests didn't find any problems with other architectures.

Thanks. This whole "let's get the stack expansion locking right"
wasn't exactly buttery smooth, but given all our crazy architectures
it was not entirely unexpected.

Let's hope it really is all done now,

Linus

2023-07-03 19:45:30

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long

On 7/3/23 18:49, Linus Torvalds wrote:
> On Mon, 3 Jul 2023 at 00:08, Helge Deller <[email protected]> wrote:
>>
>> Great, that patch fixes it!
>
> Yeah, I was pretty sure this was it, but it's good to have it
> confirmed. Committed.

Thank you!

Nice to see that Greg picked up the patch for stable that fast as well!

>> I wonder if you want to
>> #define VM_STACK_EARLY VM_GROWSDOWN
>> even for the case where the stack grows down too (instead of 0),
>> just to make clear that in both cases the stack goes downwards initially.
>
> No, that wouldn't work for the simple reason that the special bits in
> VM_STACK_INCOMPLETE_SETUP are always cleared after the stack setup is
> done.
>
> So if we added VM_GROWSDOWN to those early bits in general, the bit
> would then be cleared even when that wasn't the intent.
>
> Yes, yes, we could change the VM_STACK_INCOMPLETE_SETUP logic to only
> clear some of the bits in the end, but the end result would be
> practically the same: we'd still have to do different things for
> grows-up vs grows-down cases, so the difference might as well be here
> in the VM_STACK_EARLY bit.

Ok, thanks for explainig!

Helge

2023-07-03 20:16:18

by Sam James

[permalink] [raw]
Subject: Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long


Helge Deller <[email protected]> writes:

> On 7/3/23 18:49, Linus Torvalds wrote:
>> On Mon, 3 Jul 2023 at 00:08, Helge Deller <[email protected]> wrote:
>>>
>>> Great, that patch fixes it!
>>
>> Yeah, I was pretty sure this was it, but it's good to have it
>> confirmed. Committed.
>
> Thank you!
>
> Nice to see that Greg picked up the patch for stable that fast as well!

Sorry, where? I was just about to check if it was marked for backporting
but I can't see it in Greg's trees yet.

We need it fo 6.1, 6.3, 6.4.

(Apologies if I'm missing it somewhere obvious.)