[PATCH] qla2xxx: double free bug in qla2x00_probe_one

[PATCH] qla2xxx: double free bug in qla2x00_probe_one

[Hisashi Hifumi]

Hi.

I got double free bug at qla2x00_probe_one's error path and
kernel was panicked.
qla2x00_probe_one's probe_failed path is as follows:

 	qla2x00_free_que(ha, req, rsp);
 	qla2x00_free_device(base_vha);
 
qla2x00_free_device calls qla2x00_free_que through qla2x00_free_queues.
qla2x00_free_device frees the same pointer of req and rsp that is already 
freed by qla2x00_free_que.

Following patch fixes this problem.
Thanks.

Signed-off-by: Hisashi Hifumi <hifumi.hisashi@oss.ntt.co.jp>

diff -Nrup linux-2.6.29-rc1.org/drivers/scsi/qla2xxx/qla_os.c linux-2.6.29-rc1.qla2x/drivers/scsi/qla2xxx/qla_os.c
--- linux-2.6.29-rc1.org/drivers/scsi/qla2xxx/qla_os.c	2009-01-15 15:37:31.000000000 +0900
+++ linux-2.6.29-rc1.qla2x/drivers/scsi/qla2xxx/qla_os.c	2009-01-16 11:52:47.000000000 +0900
@@ -1999,6 +1999,8 @@ qla2x00_probe_one(struct pci_dev *pdev, 
 
 probe_failed:
 	qla2x00_free_que(ha, req, rsp);
+	ha->rsp_q_map[0] = NULL;
+	ha->req_q_map[0] = NULL;
 	qla2x00_free_device(base_vha);
 
 	scsi_host_put(base_vha->host);

Re: [PATCH] qla2xxx: double free bug in qla2x00_probe_one

[Anirban Chakraborty]

On Jan 15, 2009, at 7:13 PM, Hisashi Hifumi wrote:

> Hi.
>
> I got double free bug at qla2x00_probe_one's error path and
> kernel was panicked.
> qla2x00_probe_one's probe_failed path is as follows:
>
> 	qla2x00_free_que(ha, req, rsp);
> 	qla2x00_free_device(base_vha);
>
> qla2x00_free_device calls qla2x00_free_que through  
> qla2x00_free_queues.
> qla2x00_free_device frees the same pointer of req and rsp that is  
> already
> freed by qla2x00_free_que.

This has been taken care of in the ISP restart bug fix patch. Please  
find it here:
http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c
Thanks,
Anirban

Re: [PATCH] qla2xxx: double free bug in qla2x00_probe_one

[Hisashi Hifumi]

At 01:56 09/01/17, Anirban Chakraborty wrote:

>On Jan 15, 2009, at 7:13 PM, Hisashi Hifumi wrote:
>
>>Hi.
>>
>>I got double free bug at qla2x00_probe_one's error path and
>>kernel was panicked.
>>qla2x00_probe_one's probe_failed path is as follows:
>>
>>qla2x00_free_que(ha, req, rsp);
>>qla2x00_free_device(base_vha);
>>
>>qla2x00_free_device calls qla2x00_free_que through qla2x00_free_queues.
>>qla2x00_free_device frees the same pointer of req and rsp that is already 
>>freed by qla2x00_free_que.
>>
>>Following patch fixes this problem.
>>Thanks.
>This has been fixed as a part of the ISP restart bug fix. <http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c>http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c
>Thanks,
>Anirban

Your patch is as follows:

 probe_failed:
-       qla2x00_free_que(ha, req, rsp);
        qla2x00_free_device(base_vha);


I think just removing qla2x00_free_que here is wrong, because when qla2x00_request_irqs or
qla2x00_alloc_queues fails, no one frees rsp and req.

Re: [PATCH] qla2xxx: double free bug in qla2x00_probe_one

[James Bottomley]

On Mon, 2009-01-19 at 09:59 +0900, Hisashi Hifumi wrote:
> At 01:56 09/01/17, Anirban Chakraborty wrote:
> 
> >On Jan 15, 2009, at 7:13 PM, Hisashi Hifumi wrote:
> >
> >>Hi.
> >>
> >>I got double free bug at qla2x00_probe_one's error path and
> >>kernel was panicked.
> >>qla2x00_probe_one's probe_failed path is as follows:
> >>
> >>qla2x00_free_que(ha, req, rsp);
> >>qla2x00_free_device(base_vha);
> >>
> >>qla2x00_free_device calls qla2x00_free_que through qla2x00_free_queues.
> >>qla2x00_free_device frees the same pointer of req and rsp that is already 
> >>freed by qla2x00_free_que.
> >>
> >>Following patch fixes this problem.
> >>Thanks.
> >This has been fixed as a part of the ISP restart bug fix. <http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c>http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c
> >Thanks,
> >Anirban
> 
> Your patch is as follows:
> 
>  probe_failed:
> -       qla2x00_free_que(ha, req, rsp);
>         qla2x00_free_device(base_vha);
> 
> 
> I think just removing qla2x00_free_que here is wrong, because when qla2x00_request_irqs or
> qla2x00_alloc_queues fails, no one frees rsp and req.

Aren't these freed by

qla2x00_free_device()
qla2x00_free_queues()
qla2x00_free_que()

along that path?

James

Re: [PATCH] qla2xxx: double free bug in qla2x00_probe_one

[Hisashi Hifumi]

At 10:22 09/01/19, James Bottomley wrote:
>On Mon, 2009-01-19 at 09:59 +0900, Hisashi Hifumi wrote:
>> At 01:56 09/01/17, Anirban Chakraborty wrote:
>> 
>> >On Jan 15, 2009, at 7:13 PM, Hisashi Hifumi wrote:
>> >
>> >>Hi.
>> >>
>> >>I got double free bug at qla2x00_probe_one's error path and
>> >>kernel was panicked.
>> >>qla2x00_probe_one's probe_failed path is as follows:
>> >>
>> >>qla2x00_free_que(ha, req, rsp);
>> >>qla2x00_free_device(base_vha);
>> >>
>> >>qla2x00_free_device calls qla2x00_free_que through qla2x00_free_queues.
>> >>qla2x00_free_device frees the same pointer of req and rsp that is already 
>> >>freed by qla2x00_free_que.
>> >>
>> >>Following patch fixes this problem.
>> >>Thanks.
>> >This has been fixed as a part of the ISP restart bug fix. 
><http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c>http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c
>> >Thanks,
>> >Anirban
>> 
>> Your patch is as follows:
>> 
>>  probe_failed:
>> -       qla2x00_free_que(ha, req, rsp);
>>         qla2x00_free_device(base_vha);
>> 
>> 
>> I think just removing qla2x00_free_que here is wrong, because when 
>qla2x00_request_irqs or
>> qla2x00_alloc_queues fails, no one frees rsp and req.
>
>Aren't these freed by
>
>qla2x00_free_device()
>qla2x00_free_queues()
>qla2x00_free_que()
>
>along that path?
>
>James

Only qla2x00_free_que frees rsp and req above path, but just qla2x00_free_device
without qla2x00_free_que is ok after ha->rsp_q_map[0] and ha->req_q_map[0] definition
, such as failure of qla2x00_initialize_adapter , kthread_create or scsi_add_host.

Re: [PATCH] qla2xxx: double free bug in qla2x00_probe_one

[Anirban Chakraborty]

On Jan 18, 2009, at 5:50 PM, Hisashi Hifumi wrote:

>
> At 10:22 09/01/19, James Bottomley wrote:
>> On Mon, 2009-01-19 at 09:59 +0900, Hisashi Hifumi wrote:
>>> At 01:56 09/01/17, Anirban Chakraborty wrote:
>>>
>>>> On Jan 15, 2009, at 7:13 PM, Hisashi Hifumi wrote:
>>>>
>>>>> Hi.
>>>>>
>>>>> I got double free bug at qla2x00_probe_one's error path and
>>>>> kernel was panicked.
>>>>> qla2x00_probe_one's probe_failed path is as follows:
>>>>>
>>>>> qla2x00_free_que(ha, req, rsp);
>>>>> qla2x00_free_device(base_vha);
>>>>>
>>>>> qla2x00_free_device calls qla2x00_free_que through  
>>>>> qla2x00_free_queues.
>>>>> qla2x00_free_device frees the same pointer of req and rsp that  
>>>>> is already
>>>>> freed by qla2x00_free_que.
>>>>>
>>>>> Following patch fixes this problem.
>>>>> Thanks.
>>>> This has been fixed as a part of the ISP restart bug fix.
>> <http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c 
>> >http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=29bdccbee69c199910b2b39377e66ee5c33f241c
>>>> Thanks,
>>>> Anirban
>>>
>>> Your patch is as follows:
>>>
>>> probe_failed:
>>> -       qla2x00_free_que(ha, req, rsp);
>>>        qla2x00_free_device(base_vha);
>>>
>>>
>>> I think just removing qla2x00_free_que here is wrong, because when
>> qla2x00_request_irqs or
>>> qla2x00_alloc_queues fails, no one frees rsp and req.
>>
>> Aren't these freed by
>>
>> qla2x00_free_device()
>> qla2x00_free_queues()
>> qla2x00_free_que()
>>
>> along that path?
>>
>> James
>
> Only qla2x00_free_que frees rsp and req above path, but just  
> qla2x00_free_device
> without qla2x00_free_que is ok after ha->rsp_q_map[0] and ha- 
> >req_q_map[0] definition
> , such as failure of qla2x00_initialize_adapter , kthread_create or  
> scsi_add_host.
You are right. The resources are not freed if call to  
qla2x00_create_host fails. I will send the patch in subsequent mail.
Thanks a lot for pointing this out.

-Anirban