* [PATCH 1/7] maintainers: update for memif driver
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 2/7] net/memif: fix issues in statistics Stephen Hemminger
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Sriram Yagnaraman, Thomas Monjalon
Sriram has volunteered to be memif maintainer.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8c50c52933..56e20d993d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1160,6 +1160,7 @@ F: drivers/net/softnic/
F: doc/guides/nics/softnic.rst
Memif PMD
+M: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
M: Jakub Grajciar <jgrajcia@cisco.com>
F: drivers/net/memif/
F: doc/guides/nics/memif.rst
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/7] net/memif: fix issues in statistics
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
2026-09-22 19:40 ` [PATCH 1/7] maintainers: update for memif driver Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 3/7] net/memif: validate peer descriptors Stephen Hemminger
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Sriram Yagnaraman, Jakub Grajciar,
Ferruh Yigit
The statistics structure is already cleared in ethdev before
calling PMD.
Statistics from all queues should be counted against overall
packets; the constant RTE_ETHDEV_QUEUE_STAT_CNTRS is upper
bound on the array of queue stats.
The counters were also keyed off the negotiated ring counts in
pmd->run, which memif_disconnect() clears, so every counter read
back as zero once the peer went away. Iterate over the configured
queue counts instead, in both stats_get and stats_reset, so the
totals survive a disconnect.
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/memif/rte_eth_memif.c | 44 +++++++++++--------------------
1 file changed, 15 insertions(+), 29 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 5d153c3a5a..89796d3ed5 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1595,25 +1595,18 @@ static int
memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
struct eth_queue_stats *qstats)
{
- struct pmd_internals *pmd = dev->data->dev_private;
struct memif_queue *mq;
- int i;
- uint8_t tmp, nq;
-
- stats->ipackets = 0;
- stats->ibytes = 0;
- stats->opackets = 0;
- stats->obytes = 0;
+ unsigned int i;
- tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_s2c_rings :
- pmd->run.num_c2s_rings;
- nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
- RTE_ETHDEV_QUEUE_STAT_CNTRS;
+ /*
+ * Use the configured queue counts, not pmd->run, which memif_disconnect()
+ * clears; otherwise all counters would read zero once the peer is gone.
+ */
/* RX stats */
- for (i = 0; i < nq; i++) {
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
mq = dev->data->rx_queues[i];
- if (qstats != NULL) {
+ if (qstats != NULL && i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
qstats->q_ipackets[i] = mq->n_pkts;
qstats->q_ibytes[i] = mq->n_bytes;
}
@@ -1622,15 +1615,10 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
stats->ierrors += mq->n_err;
}
- tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_c2s_rings :
- pmd->run.num_s2c_rings;
- nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
- RTE_ETHDEV_QUEUE_STAT_CNTRS;
-
/* TX stats */
- for (i = 0; i < nq; i++) {
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
mq = dev->data->tx_queues[i];
- if (qstats != NULL) {
+ if (qstats != NULL && i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
qstats->q_opackets[i] = mq->n_pkts;
qstats->q_obytes[i] = mq->n_bytes;
}
@@ -1643,20 +1631,18 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
static int
memif_stats_reset(struct rte_eth_dev *dev)
{
- struct pmd_internals *pmd = dev->data->dev_private;
- int i;
struct memif_queue *mq;
+ unsigned int i;
- for (i = 0; i < pmd->run.num_c2s_rings; i++) {
- mq = (pmd->role == MEMIF_ROLE_CLIENT) ? dev->data->tx_queues[i] :
- dev->data->rx_queues[i];
+ /* Same as memif_stats_get(), pmd->run is cleared on disconnect. */
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ mq = dev->data->rx_queues[i];
mq->n_pkts = 0;
mq->n_bytes = 0;
mq->n_err = 0;
}
- for (i = 0; i < pmd->run.num_s2c_rings; i++) {
- mq = (pmd->role == MEMIF_ROLE_CLIENT) ? dev->data->rx_queues[i] :
- dev->data->tx_queues[i];
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ mq = dev->data->tx_queues[i];
mq->n_pkts = 0;
mq->n_bytes = 0;
mq->n_err = 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] net/memif: validate peer descriptors
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
2026-09-22 19:40 ` [PATCH 1/7] maintainers: update for memif driver Stephen Hemminger
2026-09-22 19:40 ` [PATCH 2/7] net/memif: fix issues in statistics Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 4/7] net/memif: validate control channel requests Stephen Hemminger
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, jgrajcia, Arthur Chan,
Sriram Yagnaraman, Thomas Monjalon, Anatoly Burakov, Ferruh Yigit
The buffer descriptors in the shared memory ring are writable
by the peer at any time, and a server can not trust its client.
Validation classifies a bad descriptor into the same categories
used by the VPP memif plugin. The descriptor length is 32 bits on
the wire, so it is validated at full width before being narrowed
for use; otherwise a length such as 0x10040 would be truncated to
0x40 and accepted.
A zero length buffer on a server to client ring is also rejected.
It makes no forward progress, so the transmit path would consume
ring slots without copying anything.
If server gets a bad request it is logged and reported as error.
Since buggy or hostile client is not useful, the connection
is aborted to avoid later problems.
Only the primary process owns the control channel, so a secondary
that sees a bad descriptor counts the error but can not tear the
connection down; that waits until the primary sees one itself.
Propagating it needs an mp message and is left for later.
Bugzilla ID: 2010
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Cc: jgrajcia@cisco.com
Reported-by: Arthur Chan <arthur.chan@adalogics.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
.mailmap | 1 +
drivers/net/memif/rte_eth_memif.c | 304 +++++++++++++++++++++++++++---
drivers/net/memif/rte_eth_memif.h | 1 +
3 files changed, 279 insertions(+), 27 deletions(-)
diff --git a/.mailmap b/.mailmap
index bbe3d013d2..a64a9d0549 100644
--- a/.mailmap
+++ b/.mailmap
@@ -159,6 +159,7 @@ Arshdeep Kaur <arshdeep.kaur@intel.com>
Artem V. Andreev <artem.andreev@oktetlabs.ru>
Artemii Morozov <artemii.morozov@arknetworks.am>
Artemy Kovalyov <artemyko@nvidia.com>
+Arthur Chan <arthur.chan@adalogics.com>
Artin Davari <artin.davari@broadcom.com>
Artur Rojek <ar@semihalf.com>
Artur Trybula <arturx.trybula@intel.com>
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 89796d3ed5..f7be4e4f4b 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -27,6 +27,8 @@
#include <rte_memory.h>
#include <rte_memzone.h>
#include <rte_eal_memconfig.h>
+#include <rte_stdatomic.h>
+#include <rte_alarm.h>
#include "rte_eth_memif.h"
#include "memif_socket.h"
@@ -245,10 +247,147 @@ memif_get_ring_from_queue(struct pmd_process_private *proc_private,
return (memif_ring_t *)((uint8_t *)r->addr + mq->ring_offset);
}
-static void *
-memif_get_buffer(struct pmd_process_private *proc_private, memif_desc_t *d)
+/*
+ * Result of validating a peer supplied descriptor.
+ * The names match the descriptor status codes used by the VPP memif plugin.
+ */
+enum memif_desc_status {
+ MEMIF_DESC_STATUS_OK = 0,
+ MEMIF_DESC_STATUS_ERR_BAD_REGION,
+ MEMIF_DESC_STATUS_ERR_REGION_OVERRUN,
+ MEMIF_DESC_STATUS_ERR_DATA_TOO_BIG,
+ MEMIF_DESC_STATUS_ERR_ZERO_LENGTH,
+};
+
+static const char * const memif_desc_status_str[] = {
+ [MEMIF_DESC_STATUS_OK] = "ok",
+ [MEMIF_DESC_STATUS_ERR_BAD_REGION] = "bad region",
+ [MEMIF_DESC_STATUS_ERR_REGION_OVERRUN] = "region overrun",
+ [MEMIF_DESC_STATUS_ERR_DATA_TOO_BIG] = "data too big",
+ [MEMIF_DESC_STATUS_ERR_ZERO_LENGTH] = "zero length",
+};
+
+/* Take a private copy of descriptor for validation. */
+static inline memif_desc_t
+memif_desc_read(const memif_desc_t *dp)
+{
+ memif_desc_t desc = *(const volatile memif_desc_t *)dp;
+
+ rte_compiler_barrier(); /* avoid TOCTOU issues */
+ return desc;
+}
+
+/*
+ * Validate a peer supplied descriptor.
+ * The region index and offset are controlled by the peer, so check that the
+ * [offset, offset + len) window the caller intends to access lies inside a
+ * valid shared region.
+ */
+static enum memif_desc_status
+memif_desc_is_valid(const struct pmd_process_private *proc_private,
+ const memif_desc_t *d, uint32_t len, uint32_t max_len,
+ uint8_t **data)
+{
+ const struct memif_region *region;
+ uint64_t start = d->offset;
+
+ if (unlikely(d->region >= proc_private->regions_num))
+ return MEMIF_DESC_STATUS_ERR_BAD_REGION;
+
+ region = proc_private->regions[d->region];
+ if (unlikely(region == NULL || region->addr == NULL))
+ return MEMIF_DESC_STATUS_ERR_BAD_REGION;
+
+ if (unlikely(start + len > region->region_size))
+ return MEMIF_DESC_STATUS_ERR_REGION_OVERRUN;
+
+ if (unlikely(len > max_len || len > UINT16_MAX))
+ return MEMIF_DESC_STATUS_ERR_DATA_TOO_BIG;
+
+ *data = (uint8_t *)region->addr + start;
+ return MEMIF_DESC_STATUS_OK;
+}
+
+/*
+ * Tear down a connection whose peer supplied an invalid descriptor.
+ * Runs on the control thread from an alarm when started in data path.
+ */
+static void
+memif_bad_desc_disconnect(void *arg)
{
- return ((uint8_t *)proc_private->regions[d->region]->addr + d->offset);
+ struct rte_eth_dev *dev = arg;
+ struct pmd_internals *pmd = dev->data->dev_private;
+
+ if (!rte_atomic_exchange_explicit(&pmd->bad_desc, false, rte_memory_order_relaxed))
+ return;
+
+ strlcpy(pmd->local_disc_string, "bad descriptor",
+ sizeof(pmd->local_disc_string));
+
+ rte_spinlock_lock(&pmd->cc_lock);
+ if (pmd->cc != NULL)
+ memif_msg_enq_disconnect(pmd->cc, pmd->local_disc_string, 0);
+ rte_spinlock_unlock(&pmd->cc_lock);
+
+ memif_disconnect(dev);
+}
+
+/*
+ * Report a peer supplied descriptor that failed validation.
+ *
+ * A bad descriptor means the peer is buggy or malicious, so the rings can no longer be trusted.
+ * The data path only latches the error and defers the teardown to the control thread.
+ */
+static void __rte_cold
+memif_desc_error(struct memif_queue *mq, const memif_desc_t *d, enum memif_desc_status status)
+{
+ struct rte_eth_dev *dev = &rte_eth_devices[mq->in_port];
+ struct pmd_internals *pmd = dev->data->dev_private;
+
+ ++mq->n_err;
+
+ /*
+ * Only the primary owns the control channel, so only it can tear the
+ * connection down. A secondary can just count the error here; the
+ * disconnect then waits until the primary sees a bad descriptor of
+ * its own. Telling the primary directly needs an mp message.
+ */
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return;
+
+ /* Report only the descriptor that broke the connection. */
+ if (rte_atomic_exchange_explicit(&pmd->bad_desc, true, rte_memory_order_relaxed))
+ return;
+
+ MIF_LOG(ERR, "Port %u disconnecting, bad descriptor from peer "
+ "(region %u offset %u length %u): %s",
+ mq->in_port, d->region, d->offset, d->length,
+ memif_desc_status_str[status]);
+
+ if (rte_eal_alarm_set(1, memif_bad_desc_disconnect, dev) < 0) {
+ MIF_LOG(ERR, "Port %u failed to schedule disconnect", mq->in_port);
+ rte_atomic_store_explicit(&pmd->bad_desc, false, rte_memory_order_relaxed);
+ }
+}
+
+/*
+ * Resolve a peer supplied descriptor to a buffer address,
+ * or NULL if the descriptor is invalid.
+ */
+static uint8_t *
+memif_get_buffer(const struct pmd_process_private *proc_private, struct memif_queue *mq,
+ const memif_desc_t *d, uint32_t len, uint32_t max_len)
+{
+ enum memif_desc_status status;
+ uint8_t *data = NULL;
+
+ status = memif_desc_is_valid(proc_private, d, len, max_len, &data);
+ if (unlikely(status != MEMIF_DESC_STATUS_OK)) {
+ memif_desc_error(mq, d, status);
+ return NULL;
+ }
+
+ return data;
}
/* Free mbufs received by server */
@@ -307,6 +446,8 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
uint16_t src_len, src_off, dst_len, dst_off, cp_len;
memif_ring_type_t type = mq->type;
memif_desc_t *d0;
+ memif_desc_t desc;
+ const uint8_t *src_data;
struct rte_mbuf *mbuf, *mbuf_head, *mbuf_tail;
uint64_t b;
ssize_t size __rte_unused;
@@ -362,22 +503,29 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
next_slot1:
mbuf->port = mq->in_port;
s0 = cur_slot & mask;
- d0 = &ring->desc[s0];
+ desc = memif_desc_read(&ring->desc[s0]);
- cp_len = d0->length;
+ /*
+ * One descriptor per mbuf, so length must fit the mbuf.
+ * Validate the full 32-bit peer length before narrowing it.
+ */
+ src_data = memif_get_buffer(proc_private, mq, &desc, desc.length,
+ mbuf_size);
+ if (unlikely(src_data == NULL))
+ goto discard1;
+ cp_len = desc.length;
rte_pktmbuf_data_len(mbuf) = cp_len;
rte_pktmbuf_pkt_len(mbuf) = cp_len;
if (mbuf != mbuf_head)
rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
- rte_memcpy(rte_pktmbuf_mtod(mbuf, void *),
- (uint8_t *)memif_get_buffer(proc_private, d0), cp_len);
+ rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), src_data, cp_len);
cur_slot++;
n_slots--;
- if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+ if (desc.flags & MEMIF_DESC_FLAG_NEXT) {
if (unlikely(n_slots == 0)) {
mq->n_err++;
rte_pktmbuf_free_bulk(mbufs + rx_pkts,
@@ -406,6 +554,25 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
*bufs++ = mbuf_head;
rx_pkts++;
n_rx_pkts++;
+ continue;
+
+discard1:
+ /* Skip the remainder of this descriptor chain and
+ * reuse mbuf_head for the next packet.
+ */
+ while (1) {
+ cur_slot++;
+ n_slots--;
+ if (n_slots == 0 || (desc.flags & MEMIF_DESC_FLAG_NEXT) == 0)
+ break;
+ desc = memif_desc_read(&ring->desc[cur_slot & mask]);
+ }
+
+ /* Free any segments already chained, then reset the
+ * head so it can be reused for the next packet.
+ */
+ rte_pktmbuf_free(mbuf_head->next);
+ rte_pktmbuf_reset(mbuf_head);
}
if (rx_pkts < MAX_PKT_BURST) {
@@ -426,11 +593,22 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
next_slot2:
s0 = cur_slot & mask;
- d0 = &ring->desc[s0];
+ desc = memif_desc_read(&ring->desc[s0]);
- src_len = d0->length;
src_off = 0;
+ /*
+ * Descriptor may span several mbufs, only bound by the region.
+ * Validate the full 32-bit peer length before narrowing it.
+ */
+ src_data = memif_get_buffer(proc_private, mq, &desc, desc.length,
+ UINT32_MAX);
+ if (unlikely(src_data == NULL)) {
+ rte_pktmbuf_free(mbuf_head);
+ goto discard2;
+ }
+ src_len = desc.length;
+
do {
dst_len = mbuf_size - dst_off;
if (dst_len == 0) {
@@ -457,10 +635,8 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
if (mbuf != mbuf_head)
rte_pktmbuf_pkt_len(mbuf_head) += cp_len;
- rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *,
- dst_off),
- (uint8_t *)memif_get_buffer(proc_private, d0) +
- src_off, cp_len);
+ rte_memcpy(rte_pktmbuf_mtod_offset(mbuf, void *, dst_off),
+ src_data + src_off, cp_len);
src_off += cp_len;
dst_off += cp_len;
@@ -470,7 +646,7 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
cur_slot++;
n_slots--;
- if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+ if (desc.flags & MEMIF_DESC_FLAG_NEXT) {
if (unlikely(n_slots == 0)) {
mq->n_err++;
rte_pktmbuf_free(mbuf_head);
@@ -482,6 +658,17 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
mq->n_bytes += rte_pktmbuf_pkt_len(mbuf_head);
*bufs++ = mbuf_head;
n_rx_pkts++;
+ continue;
+
+discard2:
+ /* Skip the remainder of this descriptor chain. */
+ while (1) {
+ cur_slot++;
+ n_slots--;
+ if (n_slots == 0 || (desc.flags & MEMIF_DESC_FLAG_NEXT) == 0)
+ break;
+ desc = memif_desc_read(&ring->desc[cur_slot & mask]);
+ }
}
}
@@ -657,9 +844,14 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
rte_eth_devices[mq->in_port].process_private;
memif_ring_t *ring = memif_get_ring_from_queue(proc_private, mq);
uint16_t slot, saved_slot, n_free, ring_size, mask, n_tx_pkts = 0;
+ /* Counted in n_tx_pkts to pass ownership, but never transmitted. */
+ uint16_t n_drop_pkts = 0;
uint16_t src_len, src_off, dst_len, dst_off, cp_len, nb_segs;
+ uint32_t len;
+ uint8_t *dst_data;
memif_ring_type_t type = mq->type;
memif_desc_t *d0;
+ memif_desc_t desc;
struct rte_mbuf *mbuf;
struct rte_mbuf *mbuf_head;
uint64_t a;
@@ -726,12 +918,24 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
next_in_chain1:
d0 = &ring->desc[slot & mask];
- d0->flags = 0;
+ desc = memif_desc_read(d0);
cp_len = rte_pktmbuf_data_len(mbuf);
- rte_memcpy((uint8_t *)memif_get_buffer(proc_private, d0),
- rte_pktmbuf_mtod(mbuf, void *), cp_len);
+ dst_data = memif_get_buffer(proc_private, mq, &desc, cp_len, cp_len);
+ if (unlikely(dst_data == NULL)) {
+ /*
+ * The descriptor is bad, so this packet can never be sent.
+ * Rewind the slots it used and count it as transmitted.
+ */
+ slot = saved_slot;
+ n_tx_pkts++;
+ n_drop_pkts++;
+ goto free_mbufs;
+ }
+
+ rte_memcpy(dst_data, rte_pktmbuf_mtod(mbuf, void *), cp_len);
+ d0->flags = 0;
d0->length = cp_len;
mq->n_bytes += cp_len;
slot++;
@@ -760,10 +964,25 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
saved_slot = slot;
d0 = &ring->desc[slot & mask];
+ desc = memif_desc_read(d0);
d0->flags = 0;
dst_off = 0;
- dst_len = (type == MEMIF_RING_C2S) ?
- pmd->run.pkt_buffer_size : d0->length;
+ /*
+ * On a S2C ring the buffer length is supplied by the peer,
+ * so validate it at full width before narrowing. A zero
+ * length buffer makes no progress, so reject it here.
+ */
+ len = (type == MEMIF_RING_C2S) ?
+ pmd->run.pkt_buffer_size : desc.length;
+ if (unlikely(len == 0)) {
+ memif_desc_error(mq, &desc, MEMIF_DESC_STATUS_ERR_ZERO_LENGTH);
+ goto drop_mbuf;
+ }
+
+ dst_data = memif_get_buffer(proc_private, mq, &desc, len, len);
+ if (unlikely(dst_data == NULL))
+ goto drop_mbuf;
+ dst_len = len;
next_in_chain2:
src_off = 0;
@@ -776,10 +995,25 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
n_free--;
d0->flags = MEMIF_DESC_FLAG_NEXT;
d0 = &ring->desc[slot & mask];
+ desc = memif_desc_read(d0);
d0->flags = 0;
dst_off = 0;
- dst_len = (type == MEMIF_RING_C2S) ?
- pmd->run.pkt_buffer_size : d0->length;
+ len = (type == MEMIF_RING_C2S) ?
+ pmd->run.pkt_buffer_size : desc.length;
+ if (unlikely(len == 0)) {
+ memif_desc_error(mq, &desc,
+ MEMIF_DESC_STATUS_ERR_ZERO_LENGTH);
+ slot = saved_slot;
+ goto drop_mbuf;
+ }
+
+ dst_data = memif_get_buffer(proc_private, mq,
+ &desc, len, len);
+ if (unlikely(dst_data == NULL)) {
+ slot = saved_slot;
+ goto drop_mbuf;
+ }
+ dst_len = len;
} else {
slot = saved_slot;
goto no_free_slots;
@@ -787,10 +1021,9 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
}
cp_len = RTE_MIN(dst_len, src_len);
- rte_memcpy((uint8_t *)memif_get_buffer(proc_private,
- d0) + dst_off,
- rte_pktmbuf_mtod_offset(mbuf, void *, src_off),
- cp_len);
+ rte_memcpy(dst_data + dst_off,
+ rte_pktmbuf_mtod_offset(mbuf, void *, src_off),
+ cp_len);
mq->n_bytes += cp_len;
src_off += cp_len;
@@ -811,6 +1044,13 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
n_free--;
rte_pktmbuf_free(mbuf_head);
}
+ goto no_free_slots;
+
+drop_mbuf:
+ /* The descriptor is bad, this packet can not be sent. */
+ n_tx_pkts++;
+ n_drop_pkts++;
+ rte_pktmbuf_free(mbuf_head);
}
no_free_slots:
@@ -830,7 +1070,11 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
}
}
- mq->n_pkts += n_tx_pkts;
+ /*
+ * Dropped packets are counted in n_tx_pkts so the caller does not
+ * free them again, but they were never put on the wire.
+ */
+ mq->n_pkts += n_tx_pkts - n_drop_pkts;
return n_tx_pkts;
}
@@ -1412,8 +1656,13 @@ memif_dev_start(struct rte_eth_dev *dev)
static int
memif_dev_stop(struct rte_eth_dev *dev)
{
+ struct pmd_internals *pmd = dev->data->dev_private;
uint16_t i;
+ /* Drop any deferred bad descriptor disconnect, this supersedes it. */
+ rte_eal_alarm_cancel(memif_bad_desc_disconnect, dev);
+ rte_atomic_store_explicit(&pmd->bad_desc, false, rte_memory_order_relaxed);
+
memif_disconnect(dev);
for (i = 0; i < dev->data->nb_rx_queues; i++)
@@ -1624,6 +1873,7 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
}
stats->opackets += mq->n_pkts;
stats->obytes += mq->n_bytes;
+ stats->oerrors += mq->n_err;
}
return 0;
}
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 9c7a3a93f0..398a1f7baf 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -98,6 +98,7 @@ struct pmd_internals {
struct memif_control_channel *cc; /**< control channel */
rte_spinlock_t cc_lock; /**< control channel lock */
+ RTE_ATOMIC(bool) bad_desc; /**< peer supplied bad descriptor */
/* remote info */
char remote_name[RTE_DEV_NAME_MAX_LEN]; /**< remote app name */
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] net/memif: validate control channel requests
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
` (2 preceding siblings ...)
2026-09-22 19:40 ` [PATCH 3/7] net/memif: validate peer descriptors Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 5/7] net/memif: validate descriptor length in zero-copy mode Stephen Hemminger
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Arthur Chan, Sriram Yagnaraman,
Jakub Grajciar, Ferruh Yigit
The server can not trust the connecting client, validate
connection requests before acting on them.
Check the file passed with region request to make sure that the
claimed region size is backed by the file. Mapping beyond the end
of a file succeeds and only faults on access, so a client that
overstates the size can otherwise fault the server with SIGBUS.
This also underpins the data path descriptor checks, which bound
each descriptor against the region size.
Warn when the region is not sealed against shrinking, but do not
reject it. Requiring a seal would disconnect conforming clients:
VPP's hugepage backed regions can not be sealed, and a memfd
created without MFD_ALLOW_SEALING can not be distinguished from
one whose owner chose not to seal.
For an add ring request: require rings to be added in order exactly
once so the ring counts can not exceed the number of configured
queues, bound the ring size to the advertised maximum, reject
unsupported private headers, and check that the referenced region
exists and that the ring plus its descriptor table fits inside that
region at a naturally aligned offset.
A passed file descriptor is also closed when the message that
carried it does not take one. Only add region and add ring consume
an fd, so without this a client could attach one to any other
message type and exhaust the file descriptors of the server.
Bugzilla ID: 2011
Bugzilla ID: 2012
Bugzilla ID: 2013
Bugzilla ID: 2019
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org
Reported-by: Arthur Chan <arthur.chan@adalogics.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/memif_socket.c | 129 ++++++++++++++++++++++++++-----
1 file changed, 108 insertions(+), 21 deletions(-)
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 898ad75fa6..8231a936e5 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -7,6 +7,7 @@
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
+#include <sys/stat.h>
#include <sys/ioctl.h>
#include <errno.h>
@@ -262,6 +263,8 @@ memif_msg_receive_add_region(struct rte_eth_dev *dev, memif_msg_t *msg,
struct pmd_process_private *proc_private = dev->process_private;
memif_msg_add_region_t *ar = &msg->add_region;
struct memif_region *r;
+ struct stat st;
+ int seals;
if (fd < 0) {
memif_msg_enq_disconnect(pmd->cc, "Missing region fd", 0);
@@ -272,13 +275,32 @@ memif_msg_receive_add_region(struct rte_eth_dev *dev, memif_msg_t *msg,
ar->index != proc_private->regions_num ||
proc_private->regions[ar->index] != NULL) {
memif_msg_enq_disconnect(pmd->cc, "Invalid region index", 0);
- return -1;
+ goto error;
+ }
+
+ /* The client is not trusted to describe the region it shares. */
+ if (ar->size == 0 || fstat(fd, &st) < 0 || (uint64_t)st.st_size < ar->size) {
+ memif_msg_enq_disconnect(pmd->cc, "Invalid region size", 0);
+ goto error;
}
+ /*
+ * Prefer regions sealed against shrinking so the peer can not truncate
+ * the file after connect and fault the server. Sealing is not supported
+ * on all fd types (for example hugetlbfs).
+ */
+ seals = fcntl(fd, F_GET_SEALS);
+ if (seals < 0)
+ MIF_LOG(INFO, "Port %u region %u fd does not support sealing",
+ dev->data->port_id, ar->index);
+ else if ((seals & F_SEAL_SHRINK) == 0)
+ MIF_LOG(NOTICE, "Port %u region %u fd is not sealed against shrinking",
+ dev->data->port_id, ar->index);
+
r = rte_zmalloc("region", sizeof(struct memif_region), 0);
if (r == NULL) {
memif_msg_enq_disconnect(pmd->cc, "Failed to alloc memif region.", 0);
- return -ENOMEM;
+ goto error;
}
r->fd = fd;
@@ -289,46 +311,92 @@ memif_msg_receive_add_region(struct rte_eth_dev *dev, memif_msg_t *msg,
proc_private->regions_num++;
return 0;
+error:
+ close(fd);
+ return -1;
}
static int
memif_msg_receive_add_ring(struct rte_eth_dev *dev, memif_msg_t *msg, int fd)
{
struct pmd_internals *pmd = dev->data->dev_private;
+ struct pmd_process_private *proc_private = dev->process_private;
memif_msg_add_ring_t *ar = &msg->add_ring;
+ const struct memif_region *r;
struct memif_queue *mq;
+ uint64_t ring_size;
if (fd < 0) {
memif_msg_enq_disconnect(pmd->cc, "Missing interrupt fd", 0);
return -1;
}
- /* check if we have enough queues */
+ /* rings must be added in order, exactly once */
if (ar->flags & MEMIF_MSG_ADD_RING_FLAG_C2S) {
- if (ar->index >= pmd->cfg.num_c2s_rings) {
+ if (ar->index >= pmd->cfg.num_c2s_rings ||
+ ar->index != pmd->run.num_c2s_rings) {
memif_msg_enq_disconnect(pmd->cc, "Invalid ring index", 0);
- return -1;
+ goto error;
}
- pmd->run.num_c2s_rings++;
} else {
- if (ar->index >= pmd->cfg.num_s2c_rings) {
+ if (ar->index >= pmd->cfg.num_s2c_rings ||
+ ar->index != pmd->run.num_s2c_rings) {
memif_msg_enq_disconnect(pmd->cc, "Invalid ring index", 0);
- return -1;
+ goto error;
}
- pmd->run.num_s2c_rings++;
+ }
+
+ if (ar->log2_ring_size > ETH_MEMIF_MAX_LOG2_RING_SIZE) {
+ memif_msg_enq_disconnect(pmd->cc, "Invalid ring size", 0);
+ goto error;
+ }
+
+ /* private headers are not supported */
+ if (ar->private_hdr_size != 0) {
+ memif_msg_enq_disconnect(pmd->cc, "Unsupported private header", 0);
+ goto error;
+ }
+
+ if (ar->region >= proc_private->regions_num ||
+ proc_private->regions[ar->region] == NULL) {
+ memif_msg_enq_disconnect(pmd->cc, "Invalid region index", 0);
+ goto error;
+ }
+
+ /*
+ * The ring and its descriptors must lie inside the region.
+ * Require natural alignment of the ring for atomic load/store.
+ * Existing DPDK and VPP put it on cache line boundary.
+ */
+ r = proc_private->regions[ar->region];
+ ring_size = sizeof(memif_ring_t) +
+ sizeof(memif_desc_t) * ((uint64_t)1 << ar->log2_ring_size);
+ if ((ar->offset & (sizeof(uint64_t) - 1)) != 0 ||
+ ar->offset + ring_size > r->region_size) {
+ memif_msg_enq_disconnect(pmd->cc, "Invalid ring offset", 0);
+ goto error;
}
mq = (ar->flags & MEMIF_MSG_ADD_RING_FLAG_C2S) ?
dev->data->rx_queues[ar->index] : dev->data->tx_queues[ar->index];
+ /* Takes ownership of the fd, so nothing to close after this point. */
if (rte_intr_fd_set(mq->intr_handle, fd))
- return -1;
+ goto error;
+
+ if (ar->flags & MEMIF_MSG_ADD_RING_FLAG_C2S)
+ pmd->run.num_c2s_rings++;
+ else
+ pmd->run.num_s2c_rings++;
mq->log2_ring_size = ar->log2_ring_size;
mq->region = ar->region;
mq->ring_offset = ar->offset;
return 0;
+error:
+ close(fd);
+ return -1;
}
static int
@@ -658,17 +726,11 @@ memif_msg_receive(struct memif_control_channel *cc)
return -1;
size = recvmsg(rte_intr_fd_get(cc->intr_handle), &mh, 0);
- if (size != sizeof(memif_msg_t)) {
- MIF_LOG(DEBUG, "Invalid message size = %zd", size);
- if (size > 0)
- /* 0 means end-of-file, negative size means error,
- * don't send further disconnect message in such cases.
- */
- memif_msg_enq_disconnect(cc, "Invalid message size", 0);
- return -1;
- }
- MIF_LOG(DEBUG, "Received msg type: %u.", msg.type);
+ /*
+ * Collect any passed fd first; a short message can still carry one,
+ * and it has to be closed on every path out of this function.
+ */
cmsg = CMSG_FIRSTHDR(&mh);
while (cmsg) {
if (cmsg->cmsg_level == SOL_SOCKET) {
@@ -680,10 +742,23 @@ memif_msg_receive(struct memif_control_channel *cc)
cmsg = CMSG_NXTHDR(&mh, cmsg);
}
+ if (size != sizeof(memif_msg_t)) {
+ MIF_LOG(DEBUG, "Invalid message size = %zd", size);
+ if (size > 0)
+ /* 0 means end-of-file, negative size means error,
+ * don't send further disconnect message in such cases.
+ */
+ memif_msg_enq_disconnect(cc, "Invalid message size", 0);
+ ret = -1;
+ goto exit;
+ }
+ MIF_LOG(DEBUG, "Received msg type: %u.", msg.type);
+
if (cc->dev == NULL && msg.type != MEMIF_MSG_TYPE_INIT) {
MIF_LOG(DEBUG, "Unexpected message.");
memif_msg_enq_disconnect(cc, "Unexpected message", 0);
- return -1;
+ ret = -1;
+ goto exit;
}
/* get device from hash data */
@@ -736,7 +811,9 @@ memif_msg_receive(struct memif_control_channel *cc)
goto exit;
break;
case MEMIF_MSG_TYPE_ADD_REGION:
+ /* The handler owns the fd from here, on success and on error. */
ret = memif_msg_receive_add_region(cc->dev, &msg, afd);
+ afd = -1;
if (ret < 0)
goto exit;
ret = memif_msg_enq_ack(cc->dev);
@@ -744,7 +821,9 @@ memif_msg_receive(struct memif_control_channel *cc)
goto exit;
break;
case MEMIF_MSG_TYPE_ADD_RING:
+ /* The handler owns the fd from here, on success and on error. */
ret = memif_msg_receive_add_ring(cc->dev, &msg, afd);
+ afd = -1;
if (ret < 0)
goto exit;
ret = memif_msg_enq_ack(cc->dev);
@@ -774,6 +853,14 @@ memif_msg_receive(struct memif_control_channel *cc)
}
exit:
+ /*
+ * A peer can attach an fd to any message, but only add region and
+ * add ring take one. Close the rest, otherwise a client could
+ * exhaust the file descriptors of the server.
+ */
+ if (afd >= 0)
+ close(afd);
+
return ret;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/7] net/memif: validate descriptor length in zero-copy mode
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
` (3 preceding siblings ...)
2026-09-22 19:40 ` [PATCH 4/7] net/memif: validate control channel requests Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 6/7] net/memif: add server/client connectivity test Stephen Hemminger
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Sriram Yagnaraman, Jakub Grajciar,
Ferruh Yigit
In zero-copy mode the receive buffers are the driver's own mbufs,
and the peer supplies the resulting length. That length needs to
be checked so that buggy/hostile peer doesn't crash server.
Validate the length against the buffer size advertised to the peer.
Read descriptor length once to avoid TOCTOU issues.
An invalid length means the peer is not honoring the contract on a
field whose buffer the driver owns, so nothing else in the ring can
be trusted. Drop the burst and disconnect, as is done for the other
invalid descriptor cases.
This also fixes the packet length of chained zero-copy segments.
memif_pktmbuf_chain() adds the tail data_len while it is still
zero, so a multi-segment packet previously carried only the length
of its first segment.
While here, fix a leak on the existing number-of-segments-overflow
path.
Bugzilla ID: 2018
Fixes: 43b815d88188 ("net/memif: support zero-copy slave")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
drivers/net/memif/rte_eth_memif.c | 49 ++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index f7be4e4f4b..7dbc80d3b0 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -711,7 +711,11 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
memif_ring_t *ring = memif_get_ring_from_queue(proc_private, mq);
uint16_t cur_slot, last_slot, n_slots, ring_size, mask, s0, head;
uint16_t n_rx_pkts = 0;
+ /* Buffer size advertised to the peer by the refill loop below. */
+ const uint16_t buf_size = rte_pktmbuf_data_room_size(mq->mempool) -
+ RTE_PKTMBUF_HEADROOM;
memif_desc_t *d0;
+ memif_desc_t desc;
struct rte_mbuf *mbuf, *mbuf_tail;
struct rte_mbuf *mbuf_head = NULL;
int ret;
@@ -763,14 +767,31 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
rte_prefetch0(&ring->desc[(cur_slot + 1) & mask]);
mbuf->port = mq->in_port;
- rte_pktmbuf_data_len(mbuf) = d0->length;
- rte_pktmbuf_pkt_len(mbuf) = rte_pktmbuf_data_len(mbuf);
+ desc = memif_desc_read(d0);
+
+ /* The peer only supplies the length here */
+ if (unlikely(desc.length > buf_size)) {
+ memif_desc_error(mq, &desc, MEMIF_DESC_STATUS_ERR_DATA_TOO_BIG);
+ /* Consume the slot before discarding */
+ cur_slot++;
+ n_slots--;
+ goto discard;
+ }
- mq->n_bytes += rte_pktmbuf_data_len(mbuf);
+ rte_pktmbuf_data_len(mbuf) = desc.length;
+ rte_pktmbuf_pkt_len(mbuf) = desc.length;
+ if (mbuf != mbuf_head)
+ rte_pktmbuf_pkt_len(mbuf_head) += desc.length;
+
+ mq->n_bytes += desc.length;
cur_slot++;
n_slots--;
- if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+ if (desc.flags & MEMIF_DESC_FLAG_NEXT) {
+ if (unlikely(n_slots == 0)) {
+ mq->n_err++;
+ goto discard;
+ }
s0 = cur_slot & mask;
d0 = &ring->desc[s0];
mbuf_tail = mbuf;
@@ -778,7 +799,8 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
ret = memif_pktmbuf_chain(mbuf_head, mbuf_tail, mbuf);
if (unlikely(ret < 0)) {
MIF_LOG(ERR, "number-of-segments-overflow");
- goto refill;
+ mq->n_err++;
+ goto discard;
}
goto next_slot;
}
@@ -788,6 +810,17 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
}
mq->last_tail = cur_slot;
+ goto refill;
+
+discard:
+ /*
+ * The peer is buggy or hostile, remaining descriptors cannot be trusted.
+ * Drop the partially built packet and the slots not yet consumed.
+ */
+ rte_pktmbuf_free(mbuf_head);
+ while (n_slots--)
+ rte_pktmbuf_free_seg(mq->buffers[cur_slot++ & mask]);
+ mq->last_tail = cur_slot;
/* Supply server with new buffers */
refill:
@@ -820,8 +853,9 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
d0->length = rte_pktmbuf_data_room_size(mq->mempool) -
RTE_PKTMBUF_HEADROOM;
d0->region = 1;
+ /* Use the constant, the peer can change d0->region at any time. */
d0->offset = rte_pktmbuf_mtod(mbuf, uint8_t *) -
- (uint8_t *)proc_private->regions[d0->region]->addr;
+ (uint8_t *)proc_private->regions[1]->addr;
}
no_free_mbufs:
/* The ring->head acts as a guard variable between Tx and Rx
@@ -1096,8 +1130,9 @@ memif_tx_one_zc(struct pmd_process_private *proc_private, struct memif_queue *mq
mq->n_bytes += rte_pktmbuf_data_len(mbuf);
/* FIXME: get region index */
d0->region = 1;
+ /* Use the constant, the peer can change d0->region at any time. */
d0->offset = rte_pktmbuf_mtod(mbuf, uint8_t *) -
- (uint8_t *)proc_private->regions[d0->region]->addr;
+ (uint8_t *)proc_private->regions[1]->addr;
d0->flags = 0;
/* check if buffer is chained */
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] net/memif: add server/client connectivity test
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
` (4 preceding siblings ...)
2026-09-22 19:40 ` [PATCH 5/7] net/memif: validate descriptor length in zero-copy mode Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-22 19:40 ` [PATCH 7/7] doc: clarify memif secret is not access control Stephen Hemminger
2026-09-24 11:24 ` [PATCH 0/7] net/memif: validate input from connecting peer Sriram Yagnaraman
7 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, Aaron Conole, Thomas Monjalon,
Sriram Yagnaraman, Jakub Grajciar
Add a black box test that launches a memif server and client as two
separate testpmd processes connected over a shared socket, and checks
that traffic flows in both directions with no descriptor validation
errors. This exercises the peer request validation over a real
connection without depending on any internal driver API, and provides
the two-instance harness that an adversarial (fuzzing) peer can be
driven from later.
Both instances forward what they receive, so the client to server and
server to client rings both carry traffic and the descriptors the
client writes are validated on both paths. The client seeds the first
burst with "start tx_first", which has to be issued as a command since
--tx-first can not be combined with interactive mode.
The test is run from the CI alongside test-null.sh. It skips (exit 77)
when the memif driver was not built, as in the CI jobs restricted with
-Denable_drivers=net/null, and when fewer than four cores are online
since the two primary processes need two cores each.
Bugzilla ID: 2016
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
.ci/linux-build.sh | 1 +
MAINTAINERS | 1 +
devtools/test-memif.sh | 169 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+)
create mode 100755 devtools/test-memif.sh
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e0b914a142..605265d7b6 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -178,6 +178,7 @@ if [ -z "$cross_file" ]; then
failed=
configure_coredump
devtools/test-null.sh || failed="true"
+ devtools/test-memif.sh || [ $? = 77 ] || failed="true"
catch_coredump
catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
check_traces
diff --git a/MAINTAINERS b/MAINTAINERS
index 56e20d993d..f8b117c57f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1165,6 +1165,7 @@ M: Jakub Grajciar <jgrajcia@cisco.com>
F: drivers/net/memif/
F: doc/guides/nics/memif.rst
F: doc/guides/nics/features/memif.ini
+F: devtools/test-memif.sh
Crypto Drivers
diff --git a/devtools/test-memif.sh b/devtools/test-memif.sh
new file mode 100755
index 0000000000..065ba08347
--- /dev/null
+++ b/devtools/test-memif.sh
@@ -0,0 +1,169 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2026 Stephen Hemminger
+
+# Run a memif server and client testpmd pair over a shared socket and
+# check that traffic flows in both directions without descriptor errors.
+# This is the black box connectivity and regression baseline for the
+# memif peer request validation; adversarial (fuzz) peers can be added
+# as a separate driver against the same two-instance setup.
+
+build=${1:-build} # first argument can be the build directory
+testpmd=$1 # or first argument can be the testpmd path
+srvcores=${2:-0-1} # cores for the server instance
+clicores=${3:-2-3} # cores for the client instance
+eal_options=$4
+testpmd_options=$5
+
+[ -f "$testpmd" ] && build=$(dirname $(dirname $testpmd))
+[ -f "$testpmd" ] || testpmd=$build/app/dpdk-testpmd
+[ -f "$testpmd" ] || testpmd=$build/app/testpmd
+if [ ! -f "$testpmd" ] ; then
+ echo 'ERROR: testpmd cannot be found' >&2
+ exit 1
+fi
+
+if ldd $testpmd | grep -q librte_ ; then
+ export LD_LIBRARY_PATH=$build/lib:$LD_LIBRARY_PATH
+ libs="-d $build/drivers"
+else
+ libs=
+fi
+
+# Skip (do not fail) where the driver was not built, for example a
+# build restricted with -Denable_drivers.
+config=$build/rte_build_config.h
+if [ -f "$config" ] && ! grep -q '^#define RTE_NET_MEMIF ' $config ; then
+ echo 'SKIP: memif driver is not built' >&2
+ exit 77 # automake convention for a skipped test
+fi
+
+# The server and client run as two separate primary processes, needing
+# two cores each. Skip (do not fail) where there are not enough cores.
+# Use nproc rather than getconf, it respects the affinity mask.
+ncpus=$(nproc 2>/dev/null || echo 1)
+if [ "$ncpus" -lt 4 ] ; then
+ echo "SKIP: memif test needs 4 cores, only $ncpus online" >&2
+ exit 77
+fi
+
+# Per run temporary socket and logs, cleaned up on exit.
+rundir=$(mktemp -d)
+sock=$rundir/memif.sock
+srvlog=$rundir/server.log
+clilog=$rundir/client.log
+srvpid=
+
+cleanup()
+{
+ [ -n "$srvpid" ] && kill $srvpid 2>/dev/null || true
+ rm -rf $rundir
+}
+trap cleanup EXIT
+
+common="--no-huge -m 64 --in-memory --file-prefix"
+
+# Use a pathname (non abstract) socket so its creation can be waited on
+# and so it is removed with the run directory on exit.
+vdev_srv="net_memif0,role=server,socket-abstract=no,socket=$sock"
+vdev_cli="net_memif0,role=client,socket-abstract=no,socket=$sock"
+
+# Both sides forward what they receive back to the peer, so that the
+# client to server (C2S) and server to client (S2C) rings both carry
+# traffic; descriptors supplied by the client are validated on both.
+# The server starts first, it listens on the socket.
+$testpmd $common memif_srv -l $srvcores $libs \
+ --vdev=$vdev_srv $eal_options -- \
+ --no-mlockall --total-num-mbufs=8192 \
+ --forward-mode=macswap --auto-start --stats-period 1 \
+ $testpmd_options > $srvlog 2>&1 &
+srvpid=$!
+
+# Wait for the server to create the listening socket (up to ~5s).
+tries=0
+while [ ! -S "$sock" ] ; do
+ tries=$((tries + 1))
+ if [ $tries -gt 50 ] ; then
+ echo 'ERROR: server socket not created' >&2
+ cat $srvlog >&2
+ exit 1
+ fi
+ sleep 0.1 2>/dev/null || sleep 1
+done
+
+# The client seeds the first burst with "start tx_first" and then
+# forwards what comes back, so the packets keep going round. It is
+# interactive (-i, not -ia) so that the burst is sent by that command
+# rather than by an auto-start with nothing to forward yet. Keep going
+# on a non-zero exit so that the logs below are still reported.
+clistatus=0
+(echo 'start tx_first' && sleep 3 && echo stop) | \
+$testpmd $common memif_cli -l $clicores $libs \
+ --vdev=$vdev_cli $eal_options -- \
+ --no-mlockall --total-num-mbufs=8192 \
+ --forward-mode=io --stats-period 1 \
+ $testpmd_options -i > $clilog 2>&1 || clistatus=$?
+
+# Let the server drain and print a final stats block, then stop it.
+sleep 1
+kill $srvpid 2>/dev/null || true
+wait $srvpid 2>/dev/null || true
+srvpid=
+
+fail=0
+
+if [ $clistatus -ne 0 ] ; then
+ echo "ERROR: client exited with status $clistatus" >&2
+ fail=1
+fi
+
+# testpmd prints a periodic statistics block per port and a final forward
+# statistics block. Match any line showing a non-zero count rather than
+# the last one, so that the result does not depend on when each instance
+# happened to be stopped.
+nonzero()
+{
+ grep "$2" "$1" | grep -q "$2"'[[:space:]]*[^0[:space:]]'
+}
+
+# Both rings must carry traffic: the client drives the client to server
+# ring and the server sends the same packets back over server to client.
+check_nonzero() # log pattern description
+{
+ if ! nonzero "$1" "$2" ; then
+ echo "ERROR: $3" >&2
+ fail=1
+ fi
+}
+
+check_nonzero $clilog 'TX-packets: ' 'client did not transmit any packet'
+check_nonzero $srvlog 'RX-packets: ' 'server did not receive any packet'
+check_nonzero $srvlog 'TX-packets: ' 'server did not transmit any packet'
+check_nonzero $clilog 'RX-packets: ' 'client did not receive any packet'
+
+# A conforming peer must not trip descriptor validation: no rx/tx errors
+# and no "bad descriptor" log line on either side.
+for log in $srvlog $clilog ; do
+ if nonzero $log 'RX-errors: ' ; then
+ echo "ERROR: RX-errors reported in $log" >&2
+ fail=1
+ fi
+ if nonzero $log 'TX-errors: ' ; then
+ echo "ERROR: TX-errors reported in $log" >&2
+ fail=1
+ fi
+ if grep -q 'bad descriptor' $log ; then
+ echo "ERROR: descriptor validation rejected a valid request in $log" >&2
+ fail=1
+ fi
+done
+
+if [ $fail -ne 0 ] ; then
+ echo '--- server log ---' >&2
+ cat $srvlog >&2
+ echo '--- client log ---' >&2
+ cat $clilog >&2
+ exit 1
+fi
+
+echo 'memif server/client forwarding: OK'
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] doc: clarify memif secret is not access control
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
` (5 preceding siblings ...)
2026-09-22 19:40 ` [PATCH 6/7] net/memif: add server/client connectivity test Stephen Hemminger
@ 2026-09-22 19:40 ` Stephen Hemminger
2026-09-24 15:44 ` Stephen Hemminger
2026-09-28 17:56 ` Stephen Hemminger
2026-09-24 11:24 ` [PATCH 0/7] net/memif: validate input from connecting peer Sriram Yagnaraman
7 siblings, 2 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-22 19:40 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Sriram Yagnaraman, Jakub Grajciar
The secret option was described as a security option, which invites
using it as one. It is sent in cleartext in the connection request,
and when passed as a device argument it is visible to other local
users in the process arguments.
Describe it as what it is, a check against connecting mismatched
interfaces, and document what actually restricts access to an
interface. By default the control socket is in the abstract
namespace and has no filesystem entry to own or permission. Only
with socket-abstract=no do file permissions and the owner-uid and
owner-gid options apply.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
doc/guides/nics/memif.rst | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index f8b629ab1f..5552d319d0 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -47,9 +47,27 @@ client.
"owner-uid=1000", "Set socket listener owner uid. Only relevant to server with socket-abstract=no", "unchanged", "uid_t"
"owner-gid=1000", "Set socket listener owner gid. Only relevant to server with socket-abstract=no", "unchanged", "gid_t"
"mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
- "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
+ "secret=abc123", "Optional identifier which, if specified, must be matched by peer", "", "string len 24"
"zero-copy=yes", "Enable/disable zero-copy client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"
+**Access control**
+
+Any process able to connect to the socket of a server interface is able to
+reach its shared memory rings, so what restricts access to that socket is
+the security boundary.
+
+By default the socket is in the abstract namespace (``socket-abstract=yes``).
+An abstract socket has no filesystem entry.
+Use a network namespace to restrict access to such an interface.
+
+With ``socket-abstract=no`` the socket is a filesystem object and normal
+file permissions apply, together with the ``owner-uid`` and ``owner-gid``
+options.
+
+The ``secret`` option is *not* an access control mechanism.
+It only guards against connecting mismatched interfaces by mistake,
+for example where several interfaces share one socket.
+
**Connection establishment**
In order to create memif connection, two memif interfaces, each in separate
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 7/7] doc: clarify memif secret is not access control
2026-09-22 19:40 ` [PATCH 7/7] doc: clarify memif secret is not access control Stephen Hemminger
@ 2026-09-24 15:44 ` Stephen Hemminger
2026-09-28 17:56 ` Stephen Hemminger
1 sibling, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-24 15:44 UTC (permalink / raw)
To: dev; +Cc: Sriram Yagnaraman, Jakub Grajciar
On Tue, 22 Sep 2026 12:40:58 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> The secret option was described as a security option, which invites
> using it as one. It is sent in cleartext in the connection request,
> and when passed as a device argument it is visible to other local
> users in the process arguments.
>
> Describe it as what it is, a check against connecting mismatched
> interfaces, and document what actually restricts access to an
> interface. By default the control socket is in the abstract
> namespace and has no filesystem entry to own or permission. Only
> with socket-abstract=no do file permissions and the owner-uid and
> owner-gid options apply.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> ---
Recheck-request: aws-unit-testing
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 7/7] doc: clarify memif secret is not access control
2026-09-22 19:40 ` [PATCH 7/7] doc: clarify memif secret is not access control Stephen Hemminger
2026-09-24 15:44 ` Stephen Hemminger
@ 2026-09-28 17:56 ` Stephen Hemminger
1 sibling, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-28 17:56 UTC (permalink / raw)
To: dev; +Cc: Sriram Yagnaraman, Jakub Grajciar
On Tue, 22 Sep 2026 12:40:58 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> The secret option was described as a security option, which invites
> using it as one. It is sent in cleartext in the connection request,
> and when passed as a device argument it is visible to other local
> users in the process arguments.
>
> Describe it as what it is, a check against connecting mismatched
> interfaces, and document what actually restricts access to an
> interface. By default the control socket is in the abstract
> namespace and has no filesystem entry to own or permission. Only
> with socket-abstract=no do file permissions and the owner-uid and
> owner-gid options apply.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Tested-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> ---
Recheck-request: aws-unit-testing
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 0/7] net/memif: validate input from connecting peer
2026-09-22 19:40 [PATCH 0/7] net/memif: validate input from connecting peer Stephen Hemminger
` (6 preceding siblings ...)
2026-09-22 19:40 ` [PATCH 7/7] doc: clarify memif secret is not access control Stephen Hemminger
@ 2026-09-24 11:24 ` Sriram Yagnaraman
2026-09-29 15:42 ` Stephen Hemminger
7 siblings, 1 reply; 12+ messages in thread
From: Sriram Yagnaraman @ 2026-09-24 11:24 UTC (permalink / raw)
To: Stephen Hemminger, dev@dpdk.org; +Cc: Arthur Chan
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, 22 September 2026 21:41
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Arthur Chan
> <arthur.chan@adalogics.com>
> Subject: [PATCH 0/7] net/memif: validate input from connecting peer
>
> The PMD has effectively been unmaintained for some time, which is why
> these have sat. A call for a new maintainer went out and Sriram Yagnaraman
> has volunteered; he has reviewed and tested this series.
>
Thanks for the series, Stephen.
Acked-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] net/memif: validate input from connecting peer
2026-09-24 11:24 ` [PATCH 0/7] net/memif: validate input from connecting peer Sriram Yagnaraman
@ 2026-09-29 15:42 ` Stephen Hemminger
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2026-09-29 15:42 UTC (permalink / raw)
To: Sriram Yagnaraman; +Cc: dev@dpdk.org, Arthur Chan
On Thu, 24 Sep 2026 11:24:08 +0000
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Tuesday, 22 September 2026 21:41
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger <stephen@networkplumber.org>; Arthur Chan
> > <arthur.chan@adalogics.com>
> > Subject: [PATCH 0/7] net/memif: validate input from connecting peer
> >
> > The PMD has effectively been unmaintained for some time, which is why
> > these have sat. A call for a new maintainer went out and Sriram Yagnaraman
> > has volunteered; he has reviewed and tested this series.
> >
>
> Thanks for the series, Stephen.
> Acked-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
Applied to next-net. Had to resolve minor conflict on the statistics handler.
^ permalink raw reply [flat|nested] 12+ messages in thread