On 01/29/2016 08:31 PM, Sergei Temerkhanov wrote:
> On Fri, Jan 29, 2016 at 4:50 PM, Corey Minyard <miny...@acm.org> wrote:
>> On 01/28/2016 10:17 PM, Sergey Temerkhanov wrote:
>>> Fix NULL pointer dereference at the end of multi-part message send
>>> process caused by the buffer pointer being set to NULL too early
>>
>> Dang, I know I tested this. Unfortunately, the fix you have won't work. It
>> opens a race, msg_written_handler can be called again before ssif_i2c_send
>> returns.
>>
>> Can you save the value of ssif_info->multi_data in a local then call it with
>> that?
> The actual struct ssif_info setup is performed in ssif_i2c_send() so
> what if we just split it int 2 parts and issue the call to complete()
> after we've made sure that nothing is gonna race?
I'm not sure what you mean, I'd have to see a patch.
But it seems very simple to just save ssif_info->multi_data in a local
and pass that to ssif_i2c_send.
-corey
> Regards,
> Sergey
>
>> -corey
>>
>>
>>> ---
>>> drivers/char/ipmi/ipmi_ssif.c | 11 ++++++-----
>>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
>>> index dc3491c..2b222bf 100644
>>> --- a/drivers/char/ipmi/ipmi_ssif.c
>>> +++ b/drivers/char/ipmi/ipmi_ssif.c
>>> @@ -896,6 +896,12 @@ static void msg_written_handler(struct ssif_info
>>> *ssif_info, int result,
>>> /* Length byte. */
>>> ssif_info->multi_data[ssif_info->multi_pos] = left;
>>> ssif_info->multi_pos += left;
>>> +
>>> + rv = ssif_i2c_send(ssif_info, msg_written_handler,
>>> + I2C_SMBUS_WRITE,
>>> + SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
>>> + ssif_info->multi_data +
>>> ssif_info->multi_pos,
>>> + I2C_SMBUS_BLOCK_DATA);
>>> if (left < 32)
>>> /*
>>> * Write is finished. Note that we must end
>>> @@ -905,11 +911,6 @@ static void msg_written_handler(struct ssif_info
>>> *ssif_info, int result,
>>> */
>>> ssif_info->multi_data = NULL;
>>> - rv = ssif_i2c_send(ssif_info, msg_written_handler,
>>> - I2C_SMBUS_WRITE,
>>> - SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
>>> - ssif_info->multi_data +
>>> ssif_info->multi_pos,
>>> - I2C_SMBUS_BLOCK_DATA);
>>> if (rv < 0) {
>>> /* request failed, just return the error. */
>>> ssif_inc_stat(ssif_info, send_errors);
>>
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
_______________________________________________
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
