Re: [PATCH] qla2xxx: Fix a NULL pointer dereference in an error path

[Bart Van Assche <bvanassche@acm.org>]

On 1/13/20 7:29 AM, Ewan D. Milne wrote:
> On Sun, 2020-01-12 at 13:08 -0800, Bart Van Assche wrote:
>> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
>> index c4e087217484..6560908ed50e 100644
>> --- a/drivers/scsi/qla2xxx/qla_init.c
>> +++ b/drivers/scsi/qla2xxx/qla_init.c
>> @@ -4895,6 +4895,8 @@ qla2x00_alloc_fcport(scsi_qla_host_t *vha, gfp_t flags)
>>   void
>>   qla2x00_free_fcport(fc_port_t *fcport)
>>   {
>> +	if (!fcport)
>> +		return;
>>   	if (fcport->ct_desc.ct_sns) {
>>   		dma_free_coherent(&fcport->vha->hw->pdev->dev,
>>   			sizeof(struct ct_sns_pkt), fcport->ct_desc.ct_sns,
>>
> 
> I would have fixed this by moving the label to be after the qla2x00_free_fcport()
> call in qla2x00_configure_local_loop(), which Coverity complained about.  And
> called it something different.  (The code could probably be simplified by only
> making a call to qla2x00_alloc_fcport() in one place, something to think about...)

Hi Ewan,

I have considered the solution that you proposed. The solution shown 
above was chosen because I did not want to introduce any memory leaks in 
qla2x00_configure_local_loop(). There is a "goto cleanup_allocation" 
statement in that function that occurs after the "new_fcport = 
qla2x00_alloc_fcport(vha, GFP_KERNEL)" assignment. That is hard to 
notice because the qla2x00_configure_local_loop() function is so long.

Thanks,

Bart.