From: John Weston <john.weston@senetas.com>
To: dev@dpdk.org
Subject: memif driver segfault memory corruption.
Date: Sat, 16 Jul 2022 10:15:48 +1000 [thread overview]
Message-ID: <CAGJEGKQ4vKATojn=_jCC4_Aw4vSbgqCDXQ6+fwdj5=hrOrhH-Q@mail.gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]
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.
dev@dpdk.org@@ -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
[-- Attachment #2: Type: text/html, Size: 18833 bytes --]
next reply other threads:[~2022-07-17 21:59 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-16 0:15 John Weston [this message]
2022-07-18 8:17 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGJEGKQ4vKATojn=_jCC4_Aw4vSbgqCDXQ6+fwdj5=hrOrhH-Q@mail.gmail.com' \
--to=john.weston@senetas.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).