[Skiboot] [PATCH] fsp/ipmi: Fix an illegal memory access

[Neelesh Gupta]

Hi Kamalesh,

On 07/13/2015 02:59 PM, Kamalesh Babulal wrote:
>
> On 07/13/2015 10:36 AM, Neelesh Gupta wrote:
>> The patch fixes an illegal access to the memory which has been
>> freed.
>>
>> Fixes Coverity defect # 101858
>>
>> Signed-off-by: Neelesh Gupta <neelegup at linux.vnet.ibm.com>
>> ---
>>   hw/fsp/fsp-ipmi.c |    3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/hw/fsp/fsp-ipmi.c b/hw/fsp/fsp-ipmi.c
>> index e5cd6e2..729ff93 100644
>> --- a/hw/fsp/fsp-ipmi.c
>> +++ b/hw/fsp/fsp-ipmi.c
>> @@ -87,13 +87,12 @@ static void fsp_ipmi_req_complete(struct fsp_msg *msg)
>>   {
>>   	uint8_t status = (msg->resp->word1 >> 8) & 0xff;
>>   	uint32_t length = msg->resp->data.words[0];
>> -	struct fsp_ipmi_msg *fsp_ipmi_msg;
>> +	struct fsp_ipmi_msg *fsp_ipmi_msg = msg->user_data;
>>   	struct ipmi_msg *ipmi_msg;
>>   
>>   	fsp_freemsg(msg);
>>   
>>   	if (status != FSP_STATUS_SUCCESS) {
>> -		fsp_ipmi_msg = msg->user_data;
>>   		assert(fsp_ipmi_msg == fsp_ipmi.cur_msg);
>>   
>>   		ipmi_msg = &fsp_ipmi_msg->ipmi_msg;
> fsp_free(msg) means msg->user_data is also un-allocated. It means
> fsp_ipmi_msg is pointing
> to memory which is freed. From the code, I am guessing you need
> free(msg) before you
> call fsp_ipmi_send_request(). If my assumption is right, how about below
> code ?

'msg->user_data' is a private data to this driver and the driver has to
take care of it. For instance, 'fsp_ipmi_msg' can only be free'd through
a backend callback i.e., fsp_ipmi_free_msg() in this driver.

Neelesh.

>
> --- a/hw/fsp/fsp-ipmi.c
> +++ b/hw/fsp/fsp-ipmi.c
> @@ -90,8 +90,6 @@ static void fsp_ipmi_req_complete(struct fsp_msg *msg)
>          struct fsp_ipmi_msg *fsp_ipmi_msg;
>          struct ipmi_msg *ipmi_msg;
>
> -       fsp_freemsg(msg);
> -
>          if (status != FSP_STATUS_SUCCESS) {
>                  fsp_ipmi_msg = msg->user_data;
>                  assert(fsp_ipmi_msg == fsp_ipmi.cur_msg);
> @@ -111,9 +109,14 @@ static void fsp_ipmi_req_complete(struct fsp_msg *msg)
>                                    IPMI_NETFN_RETURN_CODE(ipmi_msg->netfn),
>                                    IPMI_ERR_UNSPECIFIED);
>
> +              fsp_freemsg(msg);
> +
>                  /* Send the next request in the queue */
>                  fsp_ipmi_send_request();
> +              return;
>          }
> +
> +       fsp_freemsg(msg);
>   }
>
>   static int fsp_ipmi_send_request(void)
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20150713/20a6dfba/attachment.html>