Hi Linus,
On 7/3/23 08:20, Linus Torvalds wrote:
On Sun, 2 Jul 2023 at 22:33, Guenter Roeck <linux@xxxxxxxxxxxx> 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
