summaryrefslogtreecommitdiff
path: root/drivers
diff options
context:
space:
mode:
authorYuval Basson <ybason@marvell.com>2020-03-29 20:32:49 +0300
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2020-05-02 08:49:01 +0200
commit93af898b251fd6386e874f8803661cece1e45657 (patch)
treea21631f56fbd23c52d6ff33325335a2055eca718 /drivers
parentf1610480602a475323f9c335540d47b3a95c62fd (diff)
qed: Fix use after free in qed_chain_free
commit 8063f761cd7c17fc1d0018728936e0c33a25388a upstream. The qed_chain data structure was modified in commit 1a4a69751f4d ("qed: Chain support for external PBL") to support receiving an external pbl (due to iWARP FW requirements). The pages pointed to by the pbl are allocated in qed_chain_alloc and their virtual address are stored in an virtual addresses array to enable accessing and freeing the data. The physical addresses however weren't stored and were accessed directly from the external-pbl during free. Destroy-qp flow, leads to freeing the external pbl before the chain is freed, when the chain is freed it tries accessing the already freed external pbl, leading to a use-after-free. Therefore we need to store the physical addresses in additional to the virtual addresses in a new data structure. Fixes: 1a4a69751f4d ("qed: Chain support for external PBL") Signed-off-by: Michal Kalderon <mkalderon@marvell.com> Signed-off-by: Yuval Bason <ybason@marvell.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/net/ethernet/qlogic/qed/qed_dev.c38
1 files changed, 16 insertions, 22 deletions
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index a1ebc2b1ca0b..0bf91df80d47 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -4648,26 +4648,20 @@ static void qed_chain_free_single(struct qed_dev *cdev,
static void qed_chain_free_pbl(struct qed_dev *cdev, struct qed_chain *p_chain)
{
- void **pp_virt_addr_tbl = p_chain->pbl.pp_virt_addr_tbl;
+ struct addr_tbl_entry *pp_addr_tbl = p_chain->pbl.pp_addr_tbl;
u32 page_cnt = p_chain->page_cnt, i, pbl_size;
- u8 *p_pbl_virt = p_chain->pbl_sp.p_virt_table;
- if (!pp_virt_addr_tbl)
+ if (!pp_addr_tbl)
return;
- if (!p_pbl_virt)
- goto out;
-
for (i = 0; i < page_cnt; i++) {
- if (!pp_virt_addr_tbl[i])
+ if (!pp_addr_tbl[i].virt_addr || !pp_addr_tbl[i].dma_map)
break;
dma_free_coherent(&cdev->pdev->dev,
QED_CHAIN_PAGE_SIZE,
- pp_virt_addr_tbl[i],
- *(dma_addr_t *)p_pbl_virt);
-
- p_pbl_virt += QED_CHAIN_PBL_ENTRY_SIZE;
+ pp_addr_tbl[i].virt_addr,
+ pp_addr_tbl[i].dma_map);
}
pbl_size = page_cnt * QED_CHAIN_PBL_ENTRY_SIZE;
@@ -4677,9 +4671,9 @@ static void qed_chain_free_pbl(struct qed_dev *cdev, struct qed_chain *p_chain)
pbl_size,
p_chain->pbl_sp.p_virt_table,
p_chain->pbl_sp.p_phys_table);
-out:
- vfree(p_chain->pbl.pp_virt_addr_tbl);
- p_chain->pbl.pp_virt_addr_tbl = NULL;
+
+ vfree(p_chain->pbl.pp_addr_tbl);
+ p_chain->pbl.pp_addr_tbl = NULL;
}
void qed_chain_free(struct qed_dev *cdev, struct qed_chain *p_chain)
@@ -4780,19 +4774,19 @@ qed_chain_alloc_pbl(struct qed_dev *cdev,
{
u32 page_cnt = p_chain->page_cnt, size, i;
dma_addr_t p_phys = 0, p_pbl_phys = 0;
- void **pp_virt_addr_tbl = NULL;
+ struct addr_tbl_entry *pp_addr_tbl;
u8 *p_pbl_virt = NULL;
void *p_virt = NULL;
- size = page_cnt * sizeof(*pp_virt_addr_tbl);
- pp_virt_addr_tbl = vzalloc(size);
- if (!pp_virt_addr_tbl)
+ size = page_cnt * sizeof(*pp_addr_tbl);
+ pp_addr_tbl = vzalloc(size);
+ if (!pp_addr_tbl)
return -ENOMEM;
/* The allocation of the PBL table is done with its full size, since it
* is expected to be successive.
* qed_chain_init_pbl_mem() is called even in a case of an allocation
- * failure, since pp_virt_addr_tbl was previously allocated, and it
+ * failure, since tbl was previously allocated, and it
* should be saved to allow its freeing during the error flow.
*/
size = page_cnt * QED_CHAIN_PBL_ENTRY_SIZE;
@@ -4806,8 +4800,7 @@ qed_chain_alloc_pbl(struct qed_dev *cdev,
p_chain->b_external_pbl = true;
}
- qed_chain_init_pbl_mem(p_chain, p_pbl_virt, p_pbl_phys,
- pp_virt_addr_tbl);
+ qed_chain_init_pbl_mem(p_chain, p_pbl_virt, p_pbl_phys, pp_addr_tbl);
if (!p_pbl_virt)
return -ENOMEM;
@@ -4826,7 +4819,8 @@ qed_chain_alloc_pbl(struct qed_dev *cdev,
/* Fill the PBL table with the physical address of the page */
*(dma_addr_t *)p_pbl_virt = p_phys;
/* Keep the virtual address of the page */
- p_chain->pbl.pp_virt_addr_tbl[i] = p_virt;
+ p_chain->pbl.pp_addr_tbl[i].virt_addr = p_virt;
+ p_chain->pbl.pp_addr_tbl[i].dma_map = p_phys;
p_pbl_virt += QED_CHAIN_PBL_ENTRY_SIZE;
}