[pal:linux] posix timer after util destroy

[awidegreen]

Prerequisites

Can you reproduce the problem reliably? yes

Did you check current release notes for known issues? yes

If this is not the latest release, have you checked newer releases? yes

Description:

we have wrapped the optiga util/crypt within its own library/app. The app only requires access to the secure element every now and then - based on some external input. For that reason, the optiga related objects are created on the fly and dropped (deleted) once the operation completed. Before dropping the instances optiga_util_destroy(optiga_util*) is called!

Implementing it like this resulted in segmentation faults and timer_settime errors (EINVAL).

investigating the issues let to pal_os_event.c:

• seg fault due to access of memory (via pointer) which has already been deleted e.g optiga_cmd
• timer errors due to timers that have already been deleted timer_delete(id) via pal_os_event_destroy

Adding some checks will solve some of the issues:

diff --git a/pal/linux/pal_os_event.c b/pal/linux/pal_os_event.c
index 3df47a1..b463521 100644
--- a/pal/linux/pal_os_event.c
+++ b/pal/linux/pal_os_event.c
@@ -78,6 +78,7 @@ static void handler(int sig, siginfo_t *si, void *uc)

 static pal_os_event_t pal_os_event_0 = {0};
 static     timer_t timerid;
+static volatile sig_atomic_t g_timer_destroyed = FALSE;

 void pal_os_event_start(pal_os_event_t * p_pal_os_event, register_callback callback, void * callback_args)
 {
@@ -113,6 +114,12 @@ void pal_os_event_disarm(void)
    its.it_interval.tv_sec = 0;
    its.it_interval.tv_nsec = 0;

+   if ( g_timer_destroyed == TRUE ) {
+       TRUSTM_PAL_EVENT_DBGFN("Timer has been destroyed already");
+       TRUSTM_PAL_EVENT_DBGFN("<");
+       return;
+   }
+
    if (timer_settime(timerid, 0, &its, NULL) == -1)
    {
        printf("Error in timer_settime\n");
@@ -132,6 +139,12 @@ void pal_os_event_arm(void)
    its.it_interval.tv_sec = 0;
    its.it_interval.tv_nsec = 294967296;

+   if ( g_timer_destroyed == TRUE ) {
+       TRUSTM_PAL_EVENT_DBGFN("Timer has been destroyed already");
+       TRUSTM_PAL_EVENT_DBGFN("<");
+       return;
+   }
+
    if (timer_settime(timerid, 0, &its, NULL) == -1)
    {
        printf("Error in timer_settime\n");
@@ -178,6 +191,7 @@ pal_os_event_t * pal_os_event_create(register_callback callback, void * callback
             printf("timer_create\n");
             exit(1);
         }
+        g_timer_destroyed = FALSE;

         pal_os_event_start(&pal_os_event_0,callback,callback_args);
     }
@@ -194,6 +208,12 @@ void pal_os_event_trigger_registered_callback(void)

     TRUSTM_PAL_EVENT_DBGFN(">");

+    if (g_timer_destroyed == TRUE) {
+        TRUSTM_PAL_EVENT_DBGFN("Timer has been destroyed already");
+        TRUSTM_PAL_EVENT_DBGFN("<");
+        return;
+    }
+
     its.it_value.tv_sec = 0;
     its.it_value.tv_nsec = 0;
     its.it_interval.tv_sec = 0;
@@ -228,6 +248,12 @@ void pal_os_event_register_callback_oneshot(pal_os_event_t * p_pal_os_event,

     TRUSTM_PAL_EVENT_DBGFN(">");

+    if (!p_pal_os_event) {
+        TRUSTM_PAL_EVENT_DBGFN("pal_os_event already destroyed");
+        TRUSTM_PAL_EVENT_DBGFN("<");
+        return;
+    }
+
     //uint8_t scheduler_timer;
     p_pal_os_event->callback_registered = callback;
     p_pal_os_event->callback_ctx = callback_args;
@@ -239,6 +265,12 @@ void pal_os_event_register_callback_oneshot(pal_os_event_t * p_pal_os_event,
     its.it_interval.tv_sec = 0;
     its.it_interval.tv_nsec = 0;

+    if ( g_timer_destroyed == TRUE ) {
+        TRUSTM_PAL_EVENT_DBGFN("Timer has been destroyed already");
+        TRUSTM_PAL_EVENT_DBGFN("<");
+        return;
+    }
+
     if (( ret = timer_settime(timerid, 0, &its, NULL)) == -1)
     {
         int errsv = errno;
@@ -260,6 +292,9 @@ void pal_os_event_register_callback_oneshot(pal_os_event_t * p_pal_os_event,
 void pal_os_event_destroy(pal_os_event_t * pal_os_event)
 {
     TRUSTM_PAL_EVENT_DBGFN(">");
+    pal_os_event_disarm();
+    g_timer_destroyed = TRUE;
+    pal_os_event_0.callback_registered = NULL;
     timer_delete(timerid);
     TRUSTM_PAL_EVENT_DBGFN("<");

However, even when implementing such checks, an immediate dropping of optiga_util instance right after calling optiga_util_destroy() will lead to more issues. Even if a posix timer have been stopped, it might still fire a last time.

IMHO, the optiga_util_destroy() should make sure that all resources incl timers have been cleaned before exiting.

There a couple remarks I'd like to add for the linux pal_os_event.c:

• printing (printf) in signal handlers should not be used; code snippet above doesn't cause an issue because TRUSTM_PAL_EVENT_DEBUG is not set
• calling a user provided function from a signal handler should be async safe (see also previous point)
• only one pal_os_event_t could ever be created, as the create function returns a pointer to compilation unit local instance?! The API suggests differently!

Steps to Reproduce:

1. compile no debug

2. run

Expected Result:

clean deletion of optiga related instances

Actual Result:

segmentation faults

Build/Commit:

91d952b4a6433bdbe1979487b73972b708d89b3f

Target:

SLS32AIA010MK via I²C

Host OS and Version:

GNU/Linux 5.15.0-48 x86_64

Compiler:

aarch64-unknown-linux-gnu-gcc (GCC) 9.3.0

Environment

GNU/Linux 5.4.61 aarch64