Re: [PATCH] ipmi/powernv: Fix potential invalid pointer dereference

[Neelesh Gupta <neelegup@linux.vnet.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]

Hi Corey,

On 07/16/2015 08:31 PM, Corey Minyard wrote:
> Ok, this looks fine.  A couple of question...
>
> Do I need to send this upstream right now?  How well has this been tested?

I would want either Jeremy or Alistair to review this patch before you 
send this
upstream. There is also firmware piece 
http://patchwork.ozlabs.org/patch/496645/
awaiting review.

In the testing front, I manually made the opal_ipmi_recv() function to 
fail for testing
the error path and see if the driver recovers from it and subsequent 
ipmi commands
work all good.

>
> Do you want this backported to 4.0 stable?

Yes, I want this to be be backported to 4.0 stable.

Thanks,
Neelesh.

>
> -corey
>
> On 07/16/2015 06:16 AM, Neelesh Gupta wrote:
>> If the OPAL call to receive the ipmi message fails, then we free up the
>> smi message and return. But, the driver still holds the reference to
>> old smi message in the 'cur_msg' which can potentially be accessed later
>> and freed again leading to kernel oops. To fix it up,
>>
>> The kernel driver should reset the 'cur_msg' and send reply to the user
>> in addition to freeing the message.
>>
>> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
>> ---
>>   drivers/char/ipmi/ipmi_powernv.c |   13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c
>> index 9b409c0..637486d 100644
>> --- a/drivers/char/ipmi/ipmi_powernv.c
>> +++ b/drivers/char/ipmi/ipmi_powernv.c
>> @@ -143,9 +143,16 @@ static int ipmi_powernv_recv(struct ipmi_smi_powernv *smi)
>>   	pr_devel("%s:   -> %d (size %lld)\n", __func__,
>>   			rc, rc == 0 ? size : 0);
>>   	if (rc) {
>> -		spin_unlock_irqrestore(&smi->msg_lock, flags);
>> -		ipmi_free_smi_msg(msg);
>> -		return 0;
>> +		/* If came via the poll, and response was not yet ready */
>> +		if (rc == OPAL_EMPTY) {
>> +			spin_unlock_irqrestore(&smi->msg_lock, flags);
>> +			return 0;
>> +		} else {
>> +			smi->cur_msg = NULL;
>> +			spin_unlock_irqrestore(&smi->msg_lock, flags);
>> +			send_error_reply(smi, msg, IPMI_ERR_UNSPECIFIED);
>> +			return 0;
>> +		}
>>   	}
>>   
>>   	if (size < sizeof(*opal_msg)) {
>>


[-- Attachment #2: Type: text/html, Size: 3147 bytes --]