From ed88b81ef869ff265c449b44492830b04089e66e Mon Sep 17 00:00:00 2001 From: Seppo Takalo Date: Tue, 26 Aug 2025 09:45:31 +0300 Subject: [PATCH] modem: cmux: Handle C/R bit from address field The C/R bit in the address field should be handled as explained in 5.2.1.2 in the spec (3GPP TS 127.010). To detect if the frame is a command or a response, we need to know who was the initiator of the CMUX channel. > Initiator is the station that take the initiative to initialize > the multiplexer (i.e. sends the SABM command at DLCI 0 ) See the table from given section of the specification. Also, on UIH frames 5.4.3.1 says > The frames sent by the initiating station have the C/R bit set to 1 > and those sent by the responding station have the C/R bit set to 0. NOTE: This is different than a C/R bit in the Type field. Signed-off-by: Seppo Takalo --- include/zephyr/modem/cmux.h | 5 ++-- subsys/modem/modem_cmux.c | 32 ++++++++++++++++++------ tests/subsys/modem/modem_cmux/src/main.c | 18 +++++++++++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/include/zephyr/modem/cmux.h b/include/zephyr/modem/cmux.h index 913f4187d7292..4927062e3fc4d 100644 --- a/include/zephyr/modem/cmux.h +++ b/include/zephyr/modem/cmux.h @@ -142,10 +142,11 @@ struct modem_cmux { /* State */ enum modem_cmux_state state; - bool flow_control_on; + bool flow_control_on : 1; + bool initiator : 1; /* Work lock */ - bool attached; + bool attached : 1; struct k_spinlock work_lock; /* Receive state*/ diff --git a/subsys/modem/modem_cmux.c b/subsys/modem/modem_cmux.c index c3bf78681ee6b..5d0bdbe4db8cb 100644 --- a/subsys/modem/modem_cmux.c +++ b/subsys/modem/modem_cmux.c @@ -461,12 +461,13 @@ static void modem_cmux_on_cld_command(struct modem_cmux *cmux, struct modem_cmux static void modem_cmux_on_control_frame_ua(struct modem_cmux *cmux) { if (cmux->state != MODEM_CMUX_STATE_CONNECTING) { - LOG_DBG("Unexpected UA frame"); + LOG_DBG("Unexpected UA frame in state %d", cmux->state); return; } LOG_DBG("CMUX connected"); cmux->state = MODEM_CMUX_STATE_CONNECTED; + cmux->initiator = true; k_mutex_lock(&cmux->transmit_rb_lock, K_FOREVER); cmux->flow_control_on = true; k_mutex_unlock(&cmux->transmit_rb_lock); @@ -593,8 +594,6 @@ static void modem_cmux_dm_response_transmit(struct modem_cmux *cmux) static void modem_cmux_on_control_frame_sabm(struct modem_cmux *cmux) { - modem_cmux_connect_response_transmit(cmux); - if ((cmux->state == MODEM_CMUX_STATE_CONNECTED) || (cmux->state == MODEM_CMUX_STATE_DISCONNECTING)) { LOG_DBG("Connect request not accepted"); @@ -602,7 +601,9 @@ static void modem_cmux_on_control_frame_sabm(struct modem_cmux *cmux) } LOG_DBG("CMUX connection request received"); + cmux->initiator = false; cmux->state = MODEM_CMUX_STATE_CONNECTED; + modem_cmux_connect_response_transmit(cmux); k_mutex_lock(&cmux->transmit_rb_lock, K_FOREVER); cmux->flow_control_on = true; k_mutex_unlock(&cmux->transmit_rb_lock); @@ -615,6 +616,11 @@ static void modem_cmux_on_control_frame(struct modem_cmux *cmux) { modem_cmux_log_received_frame(&cmux->frame); + if (cmux->state == MODEM_CMUX_STATE_CONNECTED && cmux->frame.cr == cmux->initiator) { + LOG_DBG("Received a response frame, dropping"); + return; + } + switch (cmux->frame.type) { case MODEM_CMUX_FRAME_TYPE_UA: modem_cmux_on_control_frame_ua(cmux); @@ -659,6 +665,12 @@ static void modem_cmux_on_dlci_frame_dm(struct modem_cmux_dlci *dlci) static void modem_cmux_on_dlci_frame_ua(struct modem_cmux_dlci *dlci) { + /* Drop invalid UA frames */ + if (dlci->cmux->frame.cr != dlci->cmux->initiator) { + LOG_DBG("Received a response frame, dropping"); + return; + } + switch (dlci->state) { case MODEM_CMUX_DLCI_STATE_OPENING: LOG_DBG("DLCI %u opened", dlci->dlci_address); @@ -744,6 +756,11 @@ static void modem_cmux_on_dlci_frame(struct modem_cmux *cmux) modem_cmux_log_received_frame(&cmux->frame); + if (cmux->state != MODEM_CMUX_STATE_CONNECTED) { + LOG_DBG("Unexpected DLCI frame in state %d", cmux->state); + return; + } + dlci = modem_cmux_find_dlci(cmux); if (dlci == NULL) { LOG_WRN("Frame intended for unconfigured DLCI %u.", @@ -1085,6 +1102,7 @@ static void modem_cmux_connect_handler(struct k_work *item) cmux = CONTAINER_OF(dwork, struct modem_cmux, connect_work); cmux->state = MODEM_CMUX_STATE_CONNECTING; + cmux->initiator = true; static const struct modem_cmux_frame frame = { .dlci_address = 0, @@ -1117,7 +1135,7 @@ static void modem_cmux_disconnect_handler(struct k_work *item) struct modem_cmux_frame frame = { .dlci_address = 0, - .cr = true, + .cr = cmux->initiator, .pf = false, .type = MODEM_CMUX_FRAME_TYPE_UIH, .data = data, @@ -1196,7 +1214,7 @@ static int modem_cmux_dlci_pipe_api_transmit(void *data, const uint8_t *buf, siz struct modem_cmux_frame frame = { .dlci_address = dlci->dlci_address, - .cr = true, + .cr = cmux->initiator, .pf = false, .type = MODEM_CMUX_FRAME_TYPE_UIH, .data = buf, @@ -1271,7 +1289,7 @@ static void modem_cmux_dlci_open_handler(struct k_work *item) struct modem_cmux_frame frame = { .dlci_address = dlci->dlci_address, - .cr = true, + .cr = dlci->cmux->initiator, .pf = true, .type = MODEM_CMUX_FRAME_TYPE_SABM, .data = NULL, @@ -1300,7 +1318,7 @@ static void modem_cmux_dlci_close_handler(struct k_work *item) struct modem_cmux_frame frame = { .dlci_address = dlci->dlci_address, - .cr = true, + .cr = dlci->cmux->initiator, .pf = true, .type = MODEM_CMUX_FRAME_TYPE_DISC, .data = NULL, diff --git a/tests/subsys/modem/modem_cmux/src/main.c b/tests/subsys/modem/modem_cmux/src/main.c index b1adce37cc46a..cb6bd0b6c5c3e 100644 --- a/tests/subsys/modem/modem_cmux/src/main.c +++ b/tests/subsys/modem/modem_cmux/src/main.c @@ -886,4 +886,22 @@ ZTEST(modem_cmux, test_modem_cmux_split_large_data) "Incorrect number of bytes transmitted %d", ret); } +ZTEST(modem_cmux, test_modem_cmux_invalid_cr) +{ + uint32_t events; + + /* We are initiator, so any CMD with CR set, should be dropped */ + modem_backend_mock_put(&bus_mock, cmux_frame_control_cld_cmd, + sizeof(cmux_frame_control_cld_cmd)); + + modem_backend_mock_put(&bus_mock, cmux_frame_control_sabm_cmd, + sizeof(cmux_frame_control_sabm_cmd)); + + events = k_event_wait_all(&cmux_event, + (MODEM_CMUX_EVENT_CONNECTED | MODEM_CMUX_EVENT_DISCONNECTED), + false, K_MSEC(100)); + + zassert_false(events, "Wrong CMD should have been ignored"); +} + ZTEST_SUITE(modem_cmux, NULL, test_modem_cmux_setup, test_modem_cmux_before, NULL, NULL);