Fix: use modbus_free only after object is fully initialised...

fixes double-free crash in unit-test server/client
stephane · Jul 17, 2024 · fa74890 · fa74890
1 parent acacb7d
commit fa74890
Show file tree

Hide file tree

Showing 2 changed files with 254 additions and 21 deletions.
diff --git a/src/modbus-rtu.c b/src/modbus-rtu.c
@@ -72,7 +72,7 @@ static const uint8_t table_crc_lo[] = {
 
 /* Define the slave ID of the remote device to talk in master mode or set the
  * internal slave ID in slave mode */
-static int _modbus_set_slave(modbus_t *ctx, int slave)
+int _modbus_rtu_set_slave(modbus_t *ctx, int slave)
 {
     int max_slave = (ctx->quirks & MODBUS_QUIRK_MAX_SLAVE) ? 255 : 247;
 
@@ -88,7 +88,7 @@ static int _modbus_set_slave(modbus_t *ctx, int slave)
 }
 
 /* Builds a RTU request header */
-static int _modbus_rtu_build_request_basis(
+int _modbus_rtu_build_request_basis(
     modbus_t *ctx, int function, int addr, int nb, uint8_t *req)
 {
     assert(ctx->slave != -1);
@@ -103,7 +103,7 @@ static int _modbus_rtu_build_request_basis(
 }
 
 /* Builds a RTU response header */
-static int _modbus_rtu_build_response_basis(sft_t *sft, uint8_t *rsp)
+int _modbus_rtu_build_response_basis(sft_t *sft, uint8_t *rsp)
 {
     /* In this case, the slave is certainly valid because a check is already
      * done in _modbus_rtu_listen */
@@ -129,14 +129,14 @@ static uint16_t crc16(uint8_t *buffer, uint16_t buffer_length)
     return (crc_hi << 8 | crc_lo);
 }
 
-static int _modbus_rtu_prepare_response_tid(const uint8_t *req, int *req_length)
+int _modbus_rtu_prepare_response_tid(const uint8_t *req, int *req_length)
 {
     (*req_length) -= _MODBUS_RTU_CHECKSUM_LENGTH;
     /* No TID */
     return 0;
 }
 
-static int _modbus_rtu_send_msg_pre(uint8_t *req, int req_length)
+int _modbus_rtu_send_msg_pre(uint8_t *req, int req_length)
 {
     uint16_t crc = crc16(req, req_length);
 
@@ -322,10 +322,10 @@ static ssize_t _modbus_rtu_recv(modbus_t *ctx, uint8_t *rsp, int rsp_length)
 
 static int _modbus_rtu_flush(modbus_t *);
 
-static int _modbus_rtu_pre_check_confirmation(modbus_t *ctx,
-                                              const uint8_t *req,
-                                              const uint8_t *rsp,
-                                              int rsp_length)
+int _modbus_rtu_pre_check_confirmation(modbus_t *ctx,
+                                       const uint8_t *req,
+                                       const uint8_t *rsp,
+                                       int rsp_length)
 {
     /* Check responding slave is the slave we requested (except for broacast
      * request) */
@@ -346,7 +346,7 @@ static int _modbus_rtu_pre_check_confirmation(modbus_t *ctx,
 /* The check_crc16 function shall return 0 if the message is ignored and the
    message length if the CRC is valid. Otherwise it shall return -1 and set
    errno to EMBBADCRC. */
-static int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg, const int msg_length)
+int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg, const int msg_length)
 {
     uint16_t crc_calculated;
     uint16_t crc_received;
@@ -377,7 +377,7 @@ static int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg, const int ms
         }
 
         if (ctx->error_recovery & MODBUS_ERROR_RECOVERY_PROTOCOL) {
-            _modbus_rtu_flush(ctx);
+            modbus_flush(ctx);
         }
         errno = EMBBADCRC;
         return -1;
@@ -1184,7 +1184,7 @@ const modbus_backend_t _modbus_rtu_backend = {
     _MODBUS_RTU_HEADER_LENGTH,
     _MODBUS_RTU_CHECKSUM_LENGTH,
     MODBUS_RTU_MAX_ADU_LENGTH,
-    _modbus_set_slave,
+    _modbus_rtu_set_slave,
     _modbus_rtu_build_request_basis,
     _modbus_rtu_build_response_basis,
     _modbus_rtu_prepare_response_tid,
@@ -1219,15 +1219,23 @@ modbus_new_rtu(const char *device, int baud, char parity, int data_bit, int stop
     /* Check device argument */
     if (device == NULL || *device == 0) {
         modbus_trace_error(ctx, "The device string is empty\n");
+<<<<<<< Updated upstream
         modbus_free(ctx);
+=======
+        free(ctx);
+>>>>>>> Stashed changes
         errno = EINVAL;
         return NULL;
     }
 
     /* Check baud argument */
     if (baud == 0) {
         modbus_trace_error(ctx, "The baud rate value must not be zero\n");
+<<<<<<< Updated upstream
         modbus_free(ctx);
+=======
+        free(ctx);
+>>>>>>> Stashed changes
         errno = EINVAL;
         return NULL;
     }