RTEMS | libmisc/stackchk: Add stack smashing protection (!277)

[Gedare Bloom (@gedare)]

Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277 was reviewed by Gedare Bloom

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114865

 > +#ifndef RTEMS_STACK_CHECKER_GUARD_VALUE
 > +  #define RTEMS_STACK_CHECKER_GUARD_VALUE 0xDEADBEEF
 > +#endif

I think this logic probably belongs in `cpukit/include/rtems/confdefs/extensions.h` near the other configuration of the stack checker.

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114866

 > + * 
 > + * @note This function is called when a stack overflow occurs.
 > + *       User application can override it's default implementation.

"its"

--
  
Gedare Bloom started a new discussion on cpukit/include/rtems/stackchk.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114867

 > + * 
 > + * @note This function is invoked by defining the GCC flag
 > + *       -fstack-protection options.

This wording is not quite right. The function is invoked if a stack overflow is detected due to enabling the `-fstack-protection`

--
  
Gedare Bloom started a new discussion on cpukit/libmisc/stackchk/check.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114868

 > + * at the application 
 > + */
 > +__attribute__((weak,noreturn)) void __stack_chk_fail(void)

We have wrappers for attributes that should be used, `RTEMS_NO_RETURN` and `RTEMS_WEAK`.

--
  
Gedare Bloom started a new discussion on cpukit/libmisc/stackchk/check.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114869

 > +  Thread_Control *running = _Thread_Get_executing();
 > +
 > +  printk("This inside __stack_chk_fail!!!!!!!!!\n");

The default version should not print anything.

--
  
Gedare Bloom commented on a discussion on testsuites/libtests/stackchk05/stackchk05.scn: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114870

 > +*** TEST STACK CHECKER ***
 > +BLOWN STACK!!!

I'm confused why your .scn print statements are different from the print statements in your code. Make sure you update to make things consistent.

By default we would prefer to have minimal printing for tests that pass.

--
  
Gedare Bloom started a new discussion on cpukit/libmisc/stackchk/check.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114871

 > +
 > +/*
 > + * This is the default implementation. The user can override it 

you should probably add a test case that overrides it, to show how to do that.

--
  
Gedare Bloom started a new discussion on testsuites/libtests/stackchk05/system.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114872

 > +#define CONFIGURE_STACK_CHECKER_ENABLED
 > +
 > +#define CONFIGURE_STACK_CHECKER_REPORTER  rtems_stack_checker_reporter_print_details

I would use the default/quiet reporter in this test.

--
  
Gedare Bloom started a new discussion on spec/build/testsuites/libtests/stackchk05.yml: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114873

 > +build-type: test-program
 > +cflags:
 > +- "-DRTEMS_STACK_CHECKER_GUARD_VALUE=0xA7A7A7A7"

why use this when there is an application configuration for it?


-- 
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277
You're receiving this email because of your account on gitlab.rtems.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20241113/a2bab2ba/attachment-0001.htm>