Add cc1120 random number generator and stack smashing protection #297

Cathouwu · 2024-06-30T19:55:32Z

Purpose

Created a number generator function from the c1120 to generate a stack canary for stack overflow protection:

https://www.notion.so/uworbital/6375c5c5dcf1485e8a6c47f18d419ae5?v=72baf90d01f643b09d77418bd3f53b14&p=02e40c1185bb439d93fd92288137edcc&pm=s

New Changes

Added a random number generator function in the cc1120 file.
Modified state_mgr to initialize peripherals and cc1120. Changed stack guard after cc1120 is initialized.

Testing

Tested that the cc1120 can generate random numbers
Tested that the stack canary in app_main is updated with the random number.

Stack canaries generated (generated after every reboot: )

Outstanding Changes

n/a

github-actions · 2024-06-30T19:55:47Z

Pull reviewers stats

Stats of the last 120 days for UWOrbital:

	User	Total reviews	Time to review	Total comments
	Yarik-Popov	16	5d 4h 21m	112
	dgobalak	5	1h 30m	16
	Gjjjiang	4	18h 47m	22
	PratyushMakkar	4	6d 5h 12m	27
	Navtajh04	2	27d 14h	6
	kepler452b123	1	9h 15m	4
^{⚡️ Pull request stats}

obc/app/drivers/cc1120/cc1120_mcu.h

dgobalak · 2024-07-01T06:25:25Z

obc/app/app_main.c

@@ -40,6 +59,8 @@ int main(void) {
  initSciMutex();
  initI2CMutex();
  initSpiMutex();
+  cc1120Init();


@Navtajh04 any issues with this being called twice? I think we also call this in comms mgr.

Yeah I would remove this from here and change the stack guard from comms mgr. We aren't really susceptible to any kind of stack smashing attacks until comms is established anyways

especially since this is accessing cc1120, we should do it in comms mgr

obc/app/app_main.c

obc/app/drivers/cc1120/cc1120_mcu.h

obc/app/drivers/cc1120/cc1120.c

obc/app/app_main.c

obc/app/drivers/cc1120/cc1120.c

obc/app/drivers/cc1120/cc1120.h

obc/app/app_main.c

obc/app/sys/obc_errors.h

…l/OBC-firmware into catherine/stack_canary2

Already handled

Navtajh04 · 2024-09-07T01:27:40Z

obc/app/app_main.c

@@ -40,6 +59,8 @@ int main(void) {
  initSciMutex();
  initI2CMutex();
  initSpiMutex();
+  cc1120Init();


Yeah I would remove this from here and change the stack guard from comms mgr. We aren't really susceptible to any kind of stack smashing attacks until comms is established anyways

Navtajh04 · 2024-09-07T01:29:47Z

obc/app/app_main.c

+uint32_t __stack_chk_guard_init(void);
+
+uint32_t __stack_chk_guard_change(void) {


double __ is usually reserved for compiler specific stuff (ex: __stack_chk_guard is a special variable name used by GCC) so for these 2 helper functions we are defining ourselves (not special compiler keywords) we should use our regular function naming convention

Navtajh04 · 2024-09-07T01:31:39Z

obc/app/app_main.c

@@ -40,6 +59,8 @@ int main(void) {
  initSciMutex();
  initI2CMutex();
  initSpiMutex();
+  cc1120Init();


especially since this is accessing cc1120, we should do it in comms mgr

Navtajh04 · 2024-09-07T02:04:47Z

obc/app/drivers/cc1120/cc1120_mcu.h

-#define CC1120_SPI_PORT spiPORT4
-#define CC1120_SPI_CS 0
+#define CC1120_SPI_PORT spiPORT3
+#define CC1120_SPI_CS 1


This shouldn't be changed, cc1120 is on SPI port 4 on obc

obc/app/app_main.c

Co-authored-by: Navtajhundal <[email protected]>

… catherine/stack_canary2

kepler452b123

some nits, otherwise lgtm

kepler452b123 · 2025-02-04T09:46:19Z

obc/app/modules/comms_link_mgr/comms_manager.c

+#include "obc_print.h"
+#include "obc_sci_io.h"
+#include <sci.h>


kepler452b123 · 2025-02-04T09:47:17Z

obc/app/modules/state_mgr/state_mgr.c

+#include "obc_sci_io.h"
+#include "cc1120_txrx.h"
+#include "cc1120.h"
+#include "obc_print.h"


remove sci_io and print. move cc1120 includes to lines after #include lm75bd.h

kepler452b123 · 2025-02-04T09:47:25Z

obc/app/modules/state_mgr/state_mgr.c


 #include <FreeRTOS.h>
 #include <os_portmacro.h>
 #include <os_queue.h>
 #include <os_task.h>
 #include <sys_common.h>
+#include <sci.h>


kepler452b123 · 2025-02-04T09:48:00Z

obc/app/modules/state_mgr/state_mgr.c


 #if defined(DEBUG) && !defined(OBC_REVISION_2)
 #include <gio.h>
 #endif

+extern void *__stack_chk_guard;
+#define STACK_BYTES 4


nit: not a completely accurate name, i would make it more specific. e.g. STACK_CANARY_BYTES

Tested cc1120 random number generator and stack smashing protection

d41aa57

Navtajh04 reviewed Jul 1, 2024

View reviewed changes

obc/app/drivers/cc1120/cc1120_mcu.h Outdated Show resolved Hide resolved

dgobalak reviewed Jul 1, 2024

View reviewed changes

Yarik-Popov requested changes Jul 1, 2024

View reviewed changes

Cathouwu and others added 3 commits July 13, 2024 16:06

addressed comments

4536b42

small fix

8ea909c

Merge branch 'main' into catherine/stack_canary2

48f250d

Yarik-Popov previously requested changes Jul 20, 2024

View reviewed changes

obc/app/sys/obc_errors.h Outdated Show resolved Hide resolved

Cathouwu added 2 commits July 21, 2024 14:36

changed error code to common

8b434ff

Merge branch 'catherine/stack_canary2' of https://github.com/UWOrbita…

6428f23

…l/OBC-firmware into catherine/stack_canary2

Navtajh04 requested changes Sep 7, 2024

View reviewed changes

Yarik-Popov changed the title ~~Tested cc1120 random number generator and stack smashing protection~~ Add cc1120 random number generator and stack smashing protection Sep 7, 2024

Cathouwu and others added 11 commits February 1, 2025 15:00

commit suggestion

b528093

Co-authored-by: Navtajhundal <[email protected]>

Merge branch 'main' of https://github.com/UWOrbital/OBC-firmware into…

4ad2eff

… catherine/stack_canary2

addressed comments

4bb1a96

checking

eac2b2c

init cc1120 and stack canary in state manager

a80e19d

clean app_main.c

a0fdd1e

Update state_mgr.c

96ab518

Update comms_manager.h

f39cc3d

Update comms_manager.c

9d23ce2

Update comms_manager.c

27c121c

last commit. tested and stack_guard change fucntion works

075dac5

kepler452b123 requested changes Feb 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cc1120 random number generator and stack smashing protection #297

Add cc1120 random number generator and stack smashing protection #297

Cathouwu commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024

dgobalak Jul 1, 2024

Navtajh04 Sep 7, 2024

Navtajh04 Sep 7, 2024

Cathouwu Feb 1, 2025

Navtajh04 Sep 7, 2024

Navtajh04 Sep 7, 2024

Cathouwu Feb 1, 2025

Navtajh04 Sep 7, 2024

Navtajh04 Sep 7, 2024

Cathouwu Feb 1, 2025

kepler452b123 left a comment

kepler452b123 Feb 4, 2025

kepler452b123 Feb 4, 2025

kepler452b123 Feb 4, 2025

kepler452b123 Feb 4, 2025

		uint32_t __stack_chk_guard_init(void);

		uint32_t __stack_chk_guard_change(void) {

Add cc1120 random number generator and stack smashing protection #297

Are you sure you want to change the base?

Add cc1120 random number generator and stack smashing protection #297

Conversation

Cathouwu commented Jun 30, 2024 • edited Loading

Purpose

New Changes

Testing

Outstanding Changes

github-actions bot commented Jun 30, 2024

Pull reviewers stats

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kepler452b123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cathouwu commented Jun 30, 2024 •

edited

Loading