Skip to content

Commit cad4603

Browse files
bwhacksdavem330
authored andcommitted
net: qlogic: Fix error paths in ql_alloc_large_buffers()
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 1acb8f2 ("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: 1357bfc ("qla3xxx: Dynamically size the rx buffer queue ...") Fixes: 0f8ab89 ("qla3xxx: Check return code from pci_map_single() ...") Fixes: 1acb8f2 ("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>
1 parent 951c6db commit cad4603

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

drivers/net/ethernet/qlogic/qla3xxx.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2756,6 +2756,9 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
27562756
int err;
27572757

27582758
for (i = 0; i < qdev->num_large_buffers; i++) {
2759+
lrg_buf_cb = &qdev->lrg_buf[i];
2760+
memset(lrg_buf_cb, 0, sizeof(struct ql_rcv_buf_cb));
2761+
27592762
skb = netdev_alloc_skb(qdev->ndev,
27602763
qdev->lrg_buffer_len);
27612764
if (unlikely(!skb)) {
@@ -2766,11 +2769,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
27662769
ql_free_large_buffers(qdev);
27672770
return -ENOMEM;
27682771
} else {
2769-
2770-
lrg_buf_cb = &qdev->lrg_buf[i];
2771-
memset(lrg_buf_cb, 0, sizeof(struct ql_rcv_buf_cb));
27722772
lrg_buf_cb->index = i;
2773-
lrg_buf_cb->skb = skb;
27742773
/*
27752774
* We save some space to copy the ethhdr from first
27762775
* buffer
@@ -2792,6 +2791,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter *qdev)
27922791
return -ENOMEM;
27932792
}
27942793

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

0 commit comments

Comments
 (0)