Messages in this thread | | | Date | Mon, 3 Jul 2023 06:06:47 -0700 | Subject | Re: [PATCH 6.4 00/28] 6.4.1-rc1 review - hppa argument list too long | From | Guenter Roeck <> |
| |
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 <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. >
Meh, should have figured. That fixes one problem with sh4 builds and creates another. Should have figured.
Guenter
| |