On 4/22/23 5:16 AM, Peizhi Li wrote: > When goto the mem_alloc_failure, it will result in a null pointer > reference to variable 'gl' in function 'qedi_free_global_queues', > due to the 'qedi->global_queues' not being allocated. > > Fix this by returning -EINVAL directly. > > Signed-off-by: Peizhi Li <meetlpz@xxxxxxxxxxx> > Reviewed-by: Dongliang Mu <dzm91@xxxxxxxxxxx> > Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > --- > The issue is found by static analysis and remains untested. Meanwhile, > this patches is similar with qedf which Jinhong Zhu fixed already. > > https://lore.kernel.org/all/20230417135518.184595-1-jinhongzhu@xxxxxxxxxxx/ > drivers/scsi/qedi/qedi_main.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c > index f2ee49756df8..34f38e0f1c7d 100644 > --- a/drivers/scsi/qedi/qedi_main.c > +++ b/drivers/scsi/qedi/qedi_main.c > @@ -1637,8 +1637,7 @@ static int qedi_alloc_global_queues(struct qedi_ctx *qedi) > * addresses of our queues > */ > if (!qedi->p_cpuq) { > - status = -EINVAL; > - goto mem_alloc_failure; > + return -EINVAL; > } > The patch looks ok: Reviewed-by: Mike Christie <michael.christie@xxxxxxxxxx> Looks like there is another issue in this code path though. If qedi_alloc_global_queues fails in qedi_set_iscsi_pf_param the err_alloc_mem goto just ends up returning, so the p_cpuq allocation a couple lines before the qedi_set_iscsi_pf_param call will be leaked. The __qedi_probe call to qedi_set_iscsi_pf_param just frees the iscsi host, so it won't be freed via a qedi_free_iscsi_pf_param later. So, I think qedi_set_iscsi_pf_param's err_alloc_mem goto handler should free the p_cpuq.
