Commit 06b71812 authored by Ben Hutchings's avatar Ben Hutchings Committed by Kleber Sacilotto de Souza

net: qlogic: Fix error paths in ql_alloc_large_buffers()

BugLink: https://bugs.launchpad.net/bugs/1858462

[ Upstream commit cad46039 ]

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 1acb8f2a ("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: 1357bfcf ("qla3xxx: Dynamically size the rx buffer queue ...")
Fixes: 0f8ab89e ("qla3xxx: Check return code from pci_map_single() ...")
Fixes: 1acb8f2a ("net: qlogic: Fix memory leak in ql_alloc_large_buffers")
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent f7b52956
...@@ -2752,6 +2752,9 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev) ...@@ -2752,6 +2752,9 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
int err; int err;
for (i = 0; i < qdev->num_large_buffers; i++) { 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, skb = netdev_alloc_skb(qdev->ndev,
qdev->lrg_buffer_len); qdev->lrg_buffer_len);
if (unlikely(!skb)) { if (unlikely(!skb)) {
...@@ -2762,11 +2765,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev) ...@@ -2762,11 +2765,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
ql_free_large_buffers(qdev); ql_free_large_buffers(qdev);
return -ENOMEM; return -ENOMEM;
} else { } 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->index = i;
lrg_buf_cb->skb = skb;
/* /*
* We save some space to copy the ethhdr from first * We save some space to copy the ethhdr from first
* buffer * buffer
...@@ -2788,6 +2787,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev) ...@@ -2788,6 +2787,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
return -ENOMEM; return -ENOMEM;
} }
lrg_buf_cb->skb = skb;
dma_unmap_addr_set(lrg_buf_cb, mapaddr, map); dma_unmap_addr_set(lrg_buf_cb, mapaddr, map);
dma_unmap_len_set(lrg_buf_cb, maplen, dma_unmap_len_set(lrg_buf_cb, maplen,
qdev->lrg_buffer_len - qdev->lrg_buffer_len -
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment