From 066c142164a70b7179ef1bdec7a6242950469c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Afonso=20Janu=C3=A1rio?= Date: Mon, 14 Sep 2026 01:04:35 +0100 Subject: [PATCH] Fix exception function code wraparound when function >= 0x80 response_exception() builds the exception function byte with plain integer addition (function + 0x80). modbus_reply()'s default case routes any unrecognized function code there, including raw bytes from 0x80 to 0xFF, so the addition can carry past 0xFF. Once that gets truncated to a single byte for the wire in build_response_basis, the result wraps back below 0x80 and the response looks like an ordinary function code instead of an exception. Switch to a bitwise OR, which matches the Modbus spec (set the top bit) and gives the same result as the addition whenever the top bit was already clear. Adds a client-side regression test alongside the existing invalid function code test, using a request byte that already has the top bit set. --- src/modbus.c | 13 +++++++++++-- tests/unit-test-client.c | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/modbus.c b/src/modbus.c index abdd718c4..08315a67c 100644 --- a/src/modbus.c +++ b/src/modbus.c @@ -785,8 +785,17 @@ static int response_exception(modbus_t *ctx, modbus_flush(ctx); } - /* Build exception response */ - sft->function = sft->function + 0x80; + /* Build exception response. Per the Modbus spec the exception function + code is formed by setting the top bit of the request's function code. + An unrecognized function code can already have that bit set (any raw + byte from 0x80 to 0xFF ends up here through the "default" case in + modbus_reply's switch), so adding 0x80 can carry past 0xFF and the + truncation to a single byte in build_response_basis then wraps the + result back below 0x80. That leaves the response looking like a + plain, non-exception function code to the client. A bitwise OR gives + the same result as the addition when the top bit was clear and stays + correct when it was already set. */ + sft->function = sft->function | 0x80; rsp_length = ctx->backend->build_response_basis(sft, rsp); rsp[rsp_length++] = exception_code; diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index c1542e971..d38675039 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -868,6 +868,10 @@ int test_server(modbus_t *ctx, int use_backend) const int INVALID_FC = 0x42; const int INVALID_FC_REQ_LEN = 6; uint8_t invalid_fc_raw_req[] = {slave, 0x42, 0x00, 0x00, 0x00, 0x00}; + /* Same idea, but the function byte already has its top bit set, as a + real function code >= 0x80 would. */ + const int INVALID_FC_HIGH = 0xFF; + uint8_t invalid_fc_high_raw_req[] = {slave, 0xFF, 0x00, 0x00, 0x00, 0x00}; int req_length; uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH]; @@ -999,6 +1003,22 @@ int test_server(modbus_t *ctx, int use_backend) rsp[backend_offset] == (0x80 + INVALID_FC), "") + /* Same test with a function code that already has its top bit set. The + server used to build the exception response by adding 0x80 to the + function code with plain arithmetic, so a request byte >= 0x80 made + the sum carry past 0xFF; once that got truncated to a single byte for + the wire, the result wrapped back below 0x80 and looked like a + perfectly normal function code to the client instead of an + exception. */ + modbus_send_raw_request( + ctx, invalid_fc_high_raw_req, INVALID_FC_REQ_LEN * sizeof(uint8_t)); + rc = modbus_receive_confirmation(ctx, rsp); + printf("Return an exception on unknown function code with the top bit " + "already set: "); + ASSERT_TRUE(rc == (backend_length + EXCEPTION_RC) && + rsp[backend_offset] == (0x80 | INVALID_FC_HIGH), + "") + modbus_set_response_timeout(ctx, old_response_to_sec, old_response_to_usec); return 0; close: