Re: [PATCH] powerpc/fault: kernel can extend a user process's stack

[Daniel Axtens <dja@axtens.net>]

> 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
>>