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

From: Corey Minyard <[email-masked]>
Date: Thu, 16 Jul 2015 10:01:27 -0500

Ok, this looks fine.  A couple of question...

Do I need to send this upstream right now?  How well has this been tested?

Do you want this backported to 4.0 stable?

-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 <[email-masked]>
> ---
>  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)) {
>