| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240916060212.12393-1-amishin@t-argos.ru>
Date: Mon, 16 Sep 2024 09:02:12 +0300
From: Aleksandr Mishin <amishin@...rgos.ru>
To: Veerasenareddy Burru <vburru@...vell.com>
CC: Aleksandr Mishin <amishin@...rgos.ru>, Sathesh Edara <sedara@...vell.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Abhijit
Ayarekar <aayarekar@...vell.com>, Satananda Burla <sburla@...vell.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<lvc-project@...uxtesting.org>
Subject: [PATCH net v2] octeon_ep: Add SKB allocation failures handling in __octep_oq_process_rx()
build_skb() returns NULL in case of a memory allocation failure so handle
it inside __octep_oq_process_rx() to avoid NULL pointer dereference.
__octep_oq_process_rx() is called during NAPI polling by the driver. If
skb allocation fails, keep on pulling packets out of the Rx DMA queue: we
shouldn't break the polling immediately and thus falsely indicate to the
octep_napi_poll() that the Rx pressure is going down. As there is no
associated skb in this case, don't process the packets and don't push them
up the network stack - they are skipped.
The common code with skb and some index manipulations is extracted to make
the fix more readable and avoid code duplication. Also helper function is
implemented to unmmap/flush all the fragment buffers used by the dropped
packet. 'alloc_failures' counter is incremented to mark the skb allocation
error in driver statistics.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Suggested-by: Paolo Abeni <pabeni@...hat.com>
Signed-off-by: Aleksandr Mishin <amishin@...rgos.ru>
---
A similar situation is present in the __octep_vf_oq_process_rx() of the
Octeon VF driver. First we want to try the fix on __octep_oq_process_rx().
There are some doubts about increasing the 'rx_bytes'. On the one hand,
the data has not been processed, therefore, the counter does not need to
be increased. On the other hand, this counter is used to estimate the
bandwidth at the card's input.
In octeon_droq_fast_process_packet() from the Liquidio driver in
'droq->stats.bytes_received += total_len' everything that was received
from the device is considered.
/* Output Queue statistics. Each output queue has four stats fields. */
struct octep_oq_stats {
/* Number of packets received from the Device. */
u64 packets;
/* Number of bytes received from the Device. */
u64 bytes;
/* Number of times failed to allocate buffers. */
u64 alloc_failures;
};
Compile tested only.
v2:
- Implement helper instead of adding multiple checks for '!skb' and
remove 'rx_bytes' increasing in case of packet dropping as suggested
by Paolo
(https://lore.kernel.org/all/ba514498-3706-413b-a09f-f577861eef28@redhat.com/)
v1: https://lore.kernel.org/all/20240906063907.9591-1-amishin@t-argos.ru/
.../net/ethernet/marvell/octeon_ep/octep_rx.c | 80 +++++++++++++++----
1 file changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
index 4746a6b258f0..6b665263b9be 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
@@ -336,6 +336,51 @@ static int octep_oq_check_hw_for_pkts(struct octep_device *oct,
return new_pkts;
}
+/**
+ * octep_oq_drop_rx() - Free the resources associated with a packet.
+ *
+ * @oq: Octeon Rx queue data structure.
+ * @buff_info: Current packet buffer info.
+ * @read_idx: Current packet index in the ring.
+ * @desc_used: Current packet descriptor number.
+ *
+ */
+static void octep_oq_drop_rx(struct octep_oq *oq,
+ struct octep_rx_buffer *buff_info,
+ u32 *read_idx, u32 *desc_used)
+{
+ dma_unmap_page(oq->dev, oq->desc_ring[*read_idx].buffer_ptr,
+ PAGE_SIZE, DMA_FROM_DEVICE);
+ buff_info->page = NULL;
+ (*read_idx)++;
+ (*desc_used)++;
+ if (*read_idx == oq->max_count)
+ *read_idx = 0;
+
+ if (buff_info->len > oq->max_single_buffer_size) {
+ u16 data_len;
+ /* Head fragment includes response header(s);
+ * subsequent fragments contains only data.
+ */
+ data_len = buff_info->len - oq->max_single_buffer_size;
+ while (data_len) {
+ dma_unmap_page(oq->dev, oq->desc_ring[*read_idx].buffer_ptr,
+ PAGE_SIZE, DMA_FROM_DEVICE);
+ buff_info = (struct octep_rx_buffer *)
+ &oq->buff_info[*read_idx];
+ if (data_len < oq->buffer_size)
+ data_len = 0;
+ else
+ data_len -= oq->buffer_size;
+ buff_info->page = NULL;
+ (*read_idx)++;
+ (*desc_used)++;
+ if (*read_idx == oq->max_count)
+ *read_idx = 0;
+ }
+ }
+}
+
/**
* __octep_oq_process_rx() - Process hardware Rx queue and push to stack.
*
@@ -367,10 +412,7 @@ static int __octep_oq_process_rx(struct octep_device *oct,
desc_used = 0;
for (pkt = 0; pkt < pkts_to_process; pkt++) {
buff_info = (struct octep_rx_buffer *)&oq->buff_info[read_idx];
- dma_unmap_page(oq->dev, oq->desc_ring[read_idx].buffer_ptr,
- PAGE_SIZE, DMA_FROM_DEVICE);
resp_hw = page_address(buff_info->page);
- buff_info->page = NULL;
/* Swap the length field that is in Big-Endian to CPU */
buff_info->len = be64_to_cpu(resp_hw->length);
@@ -394,31 +436,37 @@ static int __octep_oq_process_rx(struct octep_device *oct,
data_offset = OCTEP_OQ_RESP_HW_SIZE;
rx_ol_flags = 0;
}
+
+ skb = build_skb((void *)resp_hw, PAGE_SIZE);
+ if (!skb) {
+ octep_oq_drop_rx(oq, buff_info,
+ &read_idx, &desc_used);
+ oq->stats.alloc_failures++;
+ continue;
+ }
+ skb_reserve(skb, data_offset);
+
+ dma_unmap_page(oq->dev, oq->desc_ring[read_idx].buffer_ptr,
+ PAGE_SIZE, DMA_FROM_DEVICE);
+ buff_info->page = NULL;
+
+ read_idx++;
+ desc_used++;
+ if (read_idx == oq->max_count)
+ read_idx = 0;
+
rx_bytes += buff_info->len;
if (buff_info->len <= oq->max_single_buffer_size) {
- skb = build_skb((void *)resp_hw, PAGE_SIZE);
- skb_reserve(skb, data_offset);
skb_put(skb, buff_info->len);
- read_idx++;
- desc_used++;
- if (read_idx == oq->max_count)
- read_idx = 0;
} else {
struct skb_shared_info *shinfo;
u16 data_len;
- skb = build_skb((void *)resp_hw, PAGE_SIZE);
- skb_reserve(skb, data_offset);
/* Head fragment includes response header(s);
* subsequent fragments contains only data.
*/
skb_put(skb, oq->max_single_buffer_size);
- read_idx++;
- desc_used++;
- if (read_idx == oq->max_count)
- read_idx = 0;
-
shinfo = skb_shinfo(skb);
data_len = buff_info->len - oq->max_single_buffer_size;
while (data_len) {
--
2.30.2
Powered by blists - more mailing lists