net/memif: validate input from connecting peer

A fuzzing effort by Arthur Chan turned up a number of ways for a memif client to upset a memif server. Taken together they are not isolated bugs so much as one systematic gap, which this series addresses.

memif is asymmetric. On connect the server hands its region file descriptors to the peer, so a client can reasonably trust the server it connected to, but the server cannot trust the client. The shared rings and their descriptors stay writable by the peer for the life of the connection, and control channel messages arrive before anything has been validated. The driver was written as if both ends were cooperative: it turned client-supplied region sizes, ring geometry and buffer descriptors into pointers and lengths without checking them. That is what the fuzzer found, from several directions.

Patches 3 to 5 do the validation (Bugzilla 2010, 2011, 2012, 2013, 2018, 2019) and are marked for stable, as is the statistics fix in patch 2. Patch 6 adds a two-process testpmd connectivity test, which is also the harness an adversarial peer can be driven from. Patches 1, 2 and 7 are unrelated to the fuzzing and bundled because they touch the same driver.

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.

Interoperability
----------------

These checks were written against the VPP plugin and libmemif so that a hardened DPDK server does not disconnect conforming clients. Two are deliberately looser than they could be:

- A missing F_SEAL_SHRINK on a region is a warning, not a rejection. libmemif seals region 0, but hugepage-backed regions cannot be sealed, and a memfd created without MFD_ALLOW_SEALING is indistinguishable from one whose owner chose not to seal. The fstat size check covers the same ground without breaking those clients.

- Ring offsets are only required to be 8-byte aligned, though implementations use 64 in practice, to avoid rejecting small-ring setups.

If anyone knows of a stricter guarantee that can be relied on here, I would rather tighten these.

Not fixed here
--------------

Some reports need agreement on the wire protocol rather than a local fix:

- Bugzilla 2014: hello advertises the driver-wide maximum ring count rather than the device's configured count. The obvious one-line fix does not work: hello is built from the control channel alone, and the device is not associated with it until init, which arrives after hello has been sent. This means changing when ring counts are negotiated.

- Bugzilla 2015: ring semantics differ from the VPP implementation.

- Bugzilla 2017: the trust model and the normative requirements for a conforming client are not written down anywhere. That amounts to specifying the protocol, better done with a maintainer to agree it.

Three more pre-existing ways for a client to upset a server turned up while writing this. All are structural rather than local, so they are listed here rather than tacked onto the end of the series:

- memif_disconnect() unmaps the regions while the data path may still be inside a burst function. The burst functions test the connected flag only on entry, so a client that disconnects under load can already fault the server. Fixing it means waiting for lcores to leave the burst functions, or deferring the unmap to a quiescent point.

- Control messages are dispatched on type alone, with no check of role or connection state. A client that sends init then hello makes the server run the client-only setup path. This wants an accept table by role and state in memif_msg_receive().

- Queues the client does not add keep the ring geometry from the previous connection: disconnect does not reset it and connect only walks the negotiated ring counts. An offset validated against the old region can then be used against a new, smaller one. Ties into 2014.

One limitation within the series: a secondary process that sees a bad descriptor counts the error but cannot tear the connection down, since only the primary owns the control channel. That needs an mp message.

Stephen Hemminger (7):
maintainers: update for memif driver net/memif: fix issues in statistics
net/memif: validate peer descriptors net/memif: validate control channel requests net/memif: validate descriptor length in zero-copy mode net/memif: add server/client connectivity test
doc: clarify memif secret is not access control

 .ci/linux-build.sh                |   1 +
 .mailmap                          |   1 +
 MAINTAINERS                       |   2 +
 devtools/test-memif.sh            | 169 +++++++++++++
 doc/guides/nics/memif.rst         |  20 +-
 drivers/net/memif/memif_socket.c  | 129 ++++++++--
 drivers/net/memif/rte_eth_memif.c | 397 +++++++++++++++++++++++++-----
 drivers/net/memif/rte_eth_memif.h |   1 +
 8 files changed, 635 insertions(+), 85 deletions(-)
 create mode 100755 devtools/test-memif.sh

2.53.0