Skip to content

Commit

Permalink
sample: fix issue with sending malformed payload on autoconnect
Browse files Browse the repository at this point in the history 
KRKNWK-18805
The issue has been investigated, and it was caused by the
`sid_put_msg` performing shallow copy of the data to send.
This was not intended, and will be fixed in next release.

Temporary fix for this issue, is to track the payload passed to
`sid_put_msg` and free it when message has been send, or when
there will be no further attempts with sending.

So this commit is temporary and should be reverted for the next release.

Signed-off-by: Robert Gałat <[email protected]>
  • Loading branch information
@RobertGalatNordic
RobertGalatNordic committed May 22, 2024
1 parent c89cbf9 commit c08bec3
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 4 deletions.
5 changes: 5 additions & 0 deletions samples/sid_end_device/include/sidewalk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#ifndef SIDEWALK_APP_H
#define SIDEWALK_APP_H

#include <zephyr/sys/slist.h>
#include <sid_api.h>
#include <zephyr/kernel.h>

typedef enum {
SID_EVENT_SIDEWALK,
Expand All @@ -32,6 +34,7 @@ typedef struct {
} sidewalk_ctx_t;

typedef struct {
sys_snode_t node;
struct sid_msg msg;
struct sid_msg_desc desc;
} sidewalk_msg_t;
Expand All @@ -46,4 +49,6 @@ void sidewalk_start(sidewalk_ctx_t *context);

int sidewalk_event_send(sidewalk_event_t event, void *ctx);

sidewalk_msg_t *get_message_buffer(uint16_t message_id);

#endif /* SIDEWALK_APP_H */
14 changes: 14 additions & 0 deletions samples/sid_end_device/src/cli/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *cont
LOG_INF("Message send success");
printk(JSON_NEW_LINE(JSON_OBJ(JSON_NAME(
"on_msg_sent", JSON_OBJ(JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0))))));
sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc,
Expand All @@ -50,6 +57,13 @@ static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc
"on_send_error",
JSON_OBJ(JSON_LIST_2(JSON_VAL_sid_error_t("error", error),
JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0)))))));
sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_factory_reset(void *context)
Expand Down
15 changes: 15 additions & 0 deletions samples/sid_end_device/src/hello/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *cont
"on_msg_sent", JSON_OBJ(JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0))))));

application_state_sending(&global_state_notifier, false);
sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc,
Expand All @@ -102,6 +109,14 @@ static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc
JSON_VAL_sid_msg_desc("sid_msg_desc", msg_desc, 0)))))));

application_state_sending(&global_state_notifier, false);

sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_factory_reset(void *context)
Expand Down
14 changes: 14 additions & 0 deletions samples/sid_end_device/src/sensor_monitoring/app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,27 @@ static void on_sidewalk_msg_received(const struct sid_msg_desc *msg_desc, const
static void on_sidewalk_msg_sent(const struct sid_msg_desc *msg_desc, void *context)
{
LOG_DBG("sent message(type: %d, id: %u)", (int)msg_desc->type, msg_desc->id);
sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_send_error(sid_error_t error, const struct sid_msg_desc *msg_desc,
void *context)
{
LOG_ERR("Send message err %d", (int)error);
LOG_DBG("Failed to send message(type: %d, id: %u)", (int)msg_desc->type, msg_desc->id);
sidewalk_msg_t *message = get_message_buffer(msg_desc->id);
if (message == NULL) {
LOG_ERR("failed to find message buffer to clean");
return;
}
sid_hal_free(message->msg.data);
sid_hal_free(message);
}

static void on_sidewalk_factory_reset(void *context)
Expand Down
36 changes: 32 additions & 4 deletions samples/sid_end_device/src/sidewalk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

#include <sid_pal_crypto_ifc.h>
#include <stdio.h>

Expand Down Expand Up @@ -43,6 +42,24 @@ LOG_MODULE_REGISTER(sidewalk_app, CONFIG_SIDEWALK_LOG_LEVEL);
static struct k_thread sid_thread;
K_THREAD_STACK_DEFINE(sid_thread_stack, CONFIG_SIDEWALK_THREAD_STACK_SIZE);

sys_slist_t pending_message_list = SYS_SLIST_STATIC_INIT(&pending_message_list);

sidewalk_msg_t *get_message_buffer(uint16_t message_id)
{
sidewalk_msg_t *pending_message;
sidewalk_msg_t *iterator;
SYS_SLIST_FOR_EACH_CONTAINER_SAFE (&pending_message_list, pending_message, iterator, node) {
if (pending_message->desc.id == message_id) {
if (sys_slist_find_and_remove(&pending_message_list,
&pending_message->node) == false) {
LOG_ERR("Failed to remove pending message from list");
};
return pending_message;
}
}
return NULL;
}

typedef struct sm_s {
struct smf_ctx ctx;
struct k_msgq msgq;
Expand All @@ -57,11 +74,13 @@ enum state {

static void state_sidewalk_run(void *o);
static void state_sidewalk_entry(void *o);
static void state_sidewalk_exit(void *o);
static void state_dfu_entry(void *o);
static void state_dfu_run(void *o);

static const struct smf_state sid_states[] = {
[STATE_SIDEWALK] = SMF_CREATE_STATE(state_sidewalk_entry, state_sidewalk_run, NULL),
[STATE_SIDEWALK] =
SMF_CREATE_STATE(state_sidewalk_entry, state_sidewalk_run, state_sidewalk_exit),
[STATE_DFU] = SMF_CREATE_STATE(state_dfu_entry, state_dfu_run, NULL),
};

Expand Down Expand Up @@ -283,8 +302,7 @@ static void state_sidewalk_run(void *o)
LOG_ERR("sid send err %d", (int)e);
}
LOG_DBG("sid send (type: %d, id: %u)", (int)p_msg->desc.type, p_msg->desc.id);
sid_hal_free(p_msg->msg.data);
sid_hal_free(p_msg);
sys_slist_append(&pending_message_list, &p_msg->node);
} break;
case SID_EVENT_CONNECT: {
if (!(sm->sid->config.link_mask & SID_LINK_TYPE_1)) {
Expand Down Expand Up @@ -345,6 +363,16 @@ static void state_sidewalk_run(void *o)
#endif /* CONFIG_SID_END_DEVICE_CLI */
}

static void state_sidewalk_exit(void *o)
{
while (sys_slist_is_empty(&pending_message_list) == false) {
sidewalk_msg_t *message;
message = SYS_SLIST_CONTAINER(sys_slist_get(&pending_message_list), message, node);
sid_hal_free(message->msg.data);
sid_hal_free(message);
}
}

static void state_dfu_entry(void *o)
{
sm_t *sm = (sm_t *)o;
Expand Down

0 comments on commit c08bec3

Please sign in to comment.