Input validity unchecked

[Daniel Páscoa]

In a generic way, function inputs shall be checked for validity before being used.

When successive calls are done to different functions, one might argue that inputs are to be checked only once and then considered valid in other calls. Yet this has some pitfalls like:

• function inputs are not checked in a consistent way throughout all functions:
  • Each one has to constantly double-check in comments and/or callers if inputs are instead checked elsewhere in the call tree.
  • Even if for the moment no call tree leads to invalid input issues, future maintenance operations may add new calls without validity checks.

One can assume some exceptions can occur (e.g., when there is a considerable performance impact), but as long such cases are justified within the function’s comments one can easily sort out those exceptions.

Bellow, there is a list of cases where input validity could be added:

• bsps\include\bsp\irq-generic.h: bsp_interrupt_entry_load_first function is called by bsp_interrupt_handler_dispatch_unchecked where the argument was already checked.

• bsps\include\bsp\irq-generic.h: bsp_interrupt_handler_index function is called by bsp_interrupt_entry_load_first, bsp_interrupt_entry_remove, bsp_interrupt_spurious, bsp_interrupt_entry_find, bsp_interrupt_entry_install, rtems_interrupt_handler_iterate functions. It was possible to trace in all of them a validation of the argument used in bsp_interrupt_handler_index before being called.

• bsps\shared\irq\irq-default-handler.c: bsp_interrupt_handler_default is only called by bsp_interrupt_spurious and bsp_interrupt_handler_dispatch_unchecked functions and It was possible to trace in all ot them a validation of the argument used in bsp_interrupt_handler_index before being called.

• bsps\shared\irq\irq-entry-remove.c: the *entry argument of bsp_interrupt_entry_do_remove is actually tested and validated inside of that function. That function is also the only place where bsp_interrupt_entry_remove is called. The arguments vector, *entry and previous_next are all validated by a call to the bsp_interrupt_entry_find and subsequent code that is run prior to call bsp_interrupt_entry_remove function.

• bsps\shared\irq\irq-generic.c: the bsp_interrupt_spurious function argument vector is tested before being called in bsp_interrupt_handler_dispatch_unchecked function.

• bsps\shared\irq\irq-generic.c: bsp_interrupt_entry_install_first function is called once by bsp_interrupt_entry_install function. The validation of the argument vector can be traceback to rtems_interrupt_entry_install function. The argument *entry is also passed by bsp_interrupt_entry_install function and explained in the previous paragraph.

• bsps\shared\dev\clock\clockimpl.h: in both functions (void Clock_isr and rtems_isr Clock_isr) declaration/definition (they are the same function, just declared differently depending of #if def's), both arguments are not used. Due to the evaluation of the internal #if, this functions are reduced to Clock_driver_ticks += 1; _Timecounter_Tick();

• bsps\sparc\leon3\start\bspsmp.c: the bsp_start_on_secondary_processor function calls _SMP_Start_multitasking_on_secondary_processor with the same argument as it was called. This argument in that sub-function is used to get a value cpu_index_self, which itself is validated and checked if it is a proper value or not. Indirect validation.

• bsps\sparc\leon3\start\bspsmp.c: the _CPU_SMP_Send_interrupt function is called in 3 different places: in one place (_SMP_Request_shutdown function in smp.c file) it is inside a for loop that is limited by _SMP_Processor_configured_maximum. This is OK. For the other 2 places ( from _SMP_Send_message function in smp.c file and _Thread_Dispatch_request function in threaddispatch.h file it is not clear that the argument of _CPU_SMP_Send_interrupt cannot be limited (when it should be limited at least to _SMP_Processor_configured_maximum).

• bsps\sparc\leon3\start\eirq.c: bsp_interrupt_get_attributes function actually tests vector argument. But also there is only one call to this function which is from rtems_interrupt_get_attributes function in irq-enable-disable.c file where both arguments are tested prior call to bsp_interrupt_get_attributes function.

• bsps\sparc\shared\start\bspgetworkarea.c: _SPARC_Memory_initialize function receives its *end_of_usable_ram argument from the main function in start.S file. No validation is performed there. The argument is used to initialize a struct, which later on may be used for dynamically allocated memory. The referred pointer argument is never dereferenced, it is used as the end limit of the allocated memory position. It may make sense to test in this initialisation function if it is value is above &_Memory_Areas[ 0 ].

• cpukit\libc\string\memset.c: memset function is part of space qualification profile context, and the void *<[dst]> pointer is not tested against NULL.

• cpukit\libc\string\memcpy.c: memcpy function is part of space qualification profile context, and the void *dst and void *src pointers is not tested against NULL.

• cpukit\posix\src\clocknanosleep.c: in function clock_nanosleep the *rmtp argument can be NULL if the user does not want to get the remain time, so there is no need to validated it. For the *rqtp (request) argument this should be validated if different than NULL and the tv_nsec field must be validated if between 0 and 999999999.

• cpukit\rtems\src\clocktodtoseconds.c: the _TOD_To_seconds function is called by _Timer_Fire_when function from timercreate.c file, by rtems_task_wake_when function from taskwakewhen.c file and by rtems_clock_set function from clockset.c file. In all three cases the argument with which the _TOD_To_seconds function is called is validated with a call to _TOD_Validate function prior to the call.

• cpukit\rtems\src\eventsend.c: the _Event_Seize function is called by two similar functions the rtems_event_receive from eventreceive.c file and rtems_event_system_receive from systemeventreceive.c file. The *event_out argument is validated prior the call; all other arguments are declared and defined in the caller functions prior to call the Event_Seize function.

Additional Notes:
This ticket was raised as an outcome of the Independent SW Verification and Validation (ISVV) for ESA-promoted RTEMS SMP Qualification Data Packs (​https://rtems-qual.io.esa.int). The original ISVV reference for this issue is RTEMS-SMP-VAL-029.

[Daniel Páscoa]

Correction: The original ISVV reference for this issue is RTEMS-SMP-VER-029.

[Sebastian Huber]

For internal functions it is responsibility of the caller to ensure that arguments are valid.

The C library functions memset() and memcpy() are not required to check for NULL.

clocknanosleep() checks the mentioned error conditions. There are also test cases for them.

The end_of_usable_ram is provided by the boot loader. How can the BSP validate this?