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@xxxxxxxxxxxxxxxx> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx> --- 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