Messages in this thread | | | Date | Mon, 24 Apr 2023 10:22:42 -0500 | Subject | Re: [PATCH] scsi: qedi: qedi_main.c: modify the code flow in 'qedi_alloc_global_queues' | From | Mike Christie <> |
| |
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@hust.edu.cn> > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn> > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > 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@hust.edu.cn/ > 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@oracle.com>
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.
| |