* [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates
@ 2021-07-01 19:08 Mike Christie
2021-07-19 1:19 ` Martin K. Petersen
2021-07-24 2:13 ` Martin K. Petersen
0 siblings, 2 replies; 3+ messages in thread
From: Mike Christie @ 2021-07-01 19:08 UTC (permalink / raw)
To: lyl2019, subbu.seetharaman, ketan.mukadam, jitendra.bhivare,
martin.petersen, linux-scsi, james.bottomley
Cc: Mike Christie
This fixes a bug found by Lv Yunlong where because beiscsi_exec_nemb_cmd
frees memory for the be_dma_mem cmd, we can access freed memory when
beiscsi_if_clr_ip/beiscsi_if_set_ip's call to beiscsi_exec_nemb_cmd fails
and we access the freed req. This fixes the issue by having the caller
free the cmd's memory.
Reported-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/scsi/be2iscsi/be_mgmt.c | 84 ++++++++++++++++++---------------
1 file changed, 45 insertions(+), 39 deletions(-)
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 462717bbb5b7..4e899ec1477d 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -235,8 +235,7 @@ static int beiscsi_exec_nemb_cmd(struct beiscsi_hba *phba,
wrb = alloc_mcc_wrb(phba, &tag);
if (!wrb) {
mutex_unlock(&ctrl->mbox_lock);
- rc = -ENOMEM;
- goto free_cmd;
+ return -ENOMEM;
}
sge = nonembedded_sgl(wrb);
@@ -269,24 +268,6 @@ static int beiscsi_exec_nemb_cmd(struct beiscsi_hba *phba,
/* copy the response, if any */
if (resp_buf)
memcpy(resp_buf, nonemb_cmd->va, resp_buf_len);
- /**
- * This is special case of NTWK_GET_IF_INFO where the size of
- * response is not known. beiscsi_if_get_info checks the return
- * value to free DMA buffer.
- */
- if (rc == -EAGAIN)
- return rc;
-
- /**
- * If FW is busy that is driver timed out, DMA buffer is saved with
- * the tag, only when the cmd completes this buffer is freed.
- */
- if (rc == -EBUSY)
- return rc;
-
-free_cmd:
- dma_free_coherent(&ctrl->pdev->dev, nonemb_cmd->size,
- nonemb_cmd->va, nonemb_cmd->dma);
return rc;
}
@@ -309,6 +290,19 @@ static int beiscsi_prep_nemb_cmd(struct beiscsi_hba *phba,
return 0;
}
+static void beiscsi_free_nemb_cmd(struct beiscsi_hba *phba,
+ struct be_dma_mem *cmd, int rc)
+{
+ /*
+ * If FW is busy the DMA buffer is saved with the tag. When the cmd
+ * completes this buffer is freed.
+ */
+ if (rc == -EBUSY)
+ return;
+
+ dma_free_coherent(&phba->ctrl.pdev->dev, cmd->size, cmd->va, cmd->dma);
+}
+
static void __beiscsi_eq_delay_compl(struct beiscsi_hba *phba, unsigned int tag)
{
struct be_dma_mem *tag_mem;
@@ -344,8 +338,16 @@ int beiscsi_modify_eq_delay(struct beiscsi_hba *phba,
cpu_to_le32(set_eqd[i].delay_multiplier);
}
- return beiscsi_exec_nemb_cmd(phba, &nonemb_cmd,
- __beiscsi_eq_delay_compl, NULL, 0);
+ rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, __beiscsi_eq_delay_compl,
+ NULL, 0);
+ if (rc) {
+ /*
+ * Only free on failure. Async cmds are handled like -EBUSY
+ * where it's handled for us.
+ */
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
+ }
+ return rc;
}
/**
@@ -372,6 +374,7 @@ int beiscsi_get_initiator_name(struct beiscsi_hba *phba, char *name, bool cfg)
req->hdr.version = 1;
rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL,
&resp, sizeof(resp));
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
if (rc) {
beiscsi_log(phba, KERN_ERR,
BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
@@ -449,7 +452,9 @@ static int beiscsi_if_mod_gw(struct beiscsi_hba *phba,
req->ip_addr.ip_type = ip_type;
memcpy(req->ip_addr.addr, gw,
(ip_type < BEISCSI_IP_TYPE_V6) ? IP_V4_LEN : IP_V6_LEN);
- return beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, NULL, 0);
+ rt_val = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, NULL, 0);
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rt_val);
+ return rt_val;
}
int beiscsi_if_set_gw(struct beiscsi_hba *phba, u32 ip_type, u8 *gw)
@@ -499,8 +504,10 @@ int beiscsi_if_get_gw(struct beiscsi_hba *phba, u32 ip_type,
req = nonemb_cmd.va;
req->ip_type = ip_type;
- return beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL,
- resp, sizeof(*resp));
+ rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, resp,
+ sizeof(*resp));
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
+ return rc;
}
static int
@@ -537,6 +544,7 @@ beiscsi_if_clr_ip(struct beiscsi_hba *phba,
"BG_%d : failed to clear IP: rc %d status %d\n",
rc, req->ip_params.ip_record.status);
}
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
return rc;
}
@@ -581,6 +589,7 @@ beiscsi_if_set_ip(struct beiscsi_hba *phba, u8 *ip,
if (req->ip_params.ip_record.status)
rc = -EINVAL;
}
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
return rc;
}
@@ -608,6 +617,7 @@ int beiscsi_if_en_static(struct beiscsi_hba *phba, u32 ip_type,
reldhcp->interface_hndl = phba->interface_handle;
reldhcp->ip_type = ip_type;
rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, NULL, 0);
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
if (rc < 0) {
beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
"BG_%d : failed to release existing DHCP: %d\n",
@@ -689,7 +699,7 @@ int beiscsi_if_en_dhcp(struct beiscsi_hba *phba, u32 ip_type)
dhcpreq->interface_hndl = phba->interface_handle;
dhcpreq->ip_type = ip_type;
rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, NULL, 0);
-
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
exit:
kfree(if_info);
return rc;
@@ -762,11 +772,8 @@ int beiscsi_if_get_info(struct beiscsi_hba *phba, int ip_type,
BEISCSI_LOG_INIT | BEISCSI_LOG_CONFIG,
"BG_%d : Memory Allocation Failure\n");
- /* Free the DMA memory for the IOCTL issuing */
- dma_free_coherent(&phba->ctrl.pdev->dev,
- nonemb_cmd.size,
- nonemb_cmd.va,
- nonemb_cmd.dma);
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd,
+ -ENOMEM);
return -ENOMEM;
}
@@ -781,15 +788,13 @@ int beiscsi_if_get_info(struct beiscsi_hba *phba, int ip_type,
nonemb_cmd.va)->actual_resp_len;
ioctl_size += sizeof(struct be_cmd_req_hdr);
- /* Free the previous allocated DMA memory */
- dma_free_coherent(&phba->ctrl.pdev->dev, nonemb_cmd.size,
- nonemb_cmd.va,
- nonemb_cmd.dma);
-
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
/* Free the virtual memory */
kfree(*if_info);
- } else
+ } else {
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
break;
+ }
} while (true);
return rc;
}
@@ -806,8 +811,9 @@ int mgmt_get_nic_conf(struct beiscsi_hba *phba,
if (rc)
return rc;
- return beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL,
- nic, sizeof(*nic));
+ rc = beiscsi_exec_nemb_cmd(phba, &nonemb_cmd, NULL, nic, sizeof(*nic));
+ beiscsi_free_nemb_cmd(phba, &nonemb_cmd, rc);
+ return rc;
}
static void beiscsi_boot_process_compl(struct beiscsi_hba *phba,
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates
2021-07-01 19:08 [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates Mike Christie
@ 2021-07-19 1:19 ` Martin K. Petersen
2021-07-24 2:13 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2021-07-19 1:19 UTC (permalink / raw)
To: Mike Christie
Cc: lyl2019, subbu.seetharaman, ketan.mukadam, jitendra.bhivare,
martin.petersen, linux-scsi, james.bottomley
Mike,
> This fixes a bug found by Lv Yunlong where because
> beiscsi_exec_nemb_cmd frees memory for the be_dma_mem cmd, we can
> access freed memory when beiscsi_if_clr_ip/beiscsi_if_set_ip's call to
> beiscsi_exec_nemb_cmd fails and we access the freed req. This fixes
> the issue by having the caller free the cmd's memory.
Applied to 5.15/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates
2021-07-01 19:08 [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates Mike Christie
2021-07-19 1:19 ` Martin K. Petersen
@ 2021-07-24 2:13 ` Martin K. Petersen
1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2021-07-24 2:13 UTC (permalink / raw)
To: lyl2019, linux-scsi, ketan.mukadam, Mike Christie,
james.bottomley, subbu.seetharaman, jitendra.bhivare
Cc: Martin K . Petersen
On Thu, 1 Jul 2021 14:08:40 -0500, Mike Christie wrote:
> This fixes a bug found by Lv Yunlong where because beiscsi_exec_nemb_cmd
> frees memory for the be_dma_mem cmd, we can access freed memory when
> beiscsi_if_clr_ip/beiscsi_if_set_ip's call to beiscsi_exec_nemb_cmd fails
> and we access the freed req. This fixes the issue by having the caller
> free the cmd's memory.
Applied to 5.15/scsi-queue, thanks!
[1/1] scsi: be2iscsi: fix use after free during IP updates
https://git.kernel.org/mkp/scsi/c/7b0ddc134608
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-24 2:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 19:08 [PATCH 1/1] scsi: be2iscsi: fix use after free during IP updates Mike Christie
2021-07-19 1:19 ` Martin K. Petersen
2021-07-24 2:13 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).