From 73ee75635bb4c9e3b390d0fe0c1518b5ca36acc9 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Wed, 27 Dec 2023 10:29:42 -0600 Subject: [PATCH] Changed npdu_encode function to return length when given a NULL buffer. (#549) * Changed npdu_encode function to return length when given a NULL buffer. * reduce TSM dependency in NPDU handler and use local buffer * add bacnet API with additional size of PDU argument. --------- Co-authored-by: Steve Karg --- src/bacnet/basic/npdu/h_npdu.c | 29 +++-- src/bacnet/npdu.c | 216 ++++++++++++++++++++++----------- src/bacnet/npdu.h | 8 ++ test/bacnet/npdu/src/main.c | 33 +++-- 4 files changed, 192 insertions(+), 94 deletions(-) diff --git a/src/bacnet/basic/npdu/h_npdu.c b/src/bacnet/basic/npdu/h_npdu.c index 8b5bd83e..1013facf 100644 --- a/src/bacnet/basic/npdu/h_npdu.c +++ b/src/bacnet/basic/npdu/h_npdu.c @@ -34,7 +34,6 @@ #include "bacnet/apdu.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/debug.h" -#include "bacnet/basic/tsm/tsm.h" #include "bacnet/datalink/datalink.h" #if PRINT_ENABLED @@ -76,8 +75,9 @@ int npdu_send_network_number_is( int pdu_len = 0; int bytes_sent = 0; bool data_expecting_reply = false; - BACNET_NPDU_DATA npdu_data; - BACNET_ADDRESS my_address; + BACNET_NPDU_DATA npdu_data = { 0 }; + BACNET_ADDRESS my_address = { 0 }; + uint8_t pdu[MAX_NPDU + 2 + 1] = { 0 }; /* Upon receipt of a What-Is-Network-Number message, a device that knows the local network number shall @@ -86,14 +86,14 @@ int npdu_send_network_number_is( datalink_get_my_address(&my_address); npdu_encode_npdu_network(&npdu_data, NETWORK_MESSAGE_NETWORK_NUMBER_IS, data_expecting_reply, MESSAGE_PRIORITY_NORMAL); - pdu_len = npdu_encode_pdu( - &Handler_Transmit_Buffer[0], dst, &my_address, &npdu_data); - len = encode_unsigned16(&Handler_Transmit_Buffer[pdu_len], net); - pdu_len += len; - Handler_Transmit_Buffer[pdu_len] = status; - pdu_len++; - bytes_sent = datalink_send_pdu( - dst, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len); + pdu_len = npdu_encode_pdu(pdu, dst, &my_address, &npdu_data); + if ((pdu_len > 0) && (pdu_len <= MAX_NPDU)) { + len = encode_unsigned16(&pdu[pdu_len], net); + pdu_len += len; + pdu[pdu_len] = status; + pdu_len++; + bytes_sent = datalink_send_pdu(dst, &npdu_data, pdu, pdu_len); + } return bytes_sent; } @@ -111,6 +111,7 @@ int npdu_send_what_is_network_number(BACNET_ADDRESS *dst) BACNET_NPDU_DATA npdu_data; BACNET_ADDRESS daddr = { 0 }; BACNET_ADDRESS saddr = { 0 }; + uint8_t pdu[MAX_NPDU] = { 0 }; if (dst) { bacnet_address_copy(&daddr, dst); @@ -120,12 +121,10 @@ int npdu_send_what_is_network_number(BACNET_ADDRESS *dst) datalink_get_my_address(&saddr); npdu_encode_npdu_network(&npdu_data, NETWORK_MESSAGE_WHAT_IS_NETWORK_NUMBER, data_expecting_reply, MESSAGE_PRIORITY_NORMAL); - pdu_len = npdu_encode_pdu( - &Handler_Transmit_Buffer[0], &daddr, &saddr, &npdu_data); + pdu_len = npdu_encode_pdu(pdu, &daddr, &saddr, &npdu_data); /* Now send the message */ - return datalink_send_pdu( - dst, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len); + return datalink_send_pdu(dst, &npdu_data, pdu, pdu_len); } /** @file h_npdu.c Handles messages at the NPDU level of the BACnet stack. */ diff --git a/src/bacnet/npdu.c b/src/bacnet/npdu.c index b2de713e..3d5b36b2 100644 --- a/src/bacnet/npdu.c +++ b/src/bacnet/npdu.c @@ -107,24 +107,21 @@ ABORT.indication Yes Yes Yes No * The Network Layer Protocol Control Information byte is described * in section 6.2.2 of the BACnet standard. * @param npdu [out] Buffer which will hold the encoded NPDU header bytes. - * The size isn't given, but it must be at - * least 2 bytes for the simplest case, and should always be at least 24 bytes - * to accommodate the maximal case (all fields loaded). + * The size isn't given, but it must be at least 2 bytes for the simplest + * case, and should always be at least 24 bytes to accommodate the maximal + * case (all fields loaded). If the buffer is NULL, the number of bytes + * the buffer would have held is returned. * @param dest [in] The routing destination information if the message must - * be routed to reach its destination. - * If dest->net and dest->len are 0, there is no - * routing destination information. + * be routed to reach its destination. If dest->net and dest->len are 0, + * there is no routing destination information. * @param src [in] The routing source information if the message was routed - * from another BACnet network. - * If src->net and src->len are 0, there is no - * routing source information. - * This src describes the original source of the message when - * it had to be routed to reach this BACnet Device. + * from another BACnet network. If src->net and src->len are 0, there is no + * routing source information. This src describes the original source of the + * message when it had to be routed to reach this BACnet Device. * @param npdu_data [in] The structure which describes how the NCPI and other - * NPDU bytes should be encoded. - * @return On success, returns the number of bytes which were encoded into the - * NPDU section. - * If 0 or negative, there were problems with the data or encoding. + * NPDU bytes should be encoded. + * @return On success, returns the number of bytes which were encoded into + * the NPDU section, or 0 if there were problems with the data or encoding. */ int npdu_encode_pdu(uint8_t *npdu, BACNET_ADDRESS *dest, @@ -134,73 +131,103 @@ int npdu_encode_pdu(uint8_t *npdu, int len = 0; /* return value - number of octets loaded in this function */ uint8_t i = 0; /* counter */ - if (npdu && npdu_data) { + if (npdu_data) { /* protocol version */ - npdu[0] = npdu_data->protocol_version; + if (npdu) { + npdu[0] = npdu_data->protocol_version; + } /* initialize the control octet */ - npdu[1] = 0; - /* Bit 7: 1 indicates that the NSDU conveys a network layer message. */ - /* Message Type field is present. */ - /* 0 indicates that the NSDU contains a BACnet APDU. */ - /* Message Type field is absent. */ - if (npdu_data->network_layer_message) { - npdu[1] |= BIT(7); + if (npdu) { + npdu[1] = 0; + /* Bit 7: 1 indicates that the NSDU conveys a network layer message. + */ + /* Message Type field is present. */ + /* 0 indicates that the NSDU contains a BACnet APDU. */ + /* Message Type field is absent. */ + if (npdu_data->network_layer_message) { + npdu[1] |= BIT(7); + } + /*Bit 6: Reserved. Shall be zero. */ + /*Bit 5: Destination specifier where: */ + /* 0 = DNET, DLEN, DADR, and Hop Count absent */ + /* 1 = DNET, DLEN, and Hop Count present */ + /* DLEN = 0 denotes broadcast MAC DADR and DADR field is absent */ + /* DLEN > 0 specifies length of DADR field */ + if (dest && dest->net) { + npdu[1] |= BIT(5); + } + /* Bit 4: Reserved. Shall be zero. */ + /* Bit 3: Source specifier where: */ + /* 0 = SNET, SLEN, and SADR absent */ + /* 1 = SNET, SLEN, and SADR present */ + /* SLEN = 0 Invalid */ + /* SLEN > 0 specifies length of SADR field */ + if (src && src->net && src->len) { + npdu[1] |= BIT(3); + } + /* Bit 2: The value of this bit corresponds to the */ + /* data_expecting_reply parameter in the N-UNITDATA primitives. */ + /* 1 indicates that a BACnet-Confirmed-Request-PDU, */ + /* a segment of a BACnet-ComplexACK-PDU, */ + /* or a network layer message expecting a reply is present. */ + /* 0 indicates that other than a BACnet-Confirmed-Request-PDU, */ + /* a segment of a BACnet-ComplexACK-PDU, */ + /* or a network layer message expecting a reply is present. */ + if (npdu_data->data_expecting_reply) { + npdu[1] |= BIT(2); + } + /* Bits 1,0: Network priority where: */ + /* B'11' = Life Safety message */ + /* B'10' = Critical Equipment message */ + /* B'01' = Urgent message */ + /* B'00' = Normal message */ + npdu[1] |= (npdu_data->priority & 0x03); } - /*Bit 6: Reserved. Shall be zero. */ - /*Bit 5: Destination specifier where: */ - /* 0 = DNET, DLEN, DADR, and Hop Count absent */ - /* 1 = DNET, DLEN, and Hop Count present */ - /* DLEN = 0 denotes broadcast MAC DADR and DADR field is absent */ - /* DLEN > 0 specifies length of DADR field */ - if (dest && dest->net) { - npdu[1] |= BIT(5); - } - /* Bit 4: Reserved. Shall be zero. */ - /* Bit 3: Source specifier where: */ - /* 0 = SNET, SLEN, and SADR absent */ - /* 1 = SNET, SLEN, and SADR present */ - /* SLEN = 0 Invalid */ - /* SLEN > 0 specifies length of SADR field */ - if (src && src->net && src->len) { - npdu[1] |= BIT(3); - } - /* Bit 2: The value of this bit corresponds to the */ - /* data_expecting_reply parameter in the N-UNITDATA primitives. */ - /* 1 indicates that a BACnet-Confirmed-Request-PDU, */ - /* a segment of a BACnet-ComplexACK-PDU, */ - /* or a network layer message expecting a reply is present. */ - /* 0 indicates that other than a BACnet-Confirmed-Request-PDU, */ - /* a segment of a BACnet-ComplexACK-PDU, */ - /* or a network layer message expecting a reply is present. */ - if (npdu_data->data_expecting_reply) { - npdu[1] |= BIT(2); - } - /* Bits 1,0: Network priority where: */ - /* B'11' = Life Safety message */ - /* B'10' = Critical Equipment message */ - /* B'01' = Urgent message */ - /* B'00' = Normal message */ - npdu[1] |= (npdu_data->priority & 0x03); len = 2; if (dest && dest->net) { - len += encode_unsigned16(&npdu[len], dest->net); - npdu[len++] = dest->len; + if (npdu) { + encode_unsigned16(&npdu[len], dest->net); + } + len += 2; + if (dest->len > MAX_MAC_LEN) { + dest->len = MAX_MAC_LEN; + } + if (npdu) { + npdu[len] = dest->len; + } + len++; /* DLEN = 0 denotes broadcast MAC DADR and DADR field is absent */ /* DLEN > 0 specifies length of DADR field */ if (dest->len) { for (i = 0; i < dest->len; i++) { - npdu[len++] = dest->adr[i]; + if (npdu) { + npdu[len] = dest->adr[i]; + } + len++; } } } - if (src && src->net && src->len) { /* Only insert if valid */ - len += encode_unsigned16(&npdu[len], src->net); - npdu[len++] = src->len; + if (src && src->net && src->len) { + /* Only insert if valid */ + if (npdu) { + encode_unsigned16(&npdu[len], src->net); + } + len += 2; + if (src->len > MAX_MAC_LEN) { + src->len = MAX_MAC_LEN; + } + if (npdu) { + npdu[len] = src->len; + } + len++; /* SLEN = 0 denotes broadcast MAC SADR and SADR field is absent */ /* SLEN > 0 specifies length of SADR field */ if (src->len) { for (i = 0; i < src->len; i++) { - npdu[len++] = src->adr[i]; + if (npdu) { + npdu[len] = src->adr[i]; + } + len++; } } } @@ -208,16 +235,23 @@ int npdu_encode_pdu(uint8_t *npdu, /* destined for a remote network, i.e., if DNET is present. */ /* This is a one-octet field that is initialized to a value of 0xff. */ if (dest && dest->net) { - npdu[len] = npdu_data->hop_count; + if (npdu) { + npdu[len] = npdu_data->hop_count; + } len++; } if (npdu_data->network_layer_message) { - npdu[len] = npdu_data->network_message_type; + if (npdu) { + npdu[len] = npdu_data->network_message_type; + } len++; /* Message Type field contains a value in the range 0x80 - 0xFF, */ /* then a Vendor ID field shall be present */ if (npdu_data->network_message_type >= 0x80) { - len += encode_unsigned16(&npdu[len], npdu_data->vendor_id); + if (npdu) { + encode_unsigned16(&npdu[len], npdu_data->vendor_id); + } + len += 2; } } } @@ -225,6 +259,50 @@ int npdu_encode_pdu(uint8_t *npdu, return len; } +/** + * @brief Encode the NPDU portion of a message to be sent + * based on the npdu_data and associated data. + * If this is to be a Network Layer Control Message, there are probably + * more bytes which will need to be encoded following the ones encoded here. + * The Network Layer Protocol Control Information byte is described + * in section 6.2.2 of the BACnet standard. + * @param pdu [out] Buffer which will hold the encoded NPDU header bytes. + * If pdu is NULL, the number of bytes the buffer would have held + * is returned. + * @param pdu_size Number of bytes in the buffer to hold the encoded data. + * If the size is zero, the number of bytes the buffer would have held + * is returned. + * The size isn't given, but it must be at least 2 bytes for the simplest + * case, and should always be at least 24 bytes to accommodate the maximal + * case (all fields loaded). Can be NULL to determine length of buffer. + * @param dest [in] The routing destination information if the message must + * be routed to reach its destination. If dest->net and dest->len are 0, + * there is no routing destination information. + * @param src [in] The routing source information if the message was routed + * from another BACnet network. If src->net and src->len are 0, there is no + * routing source information. This src describes the original source of the + * message when it had to be routed to reach this BACnet Device. + * @param npdu_data [in] The structure which describes how the NCPI and other + * NPDU bytes should be encoded. + * @return On success, returns the number of bytes which were encoded into + * the NPDU section, or 0 if there were problems with the data or encoding. + */ +int bacnet_npdu_encode_pdu(uint8_t *pdu, + uint16_t pdu_size, + BACNET_ADDRESS *dest, + BACNET_ADDRESS *src, + BACNET_NPDU_DATA *npdu_data) +{ + int pdu_len = 0; + + pdu_len = npdu_encode_pdu(NULL, dest, src, npdu_data); + if ((pdu != NULL) && (pdu_size > 0) && (pdu_len <= pdu_size)) { + pdu_len = npdu_encode_pdu(pdu, dest, src, npdu_data); + } + + return pdu_len; +} + /* Configure the NPDU portion of the packet for an APDU */ /* This function does not handle the network messages, just APDUs. */ /* From BACnet 5.1: diff --git a/src/bacnet/npdu.h b/src/bacnet/npdu.h index 484f2509..25c1e3f7 100644 --- a/src/bacnet/npdu.h +++ b/src/bacnet/npdu.h @@ -81,6 +81,14 @@ extern "C" { BACNET_ADDRESS * src, BACNET_NPDU_DATA * npdu_data); + BACNET_STACK_EXPORT + int bacnet_npdu_encode_pdu( + uint8_t * pdu, + uint16_t pdu_size, + BACNET_ADDRESS * dest, + BACNET_ADDRESS * src, + BACNET_NPDU_DATA * npdu_data); + BACNET_STACK_EXPORT void npdu_encode_npdu_data( BACNET_NPDU_DATA * npdu, diff --git a/test/bacnet/npdu/src/main.c b/test/bacnet/npdu/src/main.c index db0ceb94..b73ad6db 100644 --- a/test/bacnet/npdu/src/main.c +++ b/test/bacnet/npdu/src/main.c @@ -25,34 +25,47 @@ ZTEST(npdu_tests, test_NPDU_Network) static void test_NPDU_Network(void) #endif { - uint8_t pdu[480] = { 0 }; + uint8_t pdu[MAX_NPDU] = { 0 }; BACNET_ADDRESS dest = { 0 }; BACNET_ADDRESS src = { 0 }; BACNET_ADDRESS npdu_dest = { 0 }; BACNET_ADDRESS npdu_src = { 0 }; - int len = 0; + int len = 0, null_len = 0, test_len = 0; bool data_expecting_reply = true; BACNET_NETWORK_MESSAGE_TYPE network_message_type = NETWORK_MESSAGE_NETWORK_NUMBER_IS; BACNET_MESSAGE_PRIORITY priority = MESSAGE_PRIORITY_NORMAL; BACNET_NPDU_DATA npdu_data = { 0 }; - int npdu_len = 0; bool network_layer_message = true; uint16_t vendor_id = 0; /* optional, if net message type is > 0x80 */ npdu_encode_npdu_network(&npdu_data, network_message_type, data_expecting_reply, priority); - len = npdu_encode_pdu(&pdu[0], &dest, &src, &npdu_data); + null_len = bacnet_npdu_encode_pdu(NULL, 0, &dest, &src, &npdu_data); + len = bacnet_npdu_encode_pdu(&pdu[0], sizeof(pdu), &dest, &src, &npdu_data); + zassert_equal(len, null_len, NULL); zassert_not_equal(len, 0, NULL); /* can we get the info back? */ - npdu_len = bacnet_npdu_decode(pdu, sizeof(pdu), &npdu_dest, &npdu_src, &npdu_data); - zassert_not_equal(npdu_len, 0, NULL); + test_len = bacnet_npdu_decode(pdu, sizeof(pdu), &npdu_dest, &npdu_src, &npdu_data); + zassert_not_equal(test_len, 0, NULL); + zassert_equal(len, test_len, NULL); zassert_equal(npdu_data.data_expecting_reply, data_expecting_reply, NULL); zassert_equal(npdu_data.network_layer_message, network_layer_message, NULL); zassert_equal(npdu_data.network_message_type, network_message_type, NULL); zassert_equal(npdu_data.vendor_id, vendor_id, NULL); zassert_equal(npdu_data.priority, priority, NULL); + /* test for invalid short PDU */ + while (len) { + len--; + test_len = bacnet_npdu_decode(pdu, len, &npdu_dest, &npdu_src, &npdu_data); + if (len == 2) { + /* special case with no NPDU options */ + zassert_equal(len, test_len, NULL); + } else { + zassert_true(test_len <= 0, "len=%d test_len=%d\n", len, test_len); + } + } } /** @@ -64,7 +77,7 @@ ZTEST(npdu_tests, testNPDU2) static void testNPDU2(void) #endif { - uint8_t pdu[480] = { 0 }; + uint8_t pdu[MAX_NPDU] = { 0 }; BACNET_ADDRESS dest = { 0 }; BACNET_ADDRESS src = { 0 }; BACNET_ADDRESS npdu_dest = { 0 }; @@ -100,7 +113,7 @@ static void testNPDU2(void) src.adr[i] = 0x40; } npdu_encode_npdu_data(&npdu_data, true, priority); - len = npdu_encode_pdu(&pdu[0], &dest, &src, &npdu_data); + len = bacnet_npdu_encode_pdu(&pdu[0], sizeof(pdu), &dest, &src, &npdu_data); zassert_not_equal(len, 0, NULL); /* can we get the info back? */ npdu_len = bacnet_npdu_decode(pdu, sizeof(pdu), &npdu_dest, &npdu_src, &npdu_data); @@ -132,7 +145,7 @@ ZTEST(npdu_tests, testNPDU1) static void testNPDU1(void) #endif { - uint8_t pdu[480] = { 0 }; + uint8_t pdu[MAX_NPDU] = { 0 }; BACNET_ADDRESS dest = { 0 }; BACNET_ADDRESS src = { 0 }; BACNET_ADDRESS npdu_dest = { 0 }; @@ -169,7 +182,7 @@ static void testNPDU1(void) src.adr[i] = 0; } npdu_encode_npdu_data(&npdu_data, false, priority); - len = npdu_encode_pdu(&pdu[0], &dest, &src, &npdu_data); + len = bacnet_npdu_encode_pdu(&pdu[0], sizeof(pdu), &dest, &src, &npdu_data); zassert_not_equal(len, 0, NULL); /* can we get the info back? */ npdu_len = bacnet_npdu_decode(pdu, sizeof(pdu), &npdu_dest, &npdu_src, &npdu_data);