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