Merge commit from fork

A memory-protected module's object lookup arrives at the Module Manager's dispatch
layer, which decides how much of the module's name buffer the privileged comparison
may touch. It checked one byte.

_txm_module_manager_object_name_compare reads a character from each name at the top
of every iteration and tests the remaining search length at the bottom, so a declared
length N permits reads at offsets 0 through N: N+1 bytes, the extra one being the
terminator a length excludes. The extended service receives that length in its extra
parameter array, validates the array correctly, and then never uses the length to
bound the buffer it describes. The deprecated service carries no length at all; its
manager wrapper calls the same implementation with the largest value a UINT can hold,
so it removes the bound rather than lacking one.

The comparison stops at the first differing character, so a walk past the end of the
module's memory looks as though it needs the bytes there to match a name the module
chose. It does not. A create service stores the name pointer a module supplies in the
control block rather than copying the string, so a module can register an object whose
name is the very buffer it then searches for. Both sides of the comparison are then
the same address, every character matches by construction, and the walk continues
until it meets a terminator in memory the module does not own and cannot see. That
walk runs in privileged mode with _tx_thread_preempt_disable raised, and none of the
27 memory fault handlers lowers it again, so a fault on it costs more than the
requesting thread.

The fix validates the range the comparison may reach. The extended dispatcher now
checks the name over name_length + 1 bytes, refusing a length whose range cannot be
expressed, and the extra parameter array is checked first because the length comes out
of it. The deprecated dispatcher refuses a memory-protected module outright, because
no range can be derived from a pointer alone; a module without protection is unchanged,
as it is for every other check in this layer.

_txm_module_manager_object_name_compare is left as it is. Reading N+1 bytes for a
declared length of N is the documented contract, and the defect is that the contract
was never checked.

The regression test drives both lookup dispatchers and measures two extents rather than
sampling either side of a boundary, because the finding is the relationship between
them. It anchors the name buffer to the end of one of the module's regions and walks it
backwards to find the smallest room the dispatcher accepts, which is the extent
validated; and it fills memory with a filler character, plants the only terminator at a
chosen depth, and asks for an object named with exactly the bytes up to it, so that the
deepest depth the lookup can be made to return from is the extent read. Against dev's
copy of the two headers the test exits 1 with 408 failed expectations of 1130: a
31-character name with one byte of room is accepted and read 31 bytes past the region,
and the aliased walk reaches every depth offered. With the fix it exits 0 with 1490
expectations and none failed.

Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>

common_modules/module_manager/inc/txm_module_manager_dispatch.h

@@ -19,6 +19,8 @@
 
 // Some portions generated by Claude Code (Opus 5).
 
+// Some portions generated by Claude Code (Opus 5).
+
 
 /**************************************************************************/
 /**************************************************************************/
@@ -3310,11 +3312,10 @@ ALIGN_TYPE return_value;
 
     if (module_instance -> txm_module_instance_property_flags & TXM_MODULE_MEMORY_PROTECTION)
     {
-        if (!TXM_MODULE_MANAGER_PARAM_CHECK_DEREFERENCE_STRING(module_instance, param_1))
+        /* This service carries no name length, so the manager searches with the largest
-            return(TXM_MODULE_INVALID_MEMORY);
+           length a UINT can hold and no readable range can be proven for the name.  A
-
+           memory-protected module must use the extended service, which declares one.  */
-        if (!TXM_MODULE_MANAGER_PARAM_CHECK_BUFFER_WRITE(module_instance, param_2, sizeof(VOID *)))
+        return(TXM_MODULE_INVALID_MEMORY);
-            return(TXM_MODULE_INVALID_MEMORY);
     }
 
     return_value = (ALIGN_TYPE) _txm_module_manager_object_pointer_get(
@@ -3340,10 +3341,13 @@ ALIGN_TYPE return_value;
 
     if (module_instance -> txm_module_instance_property_flags & TXM_MODULE_MEMORY_PROTECTION)
     {
-        if (!TXM_MODULE_MANAGER_PARAM_CHECK_DEREFERENCE_STRING(module_instance, param_1))
+        if (!TXM_MODULE_MANAGER_ENSURE_INSIDE_MODULE_DATA(module_instance, (ALIGN_TYPE)extra_parameters, sizeof(ALIGN_TYPE[2])))
             return(TXM_MODULE_INVALID_MEMORY);
 
-        if (!TXM_MODULE_MANAGER_ENSURE_INSIDE_MODULE_DATA(module_instance, (ALIGN_TYPE)extra_parameters, sizeof(ALIGN_TYPE[2])))
+        /* The name length has to be checked before it is trusted, so the extra parameter
+           array is checked first.  The comparison reads the declared length plus the
+           terminator that follows it, and all of that must be inside the module.  */
+        if (!TXM_MODULE_MANAGER_PARAM_CHECK_DEREFERENCE_STRING_RANGE(module_instance, param_1, (UINT) extra_parameters[0]))
             return(TXM_MODULE_INVALID_MEMORY);
 
         if (!TXM_MODULE_MANAGER_PARAM_CHECK_BUFFER_WRITE(module_instance, extra_parameters[1], sizeof(VOID *)))

common_modules/module_manager/inc/txm_module_manager_util.h

@@ -135,6 +135,15 @@
     ((TXM_MODULE_MANAGER_ENSURE_INSIDE_MODULE(module_instance, string_ptr, 1)) || \
      ((void *) (string_ptr) == TX_NULL))
 
+/* Strings we walk are checked over the whole range the walk may reach: the declared
+   length plus the terminating character that follows it, since a length excludes the
+   terminator and the comparison reads it.  A length whose range cannot be expressed
+   is refused rather than truncated.  */
+#define TXM_MODULE_MANAGER_PARAM_CHECK_DEREFERENCE_STRING_RANGE(module_instance, string_ptr, string_length) \
+    (((((ALIGN_TYPE) (string_length)) < (~((ALIGN_TYPE) 0))) && \
+      (TXM_MODULE_MANAGER_ENSURE_INSIDE_MODULE(module_instance, string_ptr, ((ALIGN_TYPE) (string_length)) + ((ALIGN_TYPE) 1)))) || \
+     ((void *) (string_ptr) == TX_NULL))
+
 #define TXM_MODULE_MANAGER_UTIL_MAX_VALUE_OF_TYPE_UNSIGNED(type) ((1ULL << (sizeof(type) * 8)) - 1)
 
 #define TXM_MODULE_MANAGER_UTIL_MATH_ADD_ULONG(augend, addend, result) \

test/tx/cmake/module_manager/CMakeLists.txt

@@ -194,3 +194,25 @@ target_compile_options(
 
 add_test(${CMAKE_BUILD_TYPE}::threadx_module_manager_block_pool_parameters_test
          threadx_module_manager_block_pool_parameters_test)
+
+add_executable(
+  threadx_module_manager_object_name_range_test
+  ${SOURCE_DIR}/threadx_module_manager_object_name_range_test.c
+  ${module_manager_dir}/src/txm_module_manager_object_pointer_get.c
+  ${module_manager_dir}/src/txm_module_manager_object_pointer_get_extended.c
+  ${module_manager_dir}/src/txm_module_manager_util.c)
+
+target_include_directories(
+  threadx_module_manager_object_name_range_test
+  PRIVATE ${SOURCE_DIR}
+          ${REPO_ROOT}/common/inc
+          ${REPO_ROOT}/common_modules/inc
+          ${module_manager_dir}/inc
+          ${cortex_m4_module_dir}/inc)
+
+target_compile_options(
+  threadx_module_manager_object_name_range_test
+  PRIVATE -include ${SOURCE_DIR}/threadx_module_manager_host_test_port.h)
+
+add_test(${CMAKE_BUILD_TYPE}::threadx_module_manager_object_name_range_test
+         threadx_module_manager_object_name_range_test)