[DPDK/ethdev Bug 2011] memif: incorrect control message validation

[DPDK/ethdev Bug 2011] memif: incorrect control message validation

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=2011

            Bug ID: 2011
           Summary: memif: incorrect control message validation
           Product: DPDK
           Version: 26.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: major
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---
             Group: security

Long winded AI analysis

memif_msg_receive() in drivers/net/memif/memif_socket.c dispatches on
msg.type without checking that the message is legal for the receiving
device's role. Every message type is accepted from either direction.

MEMIF_MSG_TYPE_HELLO is a server-to-client message, but a connected
client can send one to a server. memif_msg_receive_hello() then sets
pmd->run.num_c2s_rings, pmd->run.num_s2c_rings and
pmd->run.log2_ring_size from values the peer supplied, and the
dispatcher goes on to run memif_init_regions_and_queues(), which is the
client-side initialisation path, on a server device. That path then
enqueues an ADD_REGION message for each entry in
proc_private->regions_num, meaning the server offers its own region file
descriptors to the untrusted peer.

Two consequences:

  1. A server can be driven into client-side state and can be made to
     hand its own shared memory file descriptors to the peer that
     connected to it. That inverts the trust direction the protocol
     depends on: the client is supposed to be the side that shares
     memory, and the server the side that receives it.

  2. Any server-side validation that reads pmd->run.* or
     proc_private->regions_num can be primed by the client with a
     spoofed HELLO before the messages being validated are sent. This
     matters for the ADD_REGION and ADD_RING validation filed
     separately: those checks are only sound once the state they read
     cannot be set by the peer.

Suggested fix
-------------

Reject messages sent in the wrong direction at dispatch, before any
handler runs:

  server to client only:  ACK, HELLO, CONNECTED
  client to server only:  INIT, ADD_REGION, ADD_RING, CONNECT
  both directions:        DISCONNECT

A wrong-direction message should disconnect the peer, since a conforming
implementation never sends one.

No fix has been written for this yet. It should land before, or in the
same series as, the ADD_REGION and ADD_RING validation, since those
checks read state this bug lets the peer set.

Reported by Arthur Chan <arthur.chan@adalogics.com> (Ada Logics), via
fuzzing.

-- 
You are receiving this mail because:
You are the assignee for the bug.