Skip to content

FreeRTOS layer passes pointers through ULONG, which truncates where ULONG is narrower than a pointer #587

Description

@fdesbiens

utility/rtos_compatibility_layers/FreeRTOS/tx_freertos.c passes pointers through ULONG arguments in several places, because the ThreadX entry points it calls take a ULONG for the caller supplied value:

  • xTaskCreate() and xTaskCreateStatic() cast pvParameters to ULONG for tx_thread_create().
  • xTimerCreate() and xTimerCreateStatic() cast the timer structure pointer to ULONG for tx_timer_create(), and txfr_timer_callback_wrapper() casts it back with p_timer = (txfr_timer_t *)id;.
  • xTaskCreate() returns (BaseType_t)NULL on an error path.

This is sound only where ULONG is at least as wide as a pointer. It is not on every target. The Linux port, for one, defines ULONG as unsigned int on x86_64:

#if defined(__x86_64__) && __x86_64__
typedef int                                     LONG;
typedef unsigned int                            ULONG;
#else
typedef long                                    LONG;
typedef unsigned long                           ULONG;
#endif

so a 64 bit build truncates every one of those pointers to 32 bits. GCC reports it:

warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

Impact

The task argument is handed back to application code, and the timer identifier is dereferenced by txfr_timer_callback_wrapper() on every expiry. On a target where a pointer does not fit in a ULONG, the first timer callback dereferences a truncated pointer.

This is contained today because the layer is used on 32 bit embedded targets, and the regression suite added in #583 builds 32 bit for exactly this reason, as the ThreadX and SMP suites do. It becomes a live defect the moment the layer is hosted on a target where ULONG is narrower than a pointer.

Suggested fix

The cast has to go through an integer type wide enough to hold a pointer. ALIGN_TYPE or an explicit uintptr_t would both work, though tx_thread_create()'s and tx_timer_create()'s parameter types constrain what is possible without touching the kernel API, so this may end up being a documented constraint rather than a code change. At minimum the constraint should be stated in the layer's readme.md, since it silently rules out hosting the layer where pointers are wider than ULONG.

Whatever the resolution, the -Wpointer-to-int-cast warnings should not be left standing without a note explaining why they are acceptable.

Notes

Found while building the regression suite in #583.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions