lkml.org 
[lkml]   [2020]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 078/137] net: qlogic: Fix error paths in ql_alloc_large_buffers()
    Date
    From: Ben Hutchings <ben@decadent.org.uk>

    [ Upstream commit cad46039e4c99812db067c8ac22a864960e7acc4 ]

    ql_alloc_large_buffers() has the usual RX buffer allocation
    loop where it allocates skbs and maps them for DMA. It also
    treats failure as a fatal error.

    There are (at least) three bugs in the error paths:

    1. ql_free_large_buffers() assumes that the lrg_buf[] entry for the
    first buffer that couldn't be allocated will have .skb == NULL.
    But the qla_buf[] array is not zero-initialised.

    2. ql_free_large_buffers() DMA-unmaps all skbs in lrg_buf[]. This is
    incorrect for the last allocated skb, if DMA mapping failed.

    3. Commit 1acb8f2a7a9f ("net: qlogic: Fix memory leak in
    ql_alloc_large_buffers") added a direct call to dev_kfree_skb_any()
    after the skb is recorded in lrg_buf[], so ql_free_large_buffers()
    will double-free it.

    The bugs are somewhat inter-twined, so fix them all at once:

    * Clear each entry in qla_buf[] before attempting to allocate
    an skb for it. This goes half-way to fixing bug 1.
    * Set the .skb field only after the skb is DMA-mapped. This
    fixes the rest.

    Fixes: 1357bfcf7106 ("qla3xxx: Dynamically size the rx buffer queue ...")
    Fixes: 0f8ab89e825f ("qla3xxx: Check return code from pci_map_single() ...")
    Fixes: 1acb8f2a7a9f ("net: qlogic: Fix memory leak in ql_alloc_large_buffers")
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/ethernet/qlogic/qla3xxx.c | 8 ++++----
    1 file changed, 4 insertions(+), 4 deletions(-)

    --- a/drivers/net/ethernet/qlogic/qla3xxx.c
    +++ b/drivers/net/ethernet/qlogic/qla3xxx.c
    @@ -2752,6 +2752,9 @@ static int ql_alloc_large_buffers(struct
    int err;

    for (i = 0; i < qdev->num_large_buffers; i++) {
    + lrg_buf_cb = &qdev->lrg_buf[i];
    + memset(lrg_buf_cb, 0, sizeof(struct ql_rcv_buf_cb));
    +
    skb = netdev_alloc_skb(qdev->ndev,
    qdev->lrg_buffer_len);
    if (unlikely(!skb)) {
    @@ -2762,11 +2765,7 @@ static int ql_alloc_large_buffers(struct
    ql_free_large_buffers(qdev);
    return -ENOMEM;
    } else {
    -
    - lrg_buf_cb = &qdev->lrg_buf[i];
    - memset(lrg_buf_cb, 0, sizeof(struct ql_rcv_buf_cb));
    lrg_buf_cb->index = i;
    - lrg_buf_cb->skb = skb;
    /*
    * We save some space to copy the ethhdr from first
    * buffer
    @@ -2788,6 +2787,7 @@ static int ql_alloc_large_buffers(struct
    return -ENOMEM;
    }

    + lrg_buf_cb->skb = skb;
    dma_unmap_addr_set(lrg_buf_cb, mapaddr, map);
    dma_unmap_len_set(lrg_buf_cb, maplen,
    qdev->lrg_buffer_len -

    \
     
     \ /
      Last update: 2020-01-02 23:41    [W:4.127 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site