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_114066

 > + * Define stack check guard pattern
 > + */
 > +#ifndef STACK_CHK_GUARD_VALUE

Please use the same kind of conventions for naming CPP Macros as with functions. This macro is in the "global scope" of any include of stackchk.h, so I would prefer to use `RTEMS_STACK_CHECKER_GUARD_VALUE`

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

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

doxygen should probably refer to the gcc -fstack-protection option.

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

 > + * @brief Stack Canary Pattern Initialization Function
 > + * 
 > + * @return uintptr_t 

explain what the return value is

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

 > + */
 > +static void __attribute__((constructor, 
 > +no_stack_protector)) __construct_stk_chk_guard()

should we do this in a sysinit?

I'm confused by this code. Did you write this code, or did you get it from somewhere? I don't see why we should have this, since it is eventually just using the configured value of `STACK_CHK_GUARD_VALUE`?

--
  
Gedare Bloom started a new discussion on testsuites/libtests/stackchk05/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/277#note_114070

 > +  TEST_BEGIN();
 > +
 > +  printk ("Stackchk05 test program is running!!!!\n");

remove this so the test only prints something if it fails.

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

 > +*** TEST STACK CHECKER ***
 > +Stack smashing detected! Aborting program.!!!!

shouldn't this have `"Stackchk05 test program is running!!!!\n")` in the output?


-- 
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/20241030/18006c5a/attachment-0001.htm>