[PATCH] powerpc/fadump: Fix the race in crash_fadump().

[Michael Ellerman]

Mahesh Jagannath Salgaonkar <mahesh at linux.vnet.ibm.com> writes:

> On 10/10/2016 04:22 PM, Michael Ellerman wrote:
>> Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> writes:
>> 
>>> From: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
>>>
>>> There are chances that multiple CPUs can call crash_fadump() simultaneously
>>> and would start duplicating same info to vmcoreinfo ELF note section. This
>>> causes makedumpfile to fail during kdump capture. One example is,
>>> triggering dumprestart from HMC which sends system reset to all the CPUs at
>>> once.
>> ...
>>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>>> index b3a6633..2ed9d1c 100644
>>> --- a/arch/powerpc/kernel/fadump.c
>>> +++ b/arch/powerpc/kernel/fadump.c
>>> @@ -402,8 +402,14 @@ void crash_fadump(struct pt_regs *regs, const char *str)
>>>  {
>>>  	struct fadump_crash_info_header *fdh = NULL;
>>>  
>>> -	if (!fw_dump.dump_registered || !fw_dump.fadumphdr_addr)
>>> +	mutex_lock(&fadump_mutex);
>> 
>> What happens when a crashing CPU can't get the mutex and goes to sleep?
>
> Got your point. I think I should use mutex_trylock() here. There is only
> two reason crashing CPU can't get mutex, 1) Another CPU also crashing
> that got the mutex and on its way to trigger fadump. OR 2) We are in
> middle of fadump register/un-register, in which case we can just return
> and go to normal panic.

mutex_trylock() is probably OK, though with DEBUG options on it might
still run a bunch of code, you don't want it to go and call into lockdep
for example.

Normally the advice is not to write your own locks, but in this case the
safest option might be just to do a cmpxchg() or something similarly
simple.

cheers