Skip to content

modem: cmux: Handle C/R bit from address field #96617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+46 −9
Open

modem: cmux: Handle C/R bit from address field #96617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/zephyr/modem/cmux.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
Expand Down
32 changes: 25 additions & 7 deletions subsys/modem/modem_cmux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -593,16 +594,16 @@ 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");
return;
}

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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions tests/subsys/modem/modem_cmux/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* We are initiator, so any CMD with CR set, should be dropped */
/* 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);