Re: [PATCH] powerpc: Fix possible deadlock on page fault

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Thu, 2013-09-05 at 17:18 +0530, Aneesh Kumar K.V wrote:
>> Paul Mackerras <paulus@samba.org> writes:
>> 
>> > On Thu, Sep 05, 2013 at 12:47:02PM +0530, Aneesh Kumar K.V wrote:
>> >
>> >> @@ -280,6 +280,13 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
>> >>  
>> >>  	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
>> >>  
>> >> +	/*
>> >> +	 * We want to do this outside mmap_sem, because reading code around nip
>> >> +	 * can result in fault, which will cause a deadlock when called with
>> >> +	 * mmap_sem held
>> >> +	 */
>> >> +	store_update = store_updates_sp(regs);
>> >
>> > We should only call store_updates_sp() if user_mode(regs); that was
>> > the previous behaviour.
>> 
>> Updated to 
>> 
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index 8726779..fad7af6 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -206,7 +206,7 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
>>  	int trap = TRAP(regs);
>>   	int is_exec = trap == 0x400;
>>  	int fault;
>> -	int rc = 0;
>> +	int rc = 0, store_update = 0;
>
> Keep the "sp", in the name, it's confusing otherwise. It's not just
> about "store update", it's about specifically recognizing instructions
> used to update the stack frame in order to let them and only them
> significantly lower the stack pointer.
>

Ok will do that. I posted a v2. So will wait for other feedback before i
post a new version.

-aneesh