Hi Lin,
On 23/06/19, 7:28 AM, "linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of Lin
Yi" <linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of teroincn@xxxxxxx> wrote:
>if cb_arg alloc failed, we can't release orig_io_req refcount before
>we take it's refcount. call kref_get before malloc, so as to pair with
>the kref_put on the srr_err path.
>
>Signed-off-by: Lin Yi <teroincn@xxxxxxx>
>---
> drivers/scsi/bnx2fc/bnx2fc_els.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/bnx2fc/bnx2fc_els.c
>b/drivers/scsi/bnx2fc/bnx2fc_els.c
>index 709bb92..c201ddf 100644
>--- a/drivers/scsi/bnx2fc/bnx2fc_els.c
>+++ b/drivers/scsi/bnx2fc/bnx2fc_els.c
>@@ -633,13 +633,13 @@ int bnx2fc_send_srr(struct bnx2fc_cmd *orig_io_req,
>u32 offset, u8 r_ctl)
> BNX2FC_IO_DBG(orig_io_req, "Sending SRR\n");
> memset(&srr, 0, sizeof(srr));
>
>+ kref_get(&orig_io_req->refcount);
> cb_arg = kzalloc(sizeof(struct bnx2fc_els_cb_arg), GFP_ATOMIC);
> if (!cb_arg) {
> printk(KERN_ERR PFX "Unable to allocate cb_arg for SRR\n");
> rc = -ENOMEM;
> goto srr_err;
> }
>- kref_get(&orig_io_req->refcount);
>
> cb_arg->aborted_io_req = orig_io_req;
>
>--
>1.9.1
Thanks for the patch, but this is not the correct fix. If kzalloc fails,
control will reach label srr_err and try to free cb_arg.
Correct fix is to move the srr_err label down.
@@ -680,7 +680,6 @@ int bnx2fc_send_srr(struct bnx2fc_cmd *orig_io_req,
u32 offset, u8 r_ctl)
rc = bnx2fc_initiate_els(tgt, ELS_SRR, &srr, sizeof(srr),
bnx2fc_srr_compl, cb_arg,
r_a_tov);
-srr_err:
if (rc) {
BNX2FC_IO_DBG(orig_io_req, "SRR failed - release\n");
spin_lock_bh(&tgt->tgt_lock);
@@ -690,6 +689,7 @@ srr_err:
} else
set_bit(BNX2FC_FLAG_SRR_SENT, &orig_io_req->req_flags);
+srr_err:
return rc;
}
Submit an update patch.
Thanks,
~Saurav
>
