Merge branch '5017_fixing_module_avb_potential_memory_issue' into 'master'

5017 fixing module avb potential memory issue

See merge request pipewire/pipewire!2806

src/modules/module-avb/aecp-aem-cmds-resps/cmd-deregister-unsolicited-notifications.c

@@ -33,36 +33,24 @@ int handle_cmd_deregister_unsol_notif_milan_v12(struct aecp *aecp, int64_t now,
 	entity_state = (struct aecp_aem_entity_milan_state *) desc->ptr;
 	unsol = entity_state->unsol_notif_state;
 
-	/** First check if the controller was already registered */
+	/** Clear the slot matching this controller, if any */
 	for (index = 0; index < ctrler_max; index++)  {
 		uint64_t ctrl_eid = unsol[index].ctrler_entity_id;
 		bool ctrler_reged = unsol[index].is_registered;
 
 		if ((ctrl_eid == controller_id) && ctrler_reged) {
-			pw_log_debug("controller 0x%"PRIx64", already registered",
+			unsol[index].is_registered = false;
+			unsol[index].ctrler_entity_id = 0;
+			unsol[index].next_seq_id = 0;
+			unsol[index].port_id = 0;
+			memset(unsol[index].ctrler_mac_addr, 0,
+				sizeof(unsol[index].ctrler_mac_addr));
+			pw_log_info("Unsol deregistration for 0x%"PRIx64,
 					controller_id);
 			return reply_success(aecp, m, len);
 		}
 	}
 
-	/** When one slot is in the array is available use it */
+	pw_log_debug("controller 0x%"PRIx64" was not registered", controller_id);
-	for (index = 0; index < ctrler_max; index++)  {
-		if (!unsol[index].is_registered) {
-			break;
-		}
-	}
-
-	/** Reach the maximum controller allocated */
-	if (index == ctrler_max) {
-		return reply_no_resources(aecp, m, len);
-	}
-
-	unsol[index].ctrler_entity_id = controller_id;
-	memcpy(unsol[index].ctrler_mac_addr, h->src, sizeof(h->src));
-	unsol[index].is_registered = true;
-	unsol[index].port_id = 0;
-	unsol[index].next_seq_id = 0;
-
-	pw_log_info("Unsol registration for 0x%"PRIx64, controller_id);
 	return reply_success(aecp, m, len);
 }

src/modules/module-avb/aecp-aem-cmds-resps/cmd-get-set-clock-source.c

@@ -3,6 +3,7 @@
 /* SPDX-FileCopyrightText: Copyright © 2025 Alexandre Malki <alexandre.malki@kebag-logic.com> */
 /* SPDX-License-Identifier: MIT  */
 
+#include <errno.h>
 #include <stdint.h>
 #include <stdbool.h>
 
@@ -23,6 +24,9 @@ static int reply_invalid_clock_source(struct aecp *aecp,
 	struct avb_packet_aecp_aem *p = SPA_PTROFF(h, sizeof(*h), void);
 	struct avb_packet_aecp_aem_setget_clock_source *sclk_source;
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
+
 	memcpy(buf, m, len);
 	sclk_source =
 		(struct avb_packet_aecp_aem_setget_clock_source *) p->payload;
@@ -41,6 +45,9 @@ static int handle_unsol_set_clock_source(struct aecp *aecp, struct descriptor *d
 	struct aecp_aem_base_info bi = { 0 };
 	int rc;
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return -EINVAL;
+
 	memcpy(buf, m, len);
 	bi.controller_entity_id = htobe64(ctrler_id);
 	bi.expire_timeout = INT64_MAX;
@@ -67,12 +74,15 @@ int handle_cmd_get_clock_source_milan_v12(struct aecp *aecp, int64_t now,
 	uint16_t desc_index;
 	uint16_t desc_type;
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
+
 	memcpy(buf, m, len);
 	sclk_source =
 		(struct avb_packet_aecp_aem_setget_clock_source *) p->payload;
 
-	desc_index = htons(sclk_source->descriptor_id);
+	desc_type = ntohs(sclk_source->descriptor_type);
-	desc_type = htons(sclk_source->descriptor_id);
+	desc_index = ntohs(sclk_source->descriptor_id);
 
 	desc = server_find_descriptor(aecp->server, desc_type, desc_index);
 	if (desc == NULL)
@@ -84,8 +94,7 @@ int handle_cmd_get_clock_source_milan_v12(struct aecp *aecp, int64_t now,
 	/** Descriptors always keep the network endianness */
 	sclk_source->clock_source_index = dclk_domain->clock_source_index;
 
-	len = sizeof(*p) + sizeof(*sclk_source) + sizeof(*h);
+	return reply_success(aecp, buf, len);
-	return reply_success(aecp, m, len);
 }
 
 /**

src/modules/module-avb/aecp-aem-cmds-resps/cmd-get-set-control.c

@@ -3,6 +3,7 @@
 /* SPDX-FileCopyrightText: Copyright © 2026 Alexandre Malki <alexandre.malki@kebag-logic.com> */
 /* SPDX-License-Identifier: MIT */
 
+#include <errno.h>
 #include <limits.h>
 #include <inttypes.h>
 #include <stdbool.h>
@@ -44,6 +45,9 @@ static int send_unsol_control_milan_v12(struct aecp *aecp,
 	struct aecp_aem_base_info info = { 0 };
 	int rc = 0;
 
+	if (len > sizeof(unsol_buf))
+		return -EINVAL;
+
 	memcpy(unsol_buf, m, len);
 	/* Prepare a template packet */
 	info.controller_entity_id = htobe64(ctrler_id);
@@ -78,7 +82,13 @@ static int reply_control_badargs(struct aecp *aecp, const void *m, int len,
 			m, len);
 	}
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS,
+			m, len);
+
 	memcpy(buf, m, len);
+	if (pkt_size > len)
+		memset(buf + len, 0, pkt_size - len);
 	ae_reply = (struct avb_packet_aecp_aem_setget_control *)p_reply->payload;
 
 	control_copy_payload(format, ae_reply->payload, type_sz, count);
@@ -102,12 +112,19 @@ static int handle_cmd_get_control_identify(struct aecp *aecp, struct descriptor
 	ctrl_desc = desc->ptr;
 	desc_formats = ctrl_desc->value_format;
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS,
+			m, len);
+
 	memcpy(buf, m, len);
         ae_reply = (struct avb_packet_aecp_aem_setget_control *)p_reply->payload;
 
 	// Idenfity only has one value element
 	pkt_size = sizeof(*h) + sizeof(*p_reply)+ CONTROL_LINEAR_UINT8_SIZE;
 
+	if (pkt_size > len)
+		memset(buf + len, 0, pkt_size - len);
+
 	control_copy_payload(desc_formats, ae_reply->payload,
 					CONTROL_LINEAR_UINT8_SIZE, 1);
 
@@ -133,6 +150,11 @@ static int handle_cmd_set_control_identify(struct aecp *aecp, struct descriptor
 	old_value_format = desc_formats;
 	value_req = (uint8_t *)control->payload;
 
+	if (len < 0 || (size_t)len < sizeof(*h) + sizeof(*p) +
+			sizeof(*control) + CONTROL_LINEAR_UINT8_SIZE)
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS,
+			m, len);
+
 	if (*value_req == desc_formats->current_value) {
 		return reply_success(aecp, m, len);
 	}

src/modules/module-avb/aecp-aem-cmds-resps/cmd-get-set-name.c

@@ -2,6 +2,7 @@
 /* SPDX-FileCopyrightText: Copyright © 2025 Alexandre Malki <alexandre.malki@kebag-logic.com> */
 /* SPDX-License-Identifier: MIT */
 
+#include <errno.h>
 #include <limits.h>
 #include <stdbool.h>
 #include <inttypes.h>
@@ -73,6 +74,9 @@ static int send_unsol_name(struct aecp *aecp,
 	uint8_t unsol_buf[512];
 	struct aecp_aem_base_info info = { 0 };
 
+	if (len < 0 || (size_t)len > sizeof(unsol_buf))
+		return -EINVAL;
+
 	memcpy(unsol_buf, msg, len);
 	info.controller_entity_id = htobe64(p->aecp.controller_guid);
 	info.expire_timeout = INT64_MAX;
@@ -98,6 +102,12 @@ int handle_cmd_get_name_common(struct aecp *aecp, int64_t now,
 	struct descriptor *desc;
 	uint16_t desc_type, desc_id, name_index;
 	char *name_ptr;
+	size_t reply_len;
+
+	if (len < 0 || (size_t)len > sizeof(buf) ||
+	    (size_t)len < sizeof(*h) + sizeof(*p) + sizeof(*cmd))
+		return reply_status(aecp,
+				AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
 
 	cmd = (const struct avb_packet_aecp_aem_setget_name *)p->payload;
 	desc_type = ntohs(cmd->descriptor_type);
@@ -114,7 +124,9 @@ int handle_cmd_get_name_common(struct aecp *aecp, int64_t now,
 		return reply_status(aecp,
 				AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
 
-	memcpy(buf, m, len);
+	reply_len = sizeof(*h) + sizeof(*p) + sizeof(*cmd);
+
+	memcpy(buf, m, reply_len);
 	h_reply = (struct avb_ethernet_header *)buf;
 	p_reply = SPA_PTROFF(h_reply, sizeof(*h_reply), void);
 	reply = (struct avb_packet_aecp_aem_setget_name *)p_reply->payload;
@@ -126,7 +138,7 @@ int handle_cmd_get_name_common(struct aecp *aecp, int64_t now,
 	 */
 	memcpy(reply->name, name_ptr, 64);
 
-	return reply_success(aecp, buf, len);
+	return reply_success(aecp, buf, reply_len);
 }
 
 
@@ -162,6 +174,10 @@ int handle_cmd_set_name_common(struct aecp *aecp, int64_t now,
 		return reply_status(aecp,
 				AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
 
+	if (len < 0 || (size_t)len < sizeof(*h) + sizeof(*p) + sizeof(*cmd))
+		return reply_status(aecp,
+				AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
+
 	/**
 	 * IEEE 1722.1-2021: 7.4.17.1: The name does not contain a trailing NULL
 	 * but if the name is less than 64 bytes in length then it is zero

src/modules/module-avb/aecp-aem-cmds-resps/cmd-resp-helpers.h

@@ -6,6 +6,7 @@
 #ifndef __AVB_AECP_AEM_HELPERS_H__
 #define __AVB_AECP_AEM_HELPERS_H__
 
+#include <errno.h>
 #include <stdint.h>
 #include <string.h>
 
@@ -21,6 +22,11 @@ static inline int reply_status(struct aecp *aecp, int status, const void *m, int
 	struct avb_ethernet_header *h = (void*)buf;
 	struct avb_packet_aecp_header *reply = SPA_PTROFF(h, sizeof(*h), void);
 
+	if (len < 0 || (size_t)len > sizeof(buf)) {
+		pw_log_warn("reply_status: invalid len %d", len);
+		return -EINVAL;
+	}
+
 	memcpy(buf, m, len);
 
 	pw_log_debug("status 0x%x\n", status);

src/modules/module-avb/aecp-aem-cmds-resps/reply-unsol-helpers.c

@@ -111,6 +111,9 @@ static void reply_unsol_notifications_prepare(struct aecp *aecp,
 
 	/* Here the value of 12 is the delta between the target_entity_id and
 	   start of the AECP message specific data. */
+	if (len < sizeof(*h) + sizeof(*p))
+		ctrl_data_length = AVB_PACKET_CONTROL_DATA_OFFSET;
+	else
 		ctrl_data_length = len - (sizeof(*h) + sizeof(*p))
 					+ AVB_PACKET_CONTROL_DATA_OFFSET;
 

src/modules/module-avb/aecp-aem.c

@@ -100,11 +100,17 @@ static int handle_read_descriptor_common(struct aecp *aecp, int64_t now, const v
 	if (desc == NULL)
 		return reply_status(aecp, AVB_AECP_AEM_STATUS_NO_SUCH_DESCRIPTOR, m, len);
 
-	memcpy(buf, m, len);
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_BAD_ARGUMENTS, m, len);
 
 	psize = sizeof(*rd);
 	size = sizeof(*h) + sizeof(*reply) + psize;
 
+	if (desc->size > sizeof(buf) || size > sizeof(buf) - desc->size)
+		return reply_status(aecp, AVB_AECP_AEM_STATUS_NO_RESOURCES, m, len);
+
+	memcpy(buf, m, len);
+
 	memcpy(buf + size, desc->ptr, desc->size);
 	size += desc->size;
 	psize += desc->size;

src/modules/module-avb/aecp.c

@@ -2,6 +2,8 @@
 /* SPDX-FileCopyrightText: Copyright © 2022 Wim Taymans */
 /* SPDX-License-Identifier: MIT */
 
+#include <errno.h>
+
 #include <spa/utils/json.h>
 #include <spa/debug/mem.h>
 
@@ -22,10 +24,13 @@ struct msg_info {
 static int reply_not_implemented(struct aecp *aecp, const void *p, int len)
 {
 	struct server *server = aecp->server;
-	uint8_t buf[len];
+	uint8_t buf[AVB_PACKET_MILAN_DEFAULT_MTU];
 	struct avb_ethernet_header *h = (void*)buf;
 	struct avb_packet_aecp_header *reply = SPA_PTROFF(h, sizeof(*h), void);
 
+	if (len < 0 || (size_t)len > sizeof(buf))
+		return -EINVAL;
+
 	memcpy(h, p, len);
 	AVB_PACKET_AECP_SET_STATUS(reply, AVB_AECP_STATUS_NOT_IMPLEMENTED);
 
@@ -64,6 +69,10 @@ static int aecp_message(void *data, uint64_t now, const void *message, int len)
 	const struct msg_info *info;
 	int message_type;
 
+	if (len < 0 ||
+	    (size_t)len < sizeof(*h) + sizeof(*p) ||
+	    (size_t)len > AVB_PACKET_MILAN_DEFAULT_MTU)
+		return 0;
 	if (ntohs(h->type) != AVB_TSN_ETH)
 		return 0;
 	if (memcmp(h->dest, mac, 6) != 0 &&