From 5bb9ae237d94b6a5c30b2791c5207073aff68cbc Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Tue, 7 Apr 2026 07:01:45 -0400 Subject: [PATCH 1/6] module-avb: fix heap corruption in server_destroy_descriptors server_add_descriptor() allocates the descriptor and its data in a single calloc (d->ptr = SPA_PTROFF(d, sizeof(struct descriptor))), so d->ptr points inside the same allocation as d. Calling free(d->ptr) frees an interior pointer, corrupting the heap. Only free(d) is needed. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/internal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/module-avb/internal.h b/src/modules/module-avb/internal.h index 82ced2f21..e5b283fc0 100644 --- a/src/modules/module-avb/internal.h +++ b/src/modules/module-avb/internal.h @@ -102,7 +102,6 @@ static inline void server_destroy_descriptors(struct server *server) struct descriptor *d, *t; spa_list_for_each_safe(d, t, &server->descriptors, link) { - free(d->ptr); spa_list_remove(&d->link); free(d); } From c7fa28b7cf4c809ce03dfe050abe8fc34c620bce Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Tue, 7 Apr 2026 07:24:34 -0400 Subject: [PATCH 2/6] module-avb: fix potential NULL pointer dereference in MSRP/MVRP notify The msrp_notify() and mvrp_notify() functions call dispatch table notify callbacks without checking for NULL. In MSRP, the TALKER_FAILED attribute type has a NULL notify callback, which would crash if a talker-failed attribute received a registrar state change notification (e.g. RX_NEW triggering NOTIFY_NEW). Add NULL checks before calling the dispatch notify callbacks, matching the defensive pattern used in the encode path. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/msrp.c | 3 ++- src/modules/module-avb/mvrp.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/module-avb/msrp.c b/src/modules/module-avb/msrp.c index 92d1e65b4..40cb57268 100644 --- a/src/modules/module-avb/msrp.c +++ b/src/modules/module-avb/msrp.c @@ -332,7 +332,8 @@ static void msrp_notify(void *data, uint64_t now, uint8_t notify) { struct attr *a = data; struct msrp *msrp = a->msrp; - return dispatch[a->attr.type].notify(msrp, now, a, notify); + if (dispatch[a->attr.type].notify) + dispatch[a->attr.type].notify(msrp, now, a, notify); } static const struct avb_mrp_attribute_events mrp_attr_events = { diff --git a/src/modules/module-avb/mvrp.c b/src/modules/module-avb/mvrp.c index 20862c2ae..e2667ce40 100644 --- a/src/modules/module-avb/mvrp.c +++ b/src/modules/module-avb/mvrp.c @@ -171,7 +171,8 @@ static void mvrp_notify(void *data, uint64_t now, uint8_t notify) { struct attr *a = data; struct mvrp *mvrp = a->mvrp; - return dispatch[a->attr.type].notify(mvrp, now, a, notify); + if (dispatch[a->attr.type].notify) + dispatch[a->attr.type].notify(mvrp, now, a, notify); } static const struct avb_mrp_attribute_events mrp_attr_events = { From 8e7e4334126702e8039fd5fb3023ed26576ea3e9 Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Tue, 7 Apr 2026 08:23:43 -0400 Subject: [PATCH 3/6] module-avb: fix MRP NEW messages never being transmitted AVB_MRP_SEND_NEW was defined as 0, making it indistinguishable from "no pending send" in the MSRP and MVRP event handlers which check `if (!pending_send)`. This meant that when an attribute was first declared (applicant state VN or AN), the NEW message was silently dropped instead of being transmitted on the network. Fix by shifting all AVB_MRP_SEND_* values to start at 1, so that 0 unambiguously means "no send pending". Update the MSRP and MVRP encoders to subtract 1 when encoding to the IEEE 802.1Q wire format (which uses 0-based event values). Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/mrp.c | 2 ++ src/modules/module-avb/mrp.h | 14 +++++++------- src/modules/module-avb/msrp.c | 6 +++--- src/modules/module-avb/mvrp.c | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/modules/module-avb/mrp.c b/src/modules/module-avb/mrp.c index 73d5275ca..c6505d41b 100644 --- a/src/modules/module-avb/mrp.c +++ b/src/modules/module-avb/mrp.c @@ -302,6 +302,8 @@ const char *avb_mrp_notify_name(uint8_t notify) const char *avb_mrp_send_name(uint8_t send) { switch(send) { + case 0: + return "none"; case AVB_MRP_SEND_NEW: return "new"; case AVB_MRP_SEND_JOININ: diff --git a/src/modules/module-avb/mrp.h b/src/modules/module-avb/mrp.h index 78683412c..399343267 100644 --- a/src/modules/module-avb/mrp.h +++ b/src/modules/module-avb/mrp.h @@ -88,13 +88,13 @@ struct avb_packet_mrp_footer { #define AVB_MRP_ATTRIBUTE_EVENT_LV 5 #define AVB_MRP_ATTRIBUTE_EVENT_LVA 6 -#define AVB_MRP_SEND_NEW 0 -#define AVB_MRP_SEND_JOININ 1 -#define AVB_MRP_SEND_IN 2 -#define AVB_MRP_SEND_JOINMT 3 -#define AVB_MRP_SEND_MT 4 -#define AVB_MRP_SEND_LV 5 -#define AVB_MRP_SEND_LVA 6 +#define AVB_MRP_SEND_NEW 1 +#define AVB_MRP_SEND_JOININ 2 +#define AVB_MRP_SEND_IN 3 +#define AVB_MRP_SEND_JOINMT 4 +#define AVB_MRP_SEND_MT 5 +#define AVB_MRP_SEND_LV 6 +#define AVB_MRP_SEND_LVA 7 #define AVB_MRP_NOTIFY_NEW 1 #define AVB_MRP_NOTIFY_JOIN 2 diff --git a/src/modules/module-avb/msrp.c b/src/modules/module-avb/msrp.c index 40cb57268..611ddd537 100644 --- a/src/modules/module-avb/msrp.c +++ b/src/modules/module-avb/msrp.c @@ -91,7 +91,7 @@ static int encode_talker(struct msrp *msrp, struct attr *a, void *m) *t = a->attr.attr.talker; ev = SPA_PTROFF(t, sizeof(*t), uint8_t); - *ev = a->attr.mrp->pending_send * 6 * 6; + *ev = (a->attr.mrp->pending_send - 1) * 6 * 6; f = SPA_PTROFF(ev, sizeof(*ev), struct avb_packet_mrp_footer); f->end_mark = 0; @@ -170,7 +170,7 @@ static int encode_listener(struct msrp *msrp, struct attr *a, void *m) *l = a->attr.attr.listener; ev = SPA_PTROFF(l, sizeof(*l), uint8_t); - *ev = a->attr.mrp->pending_send * 6 * 6; + *ev = (a->attr.mrp->pending_send - 1) * 6 * 6; ev = SPA_PTROFF(ev, sizeof(*ev), uint8_t); *ev = a->attr.param * 4 * 4 * 4; @@ -226,7 +226,7 @@ static int encode_domain(struct msrp *msrp, struct attr *a, void *m) *d = a->attr.attr.domain; ev = SPA_PTROFF(d, sizeof(*d), uint8_t); - *ev = a->attr.mrp->pending_send * 36; + *ev = (a->attr.mrp->pending_send - 1) * 36; f = SPA_PTROFF(ev, sizeof(*ev), struct avb_packet_mrp_footer); f->end_mark = 0; diff --git a/src/modules/module-avb/mvrp.c b/src/modules/module-avb/mvrp.c index e2667ce40..e2e501a9e 100644 --- a/src/modules/module-avb/mvrp.c +++ b/src/modules/module-avb/mvrp.c @@ -84,7 +84,7 @@ static int encode_vid(struct mvrp *mvrp, struct attr *a, void *m) *d = a->attr.attr.vid; ev = SPA_PTROFF(d, sizeof(*d), uint8_t); - *ev = a->attr.mrp->pending_send * 36; + *ev = (a->attr.mrp->pending_send - 1) * 36; f = SPA_PTROFF(ev, sizeof(*ev), struct avb_packet_mrp_footer); f->end_mark = 0; From 1caa242358b25fccd0c4443e385b9d2ac39b3f58 Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Wed, 8 Apr 2026 17:10:32 -0400 Subject: [PATCH 4/6] module-avb: fix ACMP error responses sent with wrong message type In handle_connect_tx_command() and handle_disconnect_tx_command(), AVB_PACKET_ACMP_SET_MESSAGE_TYPE() is called after the goto done target. When find_stream() fails and jumps to done, the response is sent with the original command message type (e.g., CONNECT_TX_COMMAND) instead of the correct response type (CONNECT_TX_RESPONSE). Move the SET_MESSAGE_TYPE call before find_stream() so error responses are always sent with the correct response message type. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/acmp.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/modules/module-avb/acmp.c b/src/modules/module-avb/acmp.c index eacfb6133..73a84ba89 100644 --- a/src/modules/module-avb/acmp.c +++ b/src/modules/module-avb/acmp.c @@ -174,13 +174,14 @@ static int handle_connect_tx_command(struct acmp *acmp, uint64_t now, const void return 0; memcpy(buf, m, len); + AVB_PACKET_ACMP_SET_MESSAGE_TYPE(reply, AVB_ACMP_MESSAGE_TYPE_CONNECT_TX_RESPONSE); + stream = find_stream(server, SPA_DIRECTION_OUTPUT, ntohs(reply->talker_unique_id)); if (stream == NULL) { status = AVB_ACMP_STATUS_TALKER_NO_STREAM_INDEX; goto done; } - AVB_PACKET_ACMP_SET_MESSAGE_TYPE(reply, AVB_ACMP_MESSAGE_TYPE_CONNECT_TX_RESPONSE); reply->stream_id = htobe64(stream->id); stream_activate(stream, ntohs(reply->talker_unique_id), now); @@ -251,14 +252,14 @@ static int handle_disconnect_tx_command(struct acmp *acmp, uint64_t now, const v return 0; memcpy(buf, m, len); + AVB_PACKET_ACMP_SET_MESSAGE_TYPE(reply, AVB_ACMP_MESSAGE_TYPE_DISCONNECT_TX_RESPONSE); + stream = find_stream(server, SPA_DIRECTION_OUTPUT, ntohs(reply->talker_unique_id)); if (stream == NULL) { status = AVB_ACMP_STATUS_TALKER_NO_STREAM_INDEX; goto done; } - AVB_PACKET_ACMP_SET_MESSAGE_TYPE(reply, AVB_ACMP_MESSAGE_TYPE_DISCONNECT_TX_RESPONSE); - stream_deactivate(stream, now); done: From d9508ef3340d1be64ae2b17e8f7016c3aba4174d Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Tue, 7 Apr 2026 13:07:18 -0400 Subject: [PATCH 5/6] module-avb: fix legacy AECP handlers reading payload at wrong offset handle_acquire_entity_avb_legacy() and handle_lock_entity_avb_legacy() incorrectly treated the full ethernet frame pointer as the AEM packet pointer, causing p->payload to read descriptor_type and descriptor_id from the wrong offset. Fix by properly skipping the ethernet header, matching the pattern used by all other AEM command handlers. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/aecp-aem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/module-avb/aecp-aem.c b/src/modules/module-avb/aecp-aem.c index fccf6b178..e655f02c5 100644 --- a/src/modules/module-avb/aecp-aem.c +++ b/src/modules/module-avb/aecp-aem.c @@ -27,7 +27,8 @@ static int handle_acquire_entity_avb_legacy(struct aecp *aecp, int64_t now, const void *m, int len) { struct server *server = aecp->server; - const struct avb_packet_aecp_aem *p = m; + const struct avb_ethernet_header *h = m; + const struct avb_packet_aecp_aem *p = SPA_PTROFF(h, sizeof(*h), void); const struct avb_packet_aecp_aem_acquire *ae; const struct descriptor *desc; uint16_t desc_type, desc_id; @@ -53,7 +54,8 @@ static int handle_lock_entity_avb_legacy(struct aecp *aecp, int64_t now, const void *m, int len) { struct server *server = aecp->server; - const struct avb_packet_aecp_aem *p = m; + const struct avb_ethernet_header *h = m; + const struct avb_packet_aecp_aem *p = SPA_PTROFF(h, sizeof(*h), void); const struct avb_packet_aecp_aem_acquire *ae; const struct descriptor *desc; uint16_t desc_type, desc_id; From 4d0bf1c5a6ef2b93787a665a5139a34496bb8831 Mon Sep 17 00:00:00 2001 From: "Christian F.K. Schaller" Date: Tue, 7 Apr 2026 13:07:24 -0400 Subject: [PATCH 6/6] module-avb: fix Milan lock entity error response and re-lock timeout Fix two bugs in handle_cmd_lock_entity_milan_v12(): 1. When server_find_descriptor() returns NULL, reply_status() was called with the AEM packet pointer instead of the full ethernet frame, corrupting the response ethernet header. 2. When refreshing an existing lock, the expire timeout was extended by raw seconds (60) instead of nanoseconds (60 * SPA_NSEC_PER_SEC), causing the lock to expire almost immediately after re-lock. Co-Authored-By: Claude Opus 4.6 --- src/modules/module-avb/aecp-aem-cmds-resps/cmd-lock-entity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/module-avb/aecp-aem-cmds-resps/cmd-lock-entity.c b/src/modules/module-avb/aecp-aem-cmds-resps/cmd-lock-entity.c index 0149d633b..73b7d2548 100644 --- a/src/modules/module-avb/aecp-aem-cmds-resps/cmd-lock-entity.c +++ b/src/modules/module-avb/aecp-aem-cmds-resps/cmd-lock-entity.c @@ -99,7 +99,7 @@ int handle_cmd_lock_entity_milan_v12(struct aecp *aecp, int64_t now, const void desc = server_find_descriptor(server, desc_type, desc_id); if (desc == NULL) - return reply_status(aecp, AVB_AECP_AEM_STATUS_NO_SUCH_DESCRIPTOR, p, len); + return reply_status(aecp, AVB_AECP_AEM_STATUS_NO_SUCH_DESCRIPTOR, m, len); entity_state = desc->ptr; lock = &entity_state->state.lock_state; @@ -148,7 +148,7 @@ int handle_cmd_lock_entity_milan_v12(struct aecp *aecp, int64_t now, const void // If the lock is taken again by device if (ctrler_id == lock->locked_id) { lock->base_info.expire_timeout += - AECP_AEM_LOCK_ENTITY_EXPIRE_TIMEOUT_SECOND; + AECP_AEM_LOCK_ENTITY_EXPIRE_TIMEOUT_SECOND * SPA_NSEC_PER_SEC; lock->is_locked = true; } else {