On 7/16/2022 1:15 AM, John Weston wrote:
Hi,
I have been working on the dpdk memif driver and encountered
the following issues. Please consider these patches for review.
Firstly, the zero copy code in drivers/net/memif/rte_eth_memif.c can
readilly overflow the ring buffer for mbufs. This patch adjusts the
n_slots used in rte_poktmbuf_alloc_bulk to ensure it does not run off
the end of the buffers list.
[email protected] <mailto:[email protected]>@@ -524,14 +1196,23 @@ refill:
*/
head = __atomic_load_n(&ring->head, __ATOMIC_RELAXED);
n_slots = ring_size - head + mq->last_tail;
-
- if (n_slots < 32)
- goto no_free_mbufs;
+// CTAM - critical BUG FIX !
+#if 1
+// only work within mask so alloc_bulk does not overrun the buffers array
+if ((head&mask) + n_slots > ring_size)
+{
+n_slots = ring_size - (head&mask);
+}
+if (n_slots <=0)
+ goto no_free_mbufs;
+#else
+if (n_slots < 32)
+ goto no_free_mbufs;
+#endif
ret = rte_pktmbuf_alloc_bulk(mq->mempool, &mq->buffers[head & mask],
n_slots);
if (unlikely(ret < 0))
goto no_free_mbufs;
-
while (n_slots--) {
s0 = head++ & mask;
if (n_slots > 0)
Secondly, a segfault can occur on the stats routine on initialisation
due to null pointer dereferencing.
@@ -1404,10 +2167,14 @@ memif_stats_get(struct rte_eth_dev *dev, struct
rte_eth_stats *stats)
/* RX stats */
for (i = 0; i < nq; i++) {
mq = dev->data->rx_queues[i];
- stats->q_ipackets[i] = mq->n_pkts;
- stats->q_ibytes[i] = mq->n_bytes;
- stats->ipackets += mq->n_pkts;
- stats->ibytes += mq->n_bytes;
+// CTAM test this, as it may not yet initialised!
+if (mq)
+{
+ stats->q_ipackets[i] = mq->n_pkts;
+ stats->q_ibytes[i] = mq->n_bytes;
+ stats->ipackets += mq->n_pkts;
+ stats->ibytes += mq->n_bytes;
+}
}
tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_s2c_rings :
This can occur in several places in the stats code, and null dereference
guards are needed in all locations.
Hope this helps someone else.
John
Hi John,
Thanks for reporting. Cc'ed memif maintainer.
Meanwhile, can you please submit a defect to bugzilla [1], to be sure
issue is recorded properly?
Also if you want to contribute the fix yourself, please check the
contribution guide:
https://doc.dpdk.org/guides/contributing/patches.html
[1]
https://bugs.dpdk.org/
