espressif/esp-modbus
1
0
Fork 1
mirror of https://github.com/espressif/esp-modbus.git synced 2026-08-03 20:24:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'bugfix/fix_tcp_frame_handling_issues' into 'main'

Browse Source 
Bugfix/fix tcp frame handling issues

See merge request idf/esp-modbus!184
This commit is contained in:
Alex Lisitsyn
2026-05-14 05:35:16 +01:00
parent 1bb83fef7f a4d6d85d5c
commit 5ef84de69b
6 changed files with 81 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modbus/mb_ports/common/port_other.c
+8 -1
View File
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -9,6 +9,7 @@
#include "sys/lock.h"
#include "port_common.h"
#include "mb_frame.h"
/* ----------------------- Variables ----------------------------------------*/
static _Atomic(uint32_t) inst_counter = 0;
@@ -75,6 +76,9 @@ esp_err_t queue_push(QueueHandle_t queue, void *buf, size_t len, frame_entry_t *
}
if (buf && (len > 0)) {
if (len > MB_TCP_BUFF_MAX_SIZE) {
return ESP_ERR_INVALID_ARG;
}
if (!frame_info.buf) {
frame_info.buf = calloc(1, len);
}
@@ -87,6 +91,9 @@ esp_err_t queue_push(QueueHandle_t queue, void *buf, size_t len, frame_entry_t *
// try send to queue and check if the queue is full
if (xQueueSend(queue, &frame_info, portMAX_DELAY) != pdTRUE) {
if (frame_info.buf) {
free(frame_info.buf);
}
return ESP_ERR_NO_MEM;
}
return ESP_OK;
modbus/mb_ports/tcp/port_tcp_driver.c
+7 -1
View File
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -351,6 +351,12 @@ ssize_t mb_drv_write(void *ctx, int fd, const void *data, size_t size)
if (size == 0) {
return 0;
}
if (size > MB_TCP_BUFF_MAX_SIZE) {
ESP_LOGE(TAG, "%p, mb_drv_write: size %u exceeds Modbus TCP frame max %d", ctx, (unsigned)size,
MB_TCP_BUFF_MAX_SIZE);
errno = EMSGSIZE;
return 0;
}
mb_node_info_t *node_ptr = drv_obj->mb_nodes[fd];
if (!node_ptr) {
modbus/mb_ports/tcp/port_tcp_master.c
+28 -8
View File
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -143,7 +143,7 @@ mb_err_enum_t mbm_port_tcp_create(mb_tcp_opts_t *tcp_opts, mb_port_base_t **port
while (*paddr_table) {
int res = port_scan_addr_string(*paddr_table, &slave_address_info);
if (res > 0) {
ESP_LOGD(TAG, "Config: %s, IP: %s, port: %d, slave_addr: %d, ip_ver: %s",
ESP_LOGD(TAG, "Config: %s, address: %s, port: %d, slave_addr: %d, ip_ver: %s",
(char *)*paddr_table, slave_address_info.ip_addr_str, slave_address_info.port,
slave_address_info.uid, (slave_address_info.addr_type == MB_IPV4 ? "IPV4" : "IPV6"));
fd = mb_drv_open(ptcp->drv_obj, slave_address_info, 0);
@@ -252,6 +252,11 @@ bool mbm_port_tcp_send_data(mb_port_base_t *inst, uint8_t address, uint8_t *fram
mbm_tcp_port_t *port_obj = __containerof(inst, mbm_tcp_port_t, base);
bool frame_sent = false;
if (!frame || length > MB_TCP_BUFF_MAX_SIZE || length < MB_TCP_FUNC) {
ESP_LOGE(TAG, "Invalid TCP send: frame %p, length %u (allowed %d..%d)", frame, (unsigned)length,
MB_TCP_FUNC, MB_TCP_BUFF_MAX_SIZE);
return false;
}
// get slave descriptor from its address
mb_node_info_t *info_ptr = mb_drv_get_node_info_from_addr(port_obj->drv_obj, address);
@@ -566,14 +571,26 @@ MB_EVENT_HANDLER(mbm_on_send_data)
ESP_LOGD(TAG, "%p, get info: %d, sock_id: %d, queue_state: %d, state: %d.",
ctx, (int)event_info->opt_fd, (int)info_ptr->sock_id,
(int)queue_is_empty(info_ptr->tx_queue), (int)MB_GET_NODE_STATE(info_ptr));
size_t sz = queue_pop(info_ptr->tx_queue, tx_buffer, sizeof(tx_buffer), NULL);
ssize_t popped = queue_pop(info_ptr->tx_queue, tx_buffer, sizeof(tx_buffer), NULL);
if (popped < 0) {
ESP_LOGE(TAG, "%p, "MB_NODE_FMT(", tx queue pop failed."),
ctx, (int)info_ptr->index, (int)info_ptr->sock_id, info_ptr->addr_info.ip_addr_str);
return;
}
size_t sz = (size_t)popped;
if (sz == 0 || sz > sizeof(tx_buffer)) {
ESP_LOGE(TAG, "%p, "MB_NODE_FMT(", bad tx frame size %u."),
ctx, (int)info_ptr->index, (int)info_ptr->sock_id, info_ptr->addr_info.ip_addr_str,
(unsigned)sz);
return;
}
if (MB_GET_NODE_STATE(info_ptr) < MB_SOCK_STATE_CONNECTED) {
// if slave is not connected, drop data.
ESP_LOGE(TAG, "%p, "MB_NODE_FMT(", is invalid, drop send data."),
ctx, (int)info_ptr->index, (int)info_ptr->sock_id, info_ptr->addr_info.ip_addr_str);
return;
}
int ret = port_write_poll(info_ptr, tx_buffer, sz, MB_TCP_SEND_TIMEOUT_MS);
int ret = port_write_poll(info_ptr, tx_buffer, (uint16_t)sz, MB_TCP_SEND_TIMEOUT_MS);
if (ret < 0) {
ESP_LOGE(TAG, "%p, "MB_NODE_FMT(", send data failure, err(errno) = %d(%u)."),
ctx, (int)info_ptr->index, (int)info_ptr->sock_id,
@@ -608,7 +625,6 @@ MB_EVENT_HANDLER(mbm_on_recv_data)
port_driver_t *drv_obj = MB_GET_DRV_PTR(ctx);
mb_event_info_t *event_info = (mb_event_info_t *)data;
ESP_LOGD(TAG, "%s %s: fd: %d", (char *)base, __func__, (int)event_info->opt_fd);
size_t sz = 0;
uint8_t buf[MB_TCP_BUFF_MAX_SIZE] = {0};
mb_drv_check_suspend_shutdown(ctx);
// Get frame from queue, check for correctness, push back correct frame and generate receive condition.
@@ -617,9 +633,13 @@ MB_EVENT_HANDLER(mbm_on_recv_data)
if (node_ptr) {
ESP_LOGD(TAG, "%p, slave #%d(%d) [%s], receive data ready.", ctx, (int)event_info->opt_fd,
(int)node_ptr->sock_id, node_ptr->addr_info.ip_addr_str);
while ((sz <= 0) && !queue_is_empty(node_ptr->rx_queue)) {
size_t sz = queue_pop(node_ptr->rx_queue, buf, MB_TCP_BUFF_MAX_SIZE, NULL);
if ((sz > MB_TCP_FUNC) && (sz < sizeof(buf))) {
while (!queue_is_empty(node_ptr->rx_queue)) {
ssize_t popped = queue_pop(node_ptr->rx_queue, buf, MB_TCP_BUFF_MAX_SIZE, NULL);
if (popped < 0) {
break;
}
size_t sz = (size_t)popped;
if ((sz > MB_TCP_FUNC) && (sz <= sizeof(buf))) {
uint16_t tid = MB_TCP_MBAP_GET_FIELD(buf, MB_TCP_TID);
ESP_LOGD(TAG, "%p, packet TID: 0x%04" PRIx16 " received.", ctx, tid);
if (tid == (node_ptr->tid_counter - 1)) {
modbus/mb_ports/tcp/port_tcp_slave.c
+1
View File
@@ -661,6 +661,7 @@ MB_EVENT_HANDLER(mbs_on_timeout)
} else {
curr_fd++;
}
vTaskDelay(1);
}
#endif
modbus/mb_ports/tcp/port_tcp_utils.c
+27 -12
View File
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -108,23 +108,29 @@ int port_enqueue_packet(QueueHandle_t queue, uint8_t *buf, uint16_t len)
esp_err_t ret = ESP_ERR_INVALID_STATE;
if (queue && buf) {
if (len > MB_TCP_BUFF_MAX_SIZE || len <= MB_TCP_UID) {
ESP_LOGE(TAG, "Enqueue: bad actual length %u (max %d)", (unsigned)len, MB_TCP_BUFF_MAX_SIZE);
return ERR_BUF;
}
frame_info.tid = MB_TCP_MBAP_GET_FIELD(buf, MB_TCP_TID);
frame_info.uid = buf[MB_TCP_UID];
frame_info.pid = MB_TCP_MBAP_GET_FIELD(buf, MB_TCP_PID);
frame_info.len = MB_TCP_MBAP_GET_FIELD(buf, MB_TCP_LEN) + MB_TCP_UID;
if (len != frame_info.len) {
ESP_LOGE(TAG, "Packet TID (%x), length in frame %u != %u expected.", frame_info.tid, frame_info.len, len);
uint16_t derived_len = MB_TCP_MBAP_GET_FIELD(buf, MB_TCP_LEN) + MB_TCP_UID;
if (derived_len != len) {
// Header can disagree with actual bytes (truncated read, clamp, or bad peer).
ESP_LOGW(TAG, "Packet TID 0x%x: MBAP len %u != actual %u, using actual.",
(unsigned)frame_info.tid, (unsigned)derived_len, (unsigned)len);
}
assert(xPortGetFreeHeapSize() > frame_info.len);
frame_info.len = len;
ret = queue_push(queue, buf, frame_info.len, &frame_info);
ret = queue_push(queue, buf, len, &frame_info);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "Packet TID (%x), data enqueue failed.", frame_info.tid);
// The packet send fail or the task which is waiting for event is already unblocked
return ERR_BUF;
}
ESP_LOGD(TAG, "Enqueue data, length=%d, TID=0x%" PRIx16, frame_info.len, frame_info.tid);
return (int)frame_info.len;
ESP_LOGD(TAG, "Enqueue data, length=%d, TID=0x%" PRIx16, (int)len, frame_info.tid);
return (int)len;
}
ESP_LOGE(TAG, "Enqueue data fail, %p, length=%d.", buf, len);
return ERR_BUF;
@@ -216,12 +222,15 @@ int port_read_packet(mb_node_info_t *info_ptr)
// If we have received the MBAP header we can analyze it and calculate
// the number of bytes left to complete the current response.
// Second chunk is stored from ptemp_buf[MB_TCP_UID], so temp must be
// <= (MB_TCP_BUFF_MAX_SIZE - MB_TCP_UID) to fit in ptemp_buf[].
temp = MB_TCP_MBAP_GET_FIELD(ptemp_buf, MB_TCP_LEN);
if (temp > MB_TCP_BUFF_MAX_SIZE) {
ESP_LOGD(TAG, "Incorrect packet length: %d", temp);
const uint16_t mbap_payload_max = (uint16_t)(MB_TCP_BUFF_MAX_SIZE - MB_TCP_UID);
if (temp > mbap_payload_max) {
ESP_LOGD(TAG, "Incorrect packet length: %u (max %u)", (unsigned)temp, (unsigned)mbap_payload_max);
ESP_LOG_BUFFER_HEX_LEVEL(TAG, ptemp_buf, MB_TCP_FUNC, ESP_LOG_DEBUG);
info_ptr->recv_err = ERR_BUF;
temp = MB_TCP_BUFF_MAX_SIZE; // read all remaining data from buffer
temp = mbap_payload_max; // read all remaining data from buffer
}
ret = port_get_buf(info_ptr, &ptemp_buf[MB_TCP_UID], temp, MB_READ_TICK);
@@ -230,7 +239,7 @@ int port_read_packet(mb_node_info_t *info_ptr)
return ret;
}
if (ret != temp) {
if ((ret < temp) || (ret < MB_PDU_SIZE_MIN)) {
info_ptr->recv_err = ERR_VAL;
return ERR_VAL;
}
@@ -458,6 +467,12 @@ err_t port_connect(void *ctx, mb_node_info_t *info_ptr)
int port_write_poll(mb_node_info_t *info_ptr, const uint8_t *frame, uint16_t frame_len, uint32_t timeout)
{
if (frame_len > MB_TCP_BUFF_MAX_SIZE) {
ESP_LOGE(TAG, MB_NODE_FMT(", refuse send: length %u > max %d"),
info_ptr->index, info_ptr->sock_id, info_ptr->addr_info.ip_addr_str,
(unsigned)frame_len, MB_TCP_BUFF_MAX_SIZE);
return -1;
}
// Check if the socket is alive (writable and SO_ERROR == 0)
int res = (int)port_check_alive(info_ptr, timeout);
if ((res < 0) && (res != ERR_INPROGRESS)) {
modbus/mb_transports/tcp/tcp_master.c
+10 -2
View File
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -156,8 +156,16 @@ static mb_err_enum_t mbm_tcp_transp_receive(mb_trans_base_t *inst, uint8_t *rcv_
static mb_err_enum_t mbm_tcp_transp_send(mb_trans_base_t *inst, uint8_t address, const uint8_t *frame, uint16_t len)
{
mb_err_enum_t status = MB_ENOERR;
if (len > MB_PDU_SIZE_MAX) {
ESP_LOGE(TAG, "PDU length %u exceeds max %u", (unsigned)len, (unsigned)MB_PDU_SIZE_MAX);
return MB_EIO;
}
uint16_t tcp_len = (uint16_t)(len + MB_TCP_FUNC);
if (tcp_len > MB_TCP_BUFF_MAX_SIZE) {
ESP_LOGE(TAG, "TCP frame length %u exceeds max %u", (unsigned)tcp_len, (unsigned)MB_TCP_BUFF_MAX_SIZE);
return MB_EIO;
}
uint8_t *frame_ptr = (uint8_t *)frame - MB_TCP_FUNC;
uint16_t tcp_len = len + MB_TCP_FUNC;
/* The MBAP header is already initialized because the caller calls this
* function with the buffer returned by the previous call. Therefore we