Hi Linus,
On 7/3/23 08: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.
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