RE: [EXT] [PATCH] qed: Fix a potential double-free in qed_cxt_tables_alloc

[Suman Ghosh]

>> >+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
>> >@@ -933,6 +933,7 @@ static void qed_ilt_shadow_free(struct qed_hwfn
>> >*p_hwfn)
>> > 		p_dma->virt_addr = NULL;
>> > 	}
>> > 	kfree(p_mngr->ilt_shadow);
>> >+	p_hwfn->p_cxt_mngr->ilt_shadow = NULL;
>> [Suman] Hi Dinghao,
>>
>> I am not sure how this will help prevent the double free here? If
>> qed_ilt_shadow_alloc() fails to allocate memory, then still
>qed_cxt_mngr_free() will be called and kfree() will try to free the NULL
>pointer. Shouldn't it be like below?
>>
>> if (p_mngr->ilt_shadow)
>> 	Kfree(p_mngr->ilt_shadow);
>> > }
>> >
>> > static int qed_ilt_blk_alloc(struct qed_hwfn *p_hwfn,
>> >--
>> >2.17.1
>> >
>
>kfree(NULL) is safe in kernel. But kfree() will not set the freed
>pointer to NULL. Therefore, checking p_mngr->ilt_shadow will not prevent
>the kfree() for the second time. Many double-free bugs are fixed by
>setting the freed pointer to NULL (e.g.,
>6b0d0477fce7 ("media: dvb-core: Fix double free in
>dvb_register_device()")), so I just fix this bug in the same way.
>
>Regards,
>Dinghao
[Suman] Okay, I understand. Along with this change, I have couple of suggestion. But it is up to you to make them.
1. In the beginning of the function qed_ilt_shadow_free() should we add a check and return if ilt_shadew == NULL? So, that the control does not reach to the end of the function? 
2. I see in qed_ilt_shadow_alloc() we are calling "goto ilt_shadow_fail" even if the kcalloc() is failing. If kcalloc() fails, then there is nothing to free, and we can directly return from there, right?