lkml.org 
[lkml]   [2020]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.6 102/106] qed: Fix use after free in qed_chain_free
    Date
    From: Yuval Basson <ybason@marvell.com>

    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>

    ---
    drivers/net/ethernet/qlogic/qed/qed_dev.c | 38 ++++++++++++------------------
    include/linux/qed/qed_chain.h | 24 +++++++++++-------
    2 files changed, 31 insertions(+), 31 deletions(-)

    --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
    +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
    @@ -4691,26 +4691,20 @@ static void qed_chain_free_single(struct

    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;
    @@ -4720,9 +4714,9 @@ static void qed_chain_free_pbl(struct qe
    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)
    @@ -4823,19 +4817,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;
    @@ -4849,8 +4843,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;

    @@ -4869,7 +4862,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;
    }
    --- a/include/linux/qed/qed_chain.h
    +++ b/include/linux/qed/qed_chain.h
    @@ -97,6 +97,11 @@ struct qed_chain_u32 {
    u32 cons_idx;
    };

    +struct addr_tbl_entry {
    + void *virt_addr;
    + dma_addr_t dma_map;
    +};
    +
    struct qed_chain {
    /* fastpath portion of the chain - required for commands such
    * as produce / consume.
    @@ -107,10 +112,11 @@ struct qed_chain {

    /* Fastpath portions of the PBL [if exists] */
    struct {
    - /* Table for keeping the virtual addresses of the chain pages,
    - * respectively to the physical addresses in the pbl table.
    + /* Table for keeping the virtual and physical addresses of the
    + * chain pages, respectively to the physical addresses
    + * in the pbl table.
    */
    - void **pp_virt_addr_tbl;
    + struct addr_tbl_entry *pp_addr_tbl;

    union {
    struct qed_chain_pbl_u16 u16;
    @@ -287,7 +293,7 @@ qed_chain_advance_page(struct qed_chain
    *(u32 *)page_to_inc = 0;
    page_index = *(u32 *)page_to_inc;
    }
    - *p_next_elem = p_chain->pbl.pp_virt_addr_tbl[page_index];
    + *p_next_elem = p_chain->pbl.pp_addr_tbl[page_index].virt_addr;
    }
    }

    @@ -537,7 +543,7 @@ static inline void qed_chain_init_params

    p_chain->pbl_sp.p_phys_table = 0;
    p_chain->pbl_sp.p_virt_table = NULL;
    - p_chain->pbl.pp_virt_addr_tbl = NULL;
    + p_chain->pbl.pp_addr_tbl = NULL;
    }

    /**
    @@ -575,11 +581,11 @@ static inline void qed_chain_init_mem(st
    static inline void qed_chain_init_pbl_mem(struct qed_chain *p_chain,
    void *p_virt_pbl,
    dma_addr_t p_phys_pbl,
    - void **pp_virt_addr_tbl)
    + struct addr_tbl_entry *pp_addr_tbl)
    {
    p_chain->pbl_sp.p_phys_table = p_phys_pbl;
    p_chain->pbl_sp.p_virt_table = p_virt_pbl;
    - p_chain->pbl.pp_virt_addr_tbl = pp_virt_addr_tbl;
    + p_chain->pbl.pp_addr_tbl = pp_addr_tbl;
    }

    /**
    @@ -644,7 +650,7 @@ static inline void *qed_chain_get_last_e
    break;
    case QED_CHAIN_MODE_PBL:
    last_page_idx = p_chain->page_cnt - 1;
    - p_virt_addr = p_chain->pbl.pp_virt_addr_tbl[last_page_idx];
    + p_virt_addr = p_chain->pbl.pp_addr_tbl[last_page_idx].virt_addr;
    break;
    }
    /* p_virt_addr points at this stage to the last page of the chain */
    @@ -716,7 +722,7 @@ static inline void qed_chain_pbl_zero_me
    page_cnt = qed_chain_get_page_cnt(p_chain);

    for (i = 0; i < page_cnt; i++)
    - memset(p_chain->pbl.pp_virt_addr_tbl[i], 0,
    + memset(p_chain->pbl.pp_addr_tbl[i].virt_addr, 0,
    QED_CHAIN_PAGE_SIZE);
    }


    \
     
     \ /
      Last update: 2020-05-01 15:46    [W:4.305 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site