From bff6a5cce5083eba23b227fa224a6243e63b4bc6 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:41:38 +0200 Subject: [PATCH] net: l2: ieee802154: fix short/ext address endianness This changes fixes several bugs and inconsistencies in the IEEE 802.15.4 L2 implementation. These bugs were revealed while documenting intended endianness of driver, IP, socket and L2 attributes (see previous changes). Signed-off-by: Florian Grandel --- subsys/net/l2/ieee802154/ieee802154.c | 11 +++++++- subsys/net/l2/ieee802154/ieee802154_frame.c | 25 ++++++++++++++--- subsys/net/l2/ieee802154/ieee802154_mgmt.c | 15 ++++++---- subsys/net/l2/ieee802154/ieee802154_shell.c | 28 +++++++++---------- tests/net/ieee802154/l2/src/ieee802154_test.c | 5 +++- 5 files changed, 59 insertions(+), 25 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 7a695809b9e..96b2fc94f3a 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -124,6 +124,15 @@ static inline void set_pkt_ll_addr(struct net_linkaddr *addr, bool comp, addr->len = 0U; addr->addr = NULL; } + + /* Swap address byte order in place from little to big endian. + * This is ok as the ll address field comes from the header + * part of the packet buffer which will not be directly accessible + * once the packet reaches the upper layers. + */ + if (addr->len > 0) { + sys_mem_swap(addr->addr, addr->len); + } } /** @@ -163,7 +172,7 @@ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_m * address. */ if (!(dst_plain->addr.short_addr == IEEE802154_BROADCAST_ADDRESS || - dst_plain->addr.short_addr == ctx->short_addr)) { + dst_plain->addr.short_addr == sys_cpu_to_le16(ctx->short_addr))) { LOG_DBG("Frame dst address (short) does not match!"); return false; } diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index 68d4168368f..ae2641e4722 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -249,6 +249,7 @@ static inline bool validate_mac_command_cfi_to_mhr(struct ieee802154_mhr *mhr, u /* This should be set only when comp == 0 */ if (dst_brdcst_chk) { + /* broadcast address is symmetric so no need to swap byte order */ if (mhr->dst_addr->plain.addr.short_addr != IEEE802154_BROADCAST_ADDRESS) { return false; } @@ -535,7 +536,7 @@ static inline enum ieee802154_addressing_mode get_dst_addr_mode(struct net_linka if (dst->len == IEEE802154_SHORT_ADDR_LENGTH) { uint16_t short_addr = ntohs(*(uint16_t *)(dst->addr)); - *broadcast = (short_addr == 0xffff); + *broadcast = (short_addr == IEEE802154_BROADCAST_ADDRESS); return IEEE802154_ADDR_MODE_SHORT; } else { *broadcast = false; @@ -674,6 +675,20 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin params.pan_id = ctx->pan_id; if (src->addr && src->len == IEEE802154_SHORT_ADDR_LENGTH) { params.short_addr = ntohs(*(uint16_t *)(src->addr)); + if (ctx->short_addr != params.short_addr) { + return false; + } + } else { + if (src->len != IEEE802154_EXT_ADDR_LENGTH) { + return false; + } + + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(ext_addr_le, src->addr, IEEE802154_EXT_ADDR_LENGTH); + if (memcmp(ctx->ext_addr, ext_addr_le, src->len)) { + return false; + } } broadcast = data_addr_to_fs_settings(dst, fs, ¶ms); @@ -767,7 +782,7 @@ static inline bool cfi_to_fs_settings(enum ieee802154_cfi cfi, struct ieee802154 break; case IEEE802154_CFI_DATA_REQUEST: fs->fc.ar = 1U; - /* TODO: src/dst addr mode: see 5.3.4 */ + /* TODO: src/dst addr mode: see section 7.3.4 */ break; case IEEE802154_CFI_ORPHAN_NOTIFICATION: @@ -782,7 +797,7 @@ static inline bool cfi_to_fs_settings(enum ieee802154_cfi cfi, struct ieee802154 break; case IEEE802154_CFI_COORDINATOR_REALIGNEMENT: fs->fc.src_addr_mode = IEEE802154_ADDR_MODE_EXTENDED; - /* Todo: ar and dst addr mode: see 5.3.8 */ + /* TODO: ar and dst addr mode: see section 7.3.8 */ break; case IEEE802154_CFI_GTS_REQUEST: @@ -927,9 +942,11 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, uint8_t tag_size = level_2_tag_size[level]; uint8_t hdr_len = (uint8_t *)mpdu->payload - net_pkt_data(pkt); uint8_t payload_len = net_pkt_get_len(pkt) - hdr_len - tag_size; + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + sys_memcpy_swap(ext_addr_le, net_pkt_lladdr_src(pkt)->addr, IEEE802154_EXT_ADDR_LENGTH); if (!ieee802154_decrypt_auth(&ctx->sec_ctx, net_pkt_data(pkt), hdr_len, payload_len, - tag_size, net_pkt_lladdr_src(pkt)->addr, + tag_size, ext_addr_le, sys_le32_to_cpu(mpdu->mhr.aux_sec->frame_counter))) { NET_ERR("Could not decipher the frame"); return false; diff --git a/subsys/net/l2/ieee802154/ieee802154_mgmt.c b/subsys/net/l2/ieee802154/ieee802154_mgmt.c index b57265749a6..bab1b862819 100644 --- a/subsys/net/l2/ieee802154/ieee802154_mgmt.c +++ b/subsys/net/l2/ieee802154/ieee802154_mgmt.c @@ -46,7 +46,7 @@ enum net_verdict ieee802154_handle_beacon(struct net_if *iface, if (mpdu->mhr.fs->fc.src_addr_mode == IEEE802154_ADDR_MODE_SHORT) { ctx->scan_ctx->len = IEEE802154_SHORT_ADDR_LENGTH; ctx->scan_ctx->short_addr = - mpdu->mhr.src_addr->plain.addr.short_addr; + sys_le16_to_cpu(mpdu->mhr.src_addr->plain.addr.short_addr); } else { ctx->scan_ctx->len = IEEE802154_EXT_ADDR_LENGTH; sys_memcpy_swap(ctx->scan_ctx->addr, @@ -320,7 +320,9 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, if (params.dst.len == IEEE802154_SHORT_ADDR_LENGTH) { params.dst.short_addr = ctx->coord.short_addr; } else { - params.dst.ext_addr = ctx->coord.ext_addr; + params.dst.len = IEEE802154_EXT_ADDR_LENGTH; + sys_memcpy_swap(params.dst.ext_addr, ctx->coord_ext_addr, + IEEE802154_EXT_ADDR_LENGTH); } params.pan_id = ctx->pan_id; @@ -413,8 +415,11 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, return -EINVAL; } - if (memcmp(ctx->ext_addr, data, IEEE802154_EXT_ADDR_LENGTH)) { - memcpy(ctx->ext_addr, data, IEEE802154_EXT_ADDR_LENGTH); + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(ext_addr_le, data, IEEE802154_EXT_ADDR_LENGTH); + if (memcmp(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH)) { + memcpy(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH); ieee802154_filter_ieee_addr(iface, ctx->ext_addr); } } else if (mgmt_request == NET_REQUEST_IEEE802154_SET_SHORT_ADDR) { @@ -472,7 +477,7 @@ static int ieee802154_get_parameters(uint32_t mgmt_request, return -EINVAL; } - memcpy(data, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); + sys_memcpy_swap(data, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_SHORT_ADDR) { *value = ctx->short_addr; } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_TX_POWER) { diff --git a/subsys/net/l2/ieee802154/ieee802154_shell.c b/subsys/net/l2/ieee802154/ieee802154_shell.c index 77648a0a0bf..b9b1791b428 100644 --- a/subsys/net/l2/ieee802154/ieee802154_shell.c +++ b/subsys/net/l2/ieee802154/ieee802154_shell.c @@ -22,7 +22,7 @@ LOG_MODULE_REGISTER(net_ieee802154_shell, CONFIG_NET_L2_IEEE802154_LOG_LEVEL); #include "ieee802154_frame.h" -#define MAX_EXT_ADDR_STR_LEN sizeof("xx:xx:xx:xx:xx:xx:xx:xx") +#define EXT_ADDR_STR_LEN sizeof("xx:xx:xx:xx:xx:xx:xx:xx") struct ieee802154_req_params params; static struct net_mgmt_event_callback scan_cb; @@ -76,7 +76,7 @@ static int cmd_ieee802154_associate(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - char ext_addr[MAX_EXT_ADDR_STR_LEN]; + char ext_addr[EXT_ADDR_STR_LEN]; if (argc < 3) { shell_help(shell); @@ -90,9 +90,9 @@ static int cmd_ieee802154_associate(const struct shell *shell, } params.pan_id = atoi(argv[1]); - strncpy(ext_addr, argv[2], MAX_EXT_ADDR_STR_LEN - 1); + strncpy(ext_addr, argv[2], EXT_ADDR_STR_LEN - 1); - if (strlen(ext_addr) == MAX_EXT_ADDR_STR_LEN) { + if (strlen(ext_addr) == EXT_ADDR_STR_LEN) { parse_extended_address(ext_addr, params.addr); params.len = IEEE802154_EXT_ADDR_LENGTH; } else { @@ -182,7 +182,7 @@ static inline char *print_coordinator_address(char *buf, int buf_len) pos += snprintk(buf + pos, buf_len - pos, "(extended) "); - for (i = IEEE802154_EXT_ADDR_LENGTH - 1; i > -1; i--) { + for (i = 0; i < IEEE802154_EXT_ADDR_LENGTH; i++) { pos += snprintk(buf + pos, buf_len - pos, "%02X:", params.addr[i]); } @@ -421,9 +421,9 @@ static int cmd_ieee802154_set_ext_addr(const struct shell *shell, return -ENOEXEC; } - if (strlen(argv[1]) != MAX_EXT_ADDR_STR_LEN) { + if (strlen(argv[1]) != EXT_ADDR_STR_LEN) { shell_fprintf(shell, SHELL_INFO, - "%zd characters needed\n", MAX_EXT_ADDR_STR_LEN); + "%zd characters needed\n", EXT_ADDR_STR_LEN); return -ENOEXEC; } @@ -461,16 +461,16 @@ static int cmd_ieee802154_get_ext_addr(const struct shell *shell, "Could not get extended address\n"); return -ENOEXEC; } else { - static char ext_addr[MAX_EXT_ADDR_STR_LEN]; - int i, j; + static char ext_addr[EXT_ADDR_STR_LEN]; + int i, pos = 0; - for (j = 0, i = IEEE802154_EXT_ADDR_LENGTH - 1; i > -1; i--) { - snprintf(&ext_addr[j], 4, "%02X:", addr[i]); - - j += 3; + for (i = 0; i < IEEE802154_EXT_ADDR_LENGTH; i++) { + pos += snprintk(ext_addr + pos, + IEEE802154_EXT_ADDR_LENGTH - pos, + "%02X:", addr[i]); } - ext_addr[MAX_EXT_ADDR_STR_LEN - 1] = '\0'; + ext_addr[EXT_ADDR_STR_LEN - 1] = '\0'; shell_fprintf(shell, SHELL_NORMAL, "Extended address: %s\n", ext_addr); diff --git a/tests/net/ieee802154/l2/src/ieee802154_test.c b/tests/net/ieee802154/l2/src/ieee802154_test.c index 7c030a138d0..cf027015b2f 100644 --- a/tests/net/ieee802154/l2/src/ieee802154_test.c +++ b/tests/net/ieee802154/l2/src/ieee802154_test.c @@ -264,7 +264,10 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu goto release_frag; } - net_pkt_lladdr_src(current_pkt)->addr = ctx->ext_addr; + uint8_t lladdr_be[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(lladdr_be, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); + net_pkt_lladdr_src(current_pkt)->addr = lladdr_be; net_pkt_lladdr_src(current_pkt)->len = IEEE802154_MAX_ADDR_LENGTH; if (!ieee802154_decipher_data_frame(iface, current_pkt, &mpdu)) { NET_ERR("*** Cannot decipher/authenticate packet\n");