mirror of
https://github.com/eclipse-threadx/threadx.git
synced 2026-10-06 06:59:08 +08:00
Merge commit from fork
A module reaches a kernel object's delete service through the Module Manager's dispatch layer, which asked one question about the pointer: does the object lie outside the module's own code and data. That is the policy for using an object, and using an object a module does not own is supported and intended -- txm_module_object_pointer_get_extended searches the system's created objects by name and hands back objects the application created and objects other modules created, so that a module can send to a shared queue, take a shared semaphore or get a shared mutex. Destroying one of those is not sharing it. A module could name any object it had a pointer to and have the privileged dispatcher delete it: a host service lost its queue, waiters were resumed with TX_DELETED, an owned mutex's priority inheritance was unwound, and an active timer stopped. Nothing in the manager intended that. The deallocation the delete path performs afterwards has always required the memory to belong to the calling module, so a delete of anything else could only ever end in TX_PTR_ERROR -- the ownership requirement was already there and was enforced one step too late, after the kernel object had been irreversibly destroyed and its waiters woken. An error returned at that point describes a cleanup failure and restores nothing. The eight delete dispatchers now ask the question before the delete rather than after it. A memory-protected module may delete an object only if the object is one it allocated from the manager's object pool, at the exact address the manager returned to it, for an allocation made for that type's control block size -- the same conditions the create dispatchers already apply, since deletion is the inverse of creation. Anything else returns TXM_MODULE_INVALID_MEMORY, which is what every other parameter rejection in the dispatch table returns, so no new value enters the module ABI and a module cannot use a delete request to tell "not yours" apart from "not a usable address". The object stays created, nothing waiting on it is resumed, and no created count moves, because the kernel was never asked. The ownership question is deliberately separate from whether an object is there at all and of the expected type. It establishes nothing about liveness or type, and it is composed with the checks that do rather than replacing them. _txm_module_manager_object_deallocate reached the manager's private header by subtracting from whatever address the caller supplied, and read it before deciding whether it was a header. All eight delete dispatchers call that function directly, so an object the module does not own arrived there as a matter of course rather than exceptionally: for an object the application allocated statically, or for any address outside the object pool, the words in front of it were unrelated memory read in privileged mode -- and acted on, since an address whose preceding words happened to name the calling module was unlinked from that module's allocation list and handed to the byte pool. It now finds the allocation by searching the module's own allocation list. The caller's address is compared and never dereferenced, so an address that names none of this module's allocations is refused without a privileged read of anything in front of it, and the search establishes what the header read could not: that the address is the exact start of an allocation rather than somewhere inside one. The search is bounded by the count the manager keeps beside the list and runs with interrupts disabled, so a damaged list cannot make it run on and it cannot observe the list being changed under it. The fix is in the deallocation itself rather than at a call site, so it covers all eight delete dispatchers, the deallocation request a module can make directly, protected and unprotected modules alike. txm_module_manager_stop needs no exemption and was not given one. It deletes the objects a module created by calling the internal _tx_*_delete services directly, and identifies them with _txm_module_manager_created_object_check rather than through a module request, so no caller-facing check stands in its way. The 209 expectations in the new test drive all eight object types through allocation, an ownership check by the owning module and by another, a check against a larger and a smaller expected size, and a deallocation that succeeds. They cover an object the application owns, deliberately preceded by a header naming the requesting module so that reading it can be seen to have happened; another module's object, checked and refused from both sides; every aligned interior offset of an allocation and the addresses of its header, its end, the pool's ends and one past the pool; a null pointer and an address with no room for a header in front of it; a request from no module at all; memory that has changed hands, where a stale pointer names an address that now belongs to another module; the bounds on the search, against a count smaller than the list and against a count larger than a list whose links are broken; releasing the head, the middle and the last of a list of three; and an object pool that was never created. Every call asserts that the interrupt lock came back balanced, and that the search ran with interrupts disabled exactly one deep. Restoring the previous deallocation fails four of them and then segmentation faults, on the null-pointer case, where the header in front of address zero is read; removing the ownership check fails 74. Line and branch coverage of the two new functions and of the changed _txm_module_manager_object_deallocate is 100%, with two branch outcomes excepted that are test scaffolding rather than product code: the host shim's stand-ins for TX_DISABLE and TX_RESTORE carry a maximum-depth test and an underflow guard, and the real ports' primitives are inline assembly with no branch at all. All 99 tests pass in each of the five configurations the tree builds with GCC 14. Nothing in the tree compiles the dispatch header, so the eight changed dispatchers were cross-compiled by hand: the two changed C files and a translation unit that includes the header build at -Werror with arm-none-eabi-gcc 13.2.1 for every GNU 32-bit module port -- Cortex-A7, M0+, M23, M3, M33, M4 and M7 -- and produce the same -Wall -Wextra warning counts as before the change, 117 on Cortex-A7 and 116 on each of the others. Cortex-R4 and RXv2 have no GNU module port, and the two AArch64 module ports have no toolchain available here; the new arithmetic is a comparison of two addresses of the same type, so a wider ALIGN_TYPE changes nothing about it. MISRA: all if bodies are braced, the address comparison goes through ALIGN_TYPE, no pointer the caller supplied is dereferenced, and no goto appears. No deviation is required. Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
/***************************************************************************
|
||||
* Copyright (c) 2024 Microsoft Corporation
|
||||
* Copyright (c) 2025 Eclipse ThreadX Contributors
|
||||
* Copyright (c) 2026 Eclipse ThreadX contributors
|
||||
*
|
||||
* This program and the accompanying materials are made available under the
|
||||
* terms of the MIT License which is available at
|
||||
@@ -12,6 +13,8 @@
|
||||
|
||||
// Some portions generated by Claude Code (Opus 5).
|
||||
|
||||
// Some portions generated by Claude Code (Opus 5).
|
||||
|
||||
|
||||
/**************************************************************************/
|
||||
/**************************************************************************/
|
||||
@@ -106,6 +109,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_BLOCK_POOL_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_BLOCK_POOL)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_block_pool_delete(
|
||||
@@ -410,6 +416,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_BYTE_POOL_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_BYTE_POOL)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_byte_pool_delete(
|
||||
@@ -700,6 +709,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_EVENT_FLAGS_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_EVENT_FLAGS_GROUP)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_event_flags_delete(
|
||||
@@ -1005,6 +1017,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_MUTEX_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_MUTEX)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_mutex_delete(
|
||||
@@ -1302,6 +1317,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_QUEUE_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_QUEUE)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_queue_delete(
|
||||
@@ -1734,6 +1752,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_SEMAPHORE_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_SEMAPHORE)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_semaphore_delete(
|
||||
@@ -2075,6 +2096,9 @@ ALIGN_TYPE stack_status;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_THREAD_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_THREAD)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_thread_delete(thread_ptr);
|
||||
@@ -2896,6 +2920,9 @@ ALIGN_TYPE return_value;
|
||||
{
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_TYPED_OBJECT_FOR_USE(module_instance, param_0, TXM_TIMER_OBJECT))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
|
||||
if (!TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, param_0, sizeof(TX_TIMER)))
|
||||
return(TXM_MODULE_INVALID_MEMORY);
|
||||
}
|
||||
|
||||
return_value = (ALIGN_TYPE) _txe_timer_delete(
|
||||
|
||||
@@ -120,6 +120,16 @@
|
||||
#define TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_CREATION(module_instance, obj_ptr, obj_size) \
|
||||
(_txm_module_manager_param_check_object_for_creation(module_instance, obj_ptr, obj_size))
|
||||
|
||||
/* When deleting an object, being allowed to use it is not enough. Deletion is the
|
||||
inverse of creation, so it asks what creation asks: that the object is one this
|
||||
module allocated from the manager's object pool, at the exact address the manager
|
||||
gave it, for an object of this size. A module may hold a pointer to an object it
|
||||
does not own -- the manager's object lookup service hands application-owned
|
||||
objects to modules by name, and sharing an object is what that service is for --
|
||||
and this is what separates using such an object from destroying it. */
|
||||
#define TXM_MODULE_MANAGER_PARAM_CHECK_OBJECT_FOR_DELETION(module_instance, obj_ptr, obj_size) \
|
||||
(_txm_module_manager_allocated_object_check(module_instance, obj_ptr, obj_size))
|
||||
|
||||
/* Strings we dereference can be in RW/RO/Shared areas. */
|
||||
#define TXM_MODULE_MANAGER_PARAM_CHECK_DEREFERENCE_STRING(module_instance, string_ptr) \
|
||||
((TXM_MODULE_MANAGER_ENSURE_INSIDE_MODULE(module_instance, string_ptr, 1)) || \
|
||||
@@ -147,6 +157,8 @@ UINT _txm_module_manager_param_check_object_for_creation(TXM_MODULE_INSTANCE
|
||||
UINT _txm_module_manager_param_check_object_for_use(TXM_MODULE_INSTANCE *module_instance, ALIGN_TYPE object_ptr, ULONG object_size);
|
||||
UINT _txm_module_manager_param_check_typed_object_for_use(TXM_MODULE_INSTANCE *module_instance, ALIGN_TYPE object_ptr, UINT object_type);
|
||||
UINT _txm_module_manager_allocated_object_check(TXM_MODULE_INSTANCE *module_instance, ALIGN_TYPE object_ptr, ULONG object_size);
|
||||
TXM_MODULE_ALLOCATED_OBJECT
|
||||
*_txm_module_manager_allocated_object_find(TXM_MODULE_INSTANCE *module_instance, ALIGN_TYPE object_ptr, ULONG *object_size_ptr);
|
||||
UINT _txm_module_manager_object_type_size_get(UINT object_type, ULONG *object_size);
|
||||
UINT _txm_module_manager_created_object_type_check(ALIGN_TYPE object_ptr, UINT object_type);
|
||||
UINT _txm_module_manager_object_id_check(ALIGN_TYPE object_ptr, UINT object_type);
|
||||
|
||||
@@ -27,6 +27,7 @@
|
||||
#include "tx_api.h"
|
||||
#include "tx_thread.h"
|
||||
#include "txm_module.h"
|
||||
#include "txm_module_manager_util.h"
|
||||
|
||||
/**************************************************************************/
|
||||
/* */
|
||||
@@ -52,6 +53,13 @@
|
||||
/* tx_*_delete() service is sufficient; pool deallocation is handled */
|
||||
/* automatically by the dispatch layer. */
|
||||
/* */
|
||||
/* Only memory the calling module allocated from the object pool is */
|
||||
/* released, and the allocation is identified by searching the */
|
||||
/* module's own allocation list rather than by reading a private */
|
||||
/* header from in front of the address the caller supplied. An */
|
||||
/* address that names none of this module's allocations returns */
|
||||
/* TX_PTR_ERROR and is not dereferenced. */
|
||||
/* */
|
||||
/* INPUT */
|
||||
/* */
|
||||
/* object_ptr Object pointer to deallocate */
|
||||
@@ -64,6 +72,8 @@
|
||||
/* */
|
||||
/* _txe_mutex_get Get module instance mutex */
|
||||
/* _txe_mutex_put Release module instance mutex */
|
||||
/* _txm_module_manager_allocated_object_find */
|
||||
/* Find the module's allocation */
|
||||
/* _txe_byte_release Release object back to pool */
|
||||
/* */
|
||||
/* */
|
||||
@@ -90,14 +100,32 @@ UINT return_value;
|
||||
/* Pickup module instance pointer. */
|
||||
module_instance = _tx_thread_current_ptr -> tx_thread_module_instance_ptr;
|
||||
|
||||
/* Setup the memory pointer. */
|
||||
module_allocated_object_ptr = (TXM_MODULE_ALLOCATED_OBJECT *) object_ptr;
|
||||
/* Find the allocation this address names.
|
||||
|
||||
/* Position the object pointer backwards to position back to the module manager information. */
|
||||
previous_object = module_allocated_object_ptr--;
|
||||
The private header in front of an allocation is the manager's own
|
||||
record of who the memory belongs to, but it is only a header when the
|
||||
address is one the manager gave out. Reaching it by subtraction from
|
||||
whatever address the caller supplied assumes the answer: for an object
|
||||
the application allocated statically, or for any address outside the
|
||||
object pool, the words in front of it are unrelated memory, and this
|
||||
read them in privileged mode before deciding they were not a header.
|
||||
Every delete dispatcher reaches this function with the address the
|
||||
module named, so an object the module does not own arrived here as a
|
||||
matter of course rather than exceptionally.
|
||||
|
||||
Searching the module's own allocation list answers the same question
|
||||
without that assumption. The address is compared against the
|
||||
allocations the manager made for this module and is never
|
||||
dereferenced, so an address that names none of them -- an object the
|
||||
application owns, an object another module owns, or an address nowhere
|
||||
near the object pool -- is refused without a privileged read. The
|
||||
search also establishes what the header read could not: that the
|
||||
address is the exact start of an allocation rather than somewhere
|
||||
inside one. */
|
||||
module_allocated_object_ptr = _txm_module_manager_allocated_object_find(module_instance, (ALIGN_TYPE) object_ptr, TX_NULL);
|
||||
|
||||
/* Make sure the object is valid. */
|
||||
if ((module_allocated_object_ptr == TX_NULL) || (module_allocated_object_ptr -> txm_module_allocated_object_module_instance != module_instance) || (module_instance -> txm_module_instance_object_list_count == 0))
|
||||
if (module_allocated_object_ptr == TX_NULL)
|
||||
{
|
||||
/* Set return value to invalid pointer. */
|
||||
return_value = TX_PTR_ERROR;
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -103,3 +103,27 @@ target_compile_options(
|
||||
|
||||
add_test(${CMAKE_BUILD_TYPE}::threadx_module_manager_object_authentication_test
|
||||
threadx_module_manager_object_authentication_test)
|
||||
|
||||
# This test drives the manager services directly rather than through a dispatcher,
|
||||
# so it needs none of the dispatch table and no guard list.
|
||||
add_executable(
|
||||
threadx_module_manager_delete_ownership_test
|
||||
${SOURCE_DIR}/threadx_module_manager_delete_ownership_test.c
|
||||
${module_manager_dir}/src/txm_module_manager_util.c
|
||||
${module_manager_dir}/src/txm_module_manager_object_allocate.c
|
||||
${module_manager_dir}/src/txm_module_manager_object_deallocate.c)
|
||||
|
||||
target_include_directories(
|
||||
threadx_module_manager_delete_ownership_test
|
||||
PRIVATE ${SOURCE_DIR}
|
||||
${REPO_ROOT}/common/inc
|
||||
${REPO_ROOT}/common_modules/inc
|
||||
${module_manager_dir}/inc
|
||||
${cortex_a7_module_dir}/inc)
|
||||
|
||||
target_compile_options(
|
||||
threadx_module_manager_delete_ownership_test
|
||||
PRIVATE -include ${SOURCE_DIR}/threadx_module_manager_host_test_port.h)
|
||||
|
||||
add_test(${CMAKE_BUILD_TYPE}::threadx_module_manager_delete_ownership_test
|
||||
threadx_module_manager_delete_ownership_test)
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user