mctpd: prevent endless VDM query & allocation

Commit cb182be ("mctpd: Add VendorDefinedMessageTypes property") did not
check the returned response selector against anything other than 0xff,
so a remote endpoint could cause a repeated allocation of vdm types.

Keep track of the selector we requested, and ensure that the response is
as expected (ie, either +1 or 0xff). Add a test for a repeated selector
behaviour.

Fixes: cb182be ("mctpd: Add VendorDefinedMessageTypes property")
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Author: jk-ozlabs

src/mctpd.c

@@ -2598,17 +2598,17 @@ static int query_get_peer_vdm_types(struct peer *peer)
 	size_t buf_size, expect_size, new_size, num_vdm_types = 0;
 	struct mctp_ctrl_resp_get_vdm_support *resp = NULL;
 	struct mctp_ctrl_cmd_get_vdm_support req;
+	uint8_t selector, iid, fmt, *buf = NULL;
 	struct mctp_vdm_pcie_data *vdm_pcie;
 	struct mctp_vdm_iana_data *vdm_iana;
 	struct sockaddr_mctp_ext addr;
-	uint8_t iid, fmt, *buf = NULL;
 	int rc;
 
 	req.ctrl_hdr.command_code = MCTP_CTRL_CMD_GET_VENDOR_MESSAGE_SUPPORT;
-	req.vendor_id_set_selector = 0;
 
-	while (true) {
+	for (selector = 0;; selector++) {
 		iid = mctp_next_iid(peer->ctx);
+		req.vendor_id_set_selector = selector;
 
 		mctp_ctrl_msg_hdr_init_req(
 			&req.ctrl_hdr, iid,
@@ -2681,10 +2681,16 @@ static int query_get_peer_vdm_types(struct peer *peer)
 			peer->num_vdm_types = num_vdm_types;
 			rc = 0;
 			break;
+		} else if (resp->vendor_id_set_selector != selector + 1) {
+			/* DSP0236 requires set selectors to be monotonically
+			 * increasing by 1 */
+			warnx("peer %s reporting invalid VDM selector 0x%02x, "
+			      "expected 0x%02x",
+			      peer_tostr_short(peer),
+			      resp->vendor_id_set_selector, selector + 1);
+			rc = -EPROTO;
+			break;
 		}
-
-		/* Use the next selector from the response. 0xFF indicates no more entries */
-		req.vendor_id_set_selector = resp->vendor_id_set_selector;
 	}
 
 	free(buf);

tests/test_mctpd.py

@@ -749,6 +749,30 @@ async def test_query_vdm_types_unsupported(dbus, mctpd):
     await _assert_vdm_types_empty(dbus, mctp, ep)
 
 
+async def test_query_vdm_types_repeating(dbus, mctpd):
+    """Test that we need to be increasing the VDM set selector: otherwise
+    a peer may send infinite VDM types
+    """
+
+    class RepeatingVDMEndpoint(Endpoint):
+        async def handle_mctp_control(self, sock, addr, data):
+            flags, opcode = data[0:2]
+            if opcode != 0x06:
+                return await super().handle_mctp_control(sock, addr, data)
+            iid = flags & 0x1F
+            raddr = MCTPSockAddr.for_ep_resp(self, addr, sock.addr_ext)
+            hdr = [iid, opcode]
+            # always report a next selector of 1
+            resp = hdr + [0x00, 0x01, 0x00, 0xAA, 0xBB, 0xCC, 0xDD]
+            await sock.send(raddr, bytes(resp))
+
+    iface = mctpd.system.interfaces[0]
+    mctp = await mctpd_mctp_iface_obj(dbus, iface)
+    ep = RepeatingVDMEndpoint(iface, bytes([0x21]), eid=18, vdm_msg_types=None)
+    mctpd.network.add_endpoint(ep)
+    await _assert_vdm_types_empty(dbus, mctp, ep)
+
+
 async def test_network_local_eids_single(dbus, mctpd):
     """Network1.LocalEIDs should reflect locally-assigned EID state"""
     iface = mctpd.system.interfaces[0]