Messages in this thread | | | Date | Mon, 3 Jul 2023 05:59:18 -0700 | From | Guenter Roeck <> | 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 <linux@roeck-us.net> 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/20230609075528.9390-12-bhe@redhat.com/raw to be able to test sh4.
Guenter
| |