* [PATCH] powerpc/fault: kernel can extend a user process's stack
@ 2019-12-11 1:43 Daniel Axtens
2019-12-11 6:14 ` Daniel Black
2019-12-11 7:28 ` Michal Suchánek
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Axtens @ 2019-12-11 1:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Tom Lane, Daniel Black, Daniel Axtens
If a process page-faults trying to write beyond the end of its
stack, we attempt to grow the stack.
However, if the kernel attempts to write beyond the end of a
process's stack, it takes a bad fault. This can occur when the
kernel is trying to set up a signal frame.
Permit the kernel to grow a process's stack. The same general
limits as to how and when the stack can be grown apply: the kernel
code still passes through expand_stack(), so anything that goes
beyond e.g. the rlimit should still be blocked.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=205183
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Daniel Black <daniel@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
arch/powerpc/mm/fault.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index b5047f9b5dec..00183731ea22 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -287,7 +287,17 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
if (!res)
return !store_updates_sp(inst);
*must_retry = true;
+ } else if ((flags & FAULT_FLAG_WRITE) &&
+ !(flags & FAULT_FLAG_USER)) {
+ /*
+ * the kernel can also attempt to write beyond the end
+ * of a process's stack - for example setting up a
+ * signal frame. We assume this is valid, subject to
+ * the checks in expand_stack() later.
+ */
+ return false;
}
+
return true;
}
return false;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2019-12-11 1:43 [PATCH] powerpc/fault: kernel can extend a user process's stack Daniel Axtens
@ 2019-12-11 6:14 ` Daniel Black
2019-12-11 7:28 ` Michal Suchánek
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Black @ 2019-12-11 6:14 UTC (permalink / raw)
To: Daniel Axtens; +Cc: Tom Lane, linuxppc-dev
On Wed, 11 Dec 2019 12:43:37 +1100
Daniel Axtens <dja@axtens.net> wrote:
> If a process page-faults trying to write beyond the end of its
> stack, we attempt to grow the stack.
>
> However, if the kernel attempts to write beyond the end of a
> process's stack, it takes a bad fault. This can occur when the
> kernel is trying to set up a signal frame.
>
> Permit the kernel to grow a process's stack. The same general
> limits as to how and when the stack can be grown apply: the kernel
> code still passes through expand_stack(), so anything that goes
> beyond e.g. the rlimit should still be blocked.
Thanks Daniel.
Looks good from a function perspective.
danielgb@ozrom2:~$ gcc -g -Wall -O stactest.c
danielgb@ozrom2:~$ ./a.out 1240000 &
[1] 4223
danielgb@ozrom2:~$ cat /proc/$(pidof a.out)/maps | grep stack
^[[3~7ffff14d0000-7ffff1600000 rw-p 00000000 00:00 0 [stack]
danielgb@ozrom2:~$ kill -USR1 %1
danielgb@ozrom2:~$ signal delivered, stack base 0x7ffff1600000 top 0x7ffff14d1427 (1240025 used)
[1]+ Done ./a.out 1240000
danielgb@ozrom2:~$ ./a.out 1241000 &
[1] 4227
danielgb@ozrom2:~$ kill -USR1 %1
danielgb@ozrom2:~$ signal delivered, stack base 0x7fffff630000 top 0x7fffff501057 (1241001 used)
[1]+ Done ./a.out 1241000
danielgb@ozrom2:~$ uname -a
Linux ozrom2 5.5.0-rc1-00001-g83ab444248c1 #1 SMP Wed Dec 11 17:01:50 AEDT 2019 ppc64le ppc64le ppc64le GNU/Linux
Tested-by: Daniel Black <daniel@linux.ibm.com>
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=205183
> Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
> Cc: Daniel Black <daniel@linux.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> arch/powerpc/mm/fault.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index b5047f9b5dec..00183731ea22 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -287,7 +287,17 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
> if (!res)
> return !store_updates_sp(inst);
> *must_retry = true;
> + } else if ((flags & FAULT_FLAG_WRITE) &&
> + !(flags & FAULT_FLAG_USER)) {
> + /*
> + * the kernel can also attempt to write beyond the end
> + * of a process's stack - for example setting up a
> + * signal frame. We assume this is valid, subject to
> + * the checks in expand_stack() later.
> + */
> + return false;
> }
> +
> return true;
> }
> return false;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2019-12-11 1:43 [PATCH] powerpc/fault: kernel can extend a user process's stack Daniel Axtens
2019-12-11 6:14 ` Daniel Black
@ 2019-12-11 7:28 ` Michal Suchánek
2019-12-11 9:37 ` Daniel Axtens
1 sibling, 1 reply; 7+ messages in thread
From: Michal Suchánek @ 2019-12-11 7:28 UTC (permalink / raw)
To: Daniel Axtens; +Cc: Tom Lane, linuxppc-dev, Daniel Black
On Wed, Dec 11, 2019 at 12:43:37PM +1100, Daniel Axtens wrote:
> If a process page-faults trying to write beyond the end of its
> stack, we attempt to grow the stack.
>
> However, if the kernel attempts to write beyond the end of a
> process's stack, it takes a bad fault. This can occur when the
> kernel is trying to set up a signal frame.
>
> Permit the kernel to grow a process's stack. The same general
> limits as to how and when the stack can be grown apply: the kernel
> code still passes through expand_stack(), so anything that goes
> beyond e.g. the rlimit should still be blocked.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=205183
Fixes: 14cf11af6cf6 ("powerpc: Merge enough to start building in
arch/powerpc.")
> Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
> Cc: Daniel Black <daniel@linux.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> arch/powerpc/mm/fault.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index b5047f9b5dec..00183731ea22 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -287,7 +287,17 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
> if (!res)
> return !store_updates_sp(inst);
> *must_retry = true;
> + } else if ((flags & FAULT_FLAG_WRITE) &&
> + !(flags & FAULT_FLAG_USER)) {
> + /*
> + * the kernel can also attempt to write beyond the end
> + * of a process's stack - for example setting up a
> + * signal frame. We assume this is valid, subject to
> + * the checks in expand_stack() later.
> + */
> + return false;
> }
> +
> return true;
> }
> return false;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2019-12-11 7:28 ` Michal Suchánek
@ 2019-12-11 9:37 ` Daniel Axtens
2020-07-20 10:51 ` Michal Suchánek
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Axtens @ 2019-12-11 9:37 UTC (permalink / raw)
To: Michal Suchánek; +Cc: Tom Lane, linuxppc-dev, Daniel Black
> Fixes: 14cf11af6cf6 ("powerpc: Merge enough to start building in
> arch/powerpc.")
Wow, that's pretty ancient! I'm also not sure it's right - in that same
patch, arch/ppc64/mm/fault.c contains:
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 213) if (address + 2048 < uregs->gpr[1]
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 214) && (!user_mode(regs) || !store_updates_sp(regs)))
^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 215) goto bad_area;
Which is the same as the new arch/powerpc/mm/fault.c code:
14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 234) if (address + 2048 < uregs->gpr[1]
14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 235) && (!user_mode(regs) || !store_updates_sp(regs)))
14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 236) goto bad_area;
So either they're both right or they're both wrong, either way I'm not
sure how this patch is to blame.
I guess we should also cc stable@...
Regards,
Daniel
>> Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
>> Cc: Daniel Black <daniel@linux.ibm.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> ---
>> arch/powerpc/mm/fault.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index b5047f9b5dec..00183731ea22 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -287,7 +287,17 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
>> if (!res)
>> return !store_updates_sp(inst);
>> *must_retry = true;
>> + } else if ((flags & FAULT_FLAG_WRITE) &&
>> + !(flags & FAULT_FLAG_USER)) {
>> + /*
>> + * the kernel can also attempt to write beyond the end
>> + * of a process's stack - for example setting up a
>> + * signal frame. We assume this is valid, subject to
>> + * the checks in expand_stack() later.
>> + */
>> + return false;
>> }
>> +
>> return true;
>> }
>> return false;
>> --
>> 2.20.1
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2019-12-11 9:37 ` Daniel Axtens
@ 2020-07-20 10:51 ` Michal Suchánek
2020-07-20 13:52 ` Michael Ellerman
0 siblings, 1 reply; 7+ messages in thread
From: Michal Suchánek @ 2020-07-20 10:51 UTC (permalink / raw)
To: Daniel Axtens; +Cc: Tom Lane, linuxppc-dev, Daniel Black
Hello,
On Wed, Dec 11, 2019 at 08:37:21PM +1100, Daniel Axtens wrote:
> > Fixes: 14cf11af6cf6 ("powerpc: Merge enough to start building in
> > arch/powerpc.")
>
> Wow, that's pretty ancient! I'm also not sure it's right - in that same
> patch, arch/ppc64/mm/fault.c contains:
>
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 213) if (address + 2048 < uregs->gpr[1]
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 214) && (!user_mode(regs) || !store_updates_sp(regs)))
> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 215) goto bad_area;
>
> Which is the same as the new arch/powerpc/mm/fault.c code:
>
> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 234) if (address + 2048 < uregs->gpr[1]
> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 235) && (!user_mode(regs) || !store_updates_sp(regs)))
> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 236) goto bad_area;
>
> So either they're both right or they're both wrong, either way I'm not
> sure how this patch is to blame.
Is there any progress on resolving this?
I did not notice any followup patch nor this one being merged/refuted.
Thanks
Michal
>
> I guess we should also cc stable@...
>
> Regards,
> Daniel
>
> >> Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
> >> Cc: Daniel Black <daniel@linux.ibm.com>
> >> Signed-off-by: Daniel Axtens <dja@axtens.net>
> >> ---
> >> arch/powerpc/mm/fault.c | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> >> index b5047f9b5dec..00183731ea22 100644
> >> --- a/arch/powerpc/mm/fault.c
> >> +++ b/arch/powerpc/mm/fault.c
> >> @@ -287,7 +287,17 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
> >> if (!res)
> >> return !store_updates_sp(inst);
> >> *must_retry = true;
> >> + } else if ((flags & FAULT_FLAG_WRITE) &&
> >> + !(flags & FAULT_FLAG_USER)) {
> >> + /*
> >> + * the kernel can also attempt to write beyond the end
> >> + * of a process's stack - for example setting up a
> >> + * signal frame. We assume this is valid, subject to
> >> + * the checks in expand_stack() later.
> >> + */
> >> + return false;
> >> }
> >> +
> >> return true;
> >> }
> >> return false;
> >> --
> >> 2.20.1
> >>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2020-07-20 10:51 ` Michal Suchánek
@ 2020-07-20 13:52 ` Michael Ellerman
2020-07-21 0:57 ` Daniel Axtens
0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2020-07-20 13:52 UTC (permalink / raw)
To: Michal Suchánek, Daniel Axtens; +Cc: Tom Lane, linuxppc-dev, Daniel Black
Michal Suchánek <msuchanek@suse.de> writes:
> Hello,
>
> On Wed, Dec 11, 2019 at 08:37:21PM +1100, Daniel Axtens wrote:
>> > Fixes: 14cf11af6cf6 ("powerpc: Merge enough to start building in
>> > arch/powerpc.")
>>
>> Wow, that's pretty ancient! I'm also not sure it's right - in that same
>> patch, arch/ppc64/mm/fault.c contains:
>>
>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 213) if (address + 2048 < uregs->gpr[1]
>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 214) && (!user_mode(regs) || !store_updates_sp(regs)))
>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 215) goto bad_area;
>>
>> Which is the same as the new arch/powerpc/mm/fault.c code:
>>
>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 234) if (address + 2048 < uregs->gpr[1]
>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 235) && (!user_mode(regs) || !store_updates_sp(regs)))
>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 236) goto bad_area;
>>
>> So either they're both right or they're both wrong, either way I'm not
>> sure how this patch is to blame.
>
> Is there any progress on resolving this?
>
> I did not notice any followup patch nor this one being merged/refuted.
It ended up with this:
https://lore.kernel.org/linuxppc-dev/20200703141327.1732550-2-mpe@ellerman.id.au/
Which I was hoping would get some reviews :)
I'll probably merge the whole series into next this week.
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] powerpc/fault: kernel can extend a user process's stack
2020-07-20 13:52 ` Michael Ellerman
@ 2020-07-21 0:57 ` Daniel Axtens
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Axtens @ 2020-07-21 0:57 UTC (permalink / raw)
To: Michael Ellerman, Michal Suchánek
Cc: Tom Lane, linuxppc-dev, Daniel Black
Michael Ellerman <mpe@ellerman.id.au> writes:
> Michal Suchánek <msuchanek@suse.de> writes:
>> Hello,
>>
>> On Wed, Dec 11, 2019 at 08:37:21PM +1100, Daniel Axtens wrote:
>>> > Fixes: 14cf11af6cf6 ("powerpc: Merge enough to start building in
>>> > arch/powerpc.")
>>>
>>> Wow, that's pretty ancient! I'm also not sure it's right - in that same
>>> patch, arch/ppc64/mm/fault.c contains:
>>>
>>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 213) if (address + 2048 < uregs->gpr[1]
>>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 214) && (!user_mode(regs) || !store_updates_sp(regs)))
>>> ^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 215) goto bad_area;
>>>
>>> Which is the same as the new arch/powerpc/mm/fault.c code:
>>>
>>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 234) if (address + 2048 < uregs->gpr[1]
>>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 235) && (!user_mode(regs) || !store_updates_sp(regs)))
>>> 14cf11af6cf60 (Paul Mackerras 2005-09-26 16:04:21 +1000 236) goto bad_area;
>>>
>>> So either they're both right or they're both wrong, either way I'm not
>>> sure how this patch is to blame.
>>
>> Is there any progress on resolving this?
>>
>> I did not notice any followup patch nor this one being merged/refuted.
>
> It ended up with this:
>
> https://lore.kernel.org/linuxppc-dev/20200703141327.1732550-2-mpe@ellerman.id.au/
>
>
> Which I was hoping would get some reviews :)
Ah, I missed this. I'll give it a look as soon as I can.
Kind regards,
Daniel
>
> I'll probably merge the whole series into next this week.
>
> cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-21 0:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 1:43 [PATCH] powerpc/fault: kernel can extend a user process's stack Daniel Axtens
2019-12-11 6:14 ` Daniel Black
2019-12-11 7:28 ` Michal Suchánek
2019-12-11 9:37 ` Daniel Axtens
2020-07-20 10:51 ` Michal Suchánek
2020-07-20 13:52 ` Michael Ellerman
2020-07-21 0:57 ` Daniel Axtens
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).