From f92667459fac817ba547ae87260471aad1f0a90c Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Fri, 3 Jul 2026 13:59:41 +0800 Subject: [PATCH] mctpd: Require mandatory peer query commands only We don't want to fail peer enumeration if the optional Get Endpoint UUID and Get Vendor Defined Message Support commands are unimplemented, so return success from query_peer_properties if these fail. However, we will now require Get Message Type Support to be implemented; it is mandatory according to DSP0236. Bump the warning for this to non-verbose mode too. This means we need to adjust the properties-query-retry tests, to expect enumeration failure on the max retries case. While we're here, make the error paths consistent between the two callers of query_peer_properties. Fixes: https://github.com/CodeConstruct/mctp/issues/161 Signed-off-by: Jeremy Kerr --- CHANGELOG.md | 8 ++++++ src/mctpd.c | 17 +++++++---- tests/test_mctpd.py | 70 +++++++++++++++++++++++++++++++++++++++------ 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 015180e..b5e4352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). invalid, having a zeroed control message header. These are now properly populated +3. `mctpd` will no longer (incorrectly) reject SetupEndpoint / AssignEndpoint + calls for endpoints that do not implement the optional Get Endpoint UUID + and Get Vendor Defined Message Support commands. + ### Changed 1. `mctpd`'s `mode` configuration (setting bus owner vs. endpoint roles) is @@ -45,6 +49,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). Set Endpoint ID commands before performing enumeration on the (bus-owner) peer. +4. `mctpd` now requires remote endpoints to implement the Get Message Type + Support command, as it is mandatory according to DSP0236. Peers not + implementing this will not be published, as enumeration will fail. + ## [2.5] - 2026-02-17 ### Added diff --git a/src/mctpd.c b/src/mctpd.c index c9a41db..983471e 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -3425,9 +3425,10 @@ static int query_peer_properties(struct peer *peer) int rc; rc = query_get_peer_msgtypes(peer); - if (rc < 0 && peer->ctx->verbose) { + if (rc < 0) { errno = -rc; warn("Error getting endpoint types for %s", peer_tostr(peer)); + return -1; } for (unsigned int i = 0; i < peer->num_message_types; i++) { @@ -3454,7 +3455,7 @@ static int query_peer_properties(struct peer *peer) } // TODO: emit property changed? Though currently they are all const. - return rc; + return 0; } static int peer_neigh_update(struct peer *peer, uint16_t type) @@ -3536,9 +3537,9 @@ static int setup_added_peer(struct peer *peer) rc = publish_peer(peer); out: - if (rc < 0) { + if (rc < 0) remove_peer(peer); - } + return rc; } @@ -3975,9 +3976,13 @@ static int method_net_learn_endpoint(sd_bus_message *call, void *data, goto err; } - query_peer_properties(peer); + rc = query_peer_properties(peer); + if (rc) + goto err; - publish_peer(peer); + rc = publish_peer(peer); + if (rc) + goto err; peer_path = path_from_peer(peer); if (!peer_path) diff --git a/tests/test_mctpd.py b/tests/test_mctpd.py index 02e4239..7853794 100644 --- a/tests/test_mctpd.py +++ b/tests/test_mctpd.py @@ -157,6 +157,65 @@ async def test_setup_endpoint_conflict(dbus, mctpd): assert eid1 != eid2 +class CommandUnimplementedEndpoint(Endpoint): + """An endpoint where one command (specified via opcode) reports as + unimplemented. + """ + + def __init__(self, opcode, *args, **kwargs): + self.unimplemented_opcode = opcode + super().__init__(*args, **kwargs) + + async def handle_mctp_control(self, sock, src_addr, msg): + flags, opcode = msg[0:2] + if opcode != self.unimplemented_opcode: + return await super().handle_mctp_control(sock, src_addr, msg) + # report not implemented + data = bytes([flags & 0x1F, opcode, 0x05]) + dst_addr = MCTPSockAddr.for_ep_resp(self, src_addr, sock.addr_ext) + await sock.send(dst_addr, data) + + +async def test_setup_endpoint_no_get_msg_types(dbus, mctpd): + """Test that endpoint enumeration fails if the endpoint does not + support the (mandatory) Get Message Type Support command. + """ + iface = mctpd.system.interfaces[0] + ep = CommandUnimplementedEndpoint(0x05, iface, bytes([0x1E])) + mctpd.network.add_endpoint(ep) + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + with pytest.raises(asyncdbus.errors.DBusError): + await mctp.call_setup_endpoint(ep.lladdr) + + +async def test_setup_endpoint_no_get_vdm_types(dbus, mctpd): + """Test that we can enumerate an endpoint where the (optional) + Get Vendor Defined Message Support command is not implemented + """ + iface = mctpd.system.interfaces[0] + ep = CommandUnimplementedEndpoint(0x06, iface, bytes([0x1E])) + mctpd.network.add_endpoint(ep) + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + (eid, net, path, new) = await mctp.call_setup_endpoint(ep.lladdr) + assert eid == ep.eid + + +async def test_setup_endpoint_no_get_uuid(dbus, mctpd): + """Test that we can enumerate an endpoint where the (optional) + Get Endpoint UUID command is not implemented + """ + + iface = mctpd.system.interfaces[0] + ep = CommandUnimplementedEndpoint(0x03, iface, bytes([0x1E])) + mctpd.network.add_endpoint(ep) + mctp = await mctpd_mctp_iface_obj(dbus, iface) + + (eid, net, path, new) = await mctp.call_setup_endpoint(ep.lladdr) + assert eid == ep.eid + + async def test_remove_endpoint(dbus, mctpd): """Test neighbour removal""" iface = mctpd.system.interfaces[0] @@ -1870,14 +1929,9 @@ async def handle_mctp_control(self, sock, addr, data): ep.lladdr = bytes([0x1C]) # change lladdr to force retry ep.timeout_count = 5 # timeout five times before responding - # call setup_endpoint again, which will trigger query of peer properties - (eid, net, path, new) = await mctp.call_setup_endpoint(ep.lladdr) - - # timeout five times does prevent us from getting the correct message types - objep = await mctpd_mctp_endpoint_common_obj(dbus, path) - objtypes = list(await objep.get_supported_message_types()) - expected_types = [] # exceeded retry limit, so no types known - assert objtypes == expected_types + # call setup_endpoint again, which will fail on the types query + with pytest.raises(asyncdbus.errors.DBusError): + await mctp.call_setup_endpoint(ep.lladdr) # exit mctpd res = await mctpd.stop_mctpd()