Two issues with CC310 HW CryptoCell: Linking and hanging

[jakkra]

Hi, we stumbled into two issues when we enabled LESC in our application.

Relevant SDK config:

NRF_CRYPTO_ENABLED

NRF_CRYPTO_BACKEND_CC310_ENABLED

NRF_CRYPTO_BACKEND_CC310_RNG_ENABLED

First one is that pm_init() will hang if called when NRF_CRYPTO_BACKEND_CC310_RNG_ENABLED unless we call sd_nvic_critical_region_enter before and then sd_nvic_critical_region_exit after. I can't find anything documentation saying that we need to do that. See attached stacktrace when calling pm_init() without entering critical region. It's the CRYS_RndInit() which hangs and causes the WD to reset.

Second issue I think is related to linking. 

I have a theory that the linker does not link cc310_backend_init.o file as the two public functions void cc310_backend_enable(void); and void cc310_backend_disable(void); are
found in cc310_backend_shared.c and therefore CRYPTO_BACKEND_REGISTER does not
place cc310_backend_init and cc310_backend_uninit in the crypto_data section in flash.

Causing that cc310_backend_init() is never called when initiating as it's not found in the.crypto_data 

If I add a dummy function to cc310_backend_init.c the two functions are placed in the .crypto_data section in the flash. See the attached patch.txt file.

Fullscreen patch.txt Download

diff --git a/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/backend/cc310/cc310_backend_init.c b/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/backend/cc310/cc310_backend_init.c
index e3e77f0b..05f29edb 100644
--- a/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/backend/cc310/cc310_backend_init.c
+++ b/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/backend/cc310/cc310_backend_init.c
@@ -57,6 +57,18 @@ void cc310_backend_enable(void);
  */
 void cc310_backend_disable(void);
 
+// patch start
+uint32_t forceLinkingcc310BackendInit()
+{
+    /*
+    The linker does not link this file as the above two functions are found in cc310_backend_shared.c
+    And therefore CRYPTO_BACKEND_REGISTER does not place cc310_backend_init cc310_backend_uninit 
+    in the crypto_data section in flash. This is a workaround for not ignoring this file.
+    */
+    return 0;
+}
+//patch end
+
 static uint32_t init_result_get(uint32_t crys_error)
 {
     uint32_t ret_val = NRF_ERROR_INTERNAL;
     
--- a/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.c
+++ b/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.c
@@ -79,6 +79,11 @@ ret_code_t nrf_crypto_init(void)
             return ret_val;
         }
     }
+    // patch start
+#if NRF_MODULE_ENABLED(NRF_CRYPTO_BACKEND_CC310)
+    forceLinkingcc310BackendInit();
+#endif
+    // patch end
 
     // Set nrf_crypto to initialized
     m_state = INITIALIZED;
diff --git a/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.h b/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.h
index 8e10ae13..a90125b0 100644
--- a/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.h
+++ b/bsp/nRF5_SDK_15.2.0_9412b96/components/libraries/crypto/nrf_crypto_init.h
@@ -136,6 +136,10 @@ bool nrf_crypto_is_initialized(void);
  */
 bool nrf_crypto_is_initializing(void);
 
+// patch start
+uint32_t forceLinkingcc310BackendInit();
+// patch end
+
 #ifdef __cplusplus
 }
 #endif

SoftDevice: s140_nrf52_6.1.0

SDK: nRF5_SDK_15.2.0_9412b96

[Einar Thorsrud]

Hi,

1. I do not recall seeing this issue before, nor do I immediately understand what is going on. You cannot use CC310 concurrently, and therefore the backed wrapper (cc310_backend_rng.c) use the CC310 mutex. I do not see any reason for using critical sections, though. Therefore, I have a few questions just to understand more about the situation:

• How do you handle memory for the RNG module?
• Is NRF_CRYPTO_RNG_STATIC_MEMORY_BUFFERS_ENABLED set to 1 in the projects sdk_config.h?
• Since using critical sections seem to help - what do you expect is interrupting when initializing the RNG?
• Can you provide an example project that demonstrate this?

2. This is also new to me, so I have a few questions here as well:

• Which toolchain do you use?
• Does it depend on optimization level or other settings?
• Can you provide an example project that demonstrate this?

[jakkra]

Hi, thanks for looking into this:

1.

- NRF_CRYPTO_RNG_STATIC_MEMORY_BUFFERS_ENABLED 1. I did also try with dynamic but did not make a difference. I have also tested with and without NRF_CRYPTO_RNG_AUTO_INIT_ENABLED 1.

- I managed to find what code makes CRYS_RndInit() hang. Anything obvious you can spot here which causes it? This is part of our initiation.

NVIC_SetPriority(DebugMonitor_IRQn, 7ul);

// IRQ configuration
sd_nvic_SetPriority(RTC2_IRQn, APP_IRQ_PRIORITY_LOW);
sd_nvic_ClearPendingIRQ(RTC2_IRQn);
sd_nvic_EnableIRQ(RTC2_IRQn);

// Start LFCLK clock
nrf_clock_task_trigger(NRF_CLOCK_TASK_LFCLKSTART);

// Configure RTC2 frequency
nrf_rtc_prescaler_set(NRF_RTC2, cbHW_PRESCALER_RTC2);

/* Disable interrupts and events before changing configuration. */
sd_nvic_DisableIRQ(RTC2_IRQn);
sd_nvic_ClearPendingIRQ(RTC2_IRQn);

nrf_rtc_task_trigger(NRF_RTC2, NRF_RTC_TASK_STOP);
nrf_rtc_int_disable(NRF_RTC2, NRF_RTC_INT_COMPARE0_MASK);
nrf_rtc_int_disable(NRF_RTC2, NRF_RTC_INT_TICK_MASK);
nrf_rtc_event_disable(NRF_RTC2, NRF_RTC_INT_COMPARE0_MASK);
nrf_rtc_event_disable(NRF_RTC2, NRF_RTC_INT_TICK_MASK);
sysTickMode = mode;

sd_nvic_ClearPendingIRQ(RTC2_IRQn);
sd_nvic_EnableIRQ(RTC2_IRQn);

nrf_rtc_event_enable(NRF_RTC2, NRF_RTC_INT_TICK_MASK);
nrf_rtc_int_enable(NRF_RTC2, NRF_RTC_INT_TICK_MASK);
nrf_rtc_task_trigger(NRF_RTC2, NRF_RTC_TASK_START);
break;

flowControl = NRF_POWER->GPREGRET >> 3;

NRF_POWER->GPREGRET = 0;

2. Don't have at the moment, but the code above should hopefully reproduce it.

- arm-eabi-gcc.exe (GCC) 6.2.0, GNU ar (GNU Binutils) 2.27. more exact: http://gnutoolchains.com/arm-eabi/ arm-eabi-gcc6.2.0-r4

- Same issue when we build with zero optimization. Problem is that in nrf_crypto_init NRF_CRYPTO_BACKEND_SECTION_ITEM_COUNT is 0 without the patch I showed in the issue.

- I might be able to create an example project some day, but first I think this issue should be tested with the same toolchain we use. I'll also try with the latest arm-eabi-gcc and see if it solves the problem.

[Einar Thorsrud]

Hi,

I am sorry for the late reply (I have been away for some days). This seems like a linker issue of some sort, but I don't know. I have not had a chance to try to reproduce this yet but will do that early next week. Please let me know if you have any new findings. In any case I will get back to you early next week.

[jakkra]

Hi, no problem. The workaround is fine for us right now, but it would be good if we could get rid of it in upcoming SDK releases. I did try with arm-eabi-gcc7.2.0-r3, but it did not compile our application, possibly some build settings need to be changed/updated, as I got some compile issues and didn't have time digging deeper into it.

[Einar Thorsrud]

Hi,

I did not notice earlier, but why do you use arm-eabi instead of arm-none-eabi? We only test with arm-none-eabi.

[jakkra]

Hi, thanks, that is wierd, yes we should use none-eabi. We will update and see if that solves the linking problem.

[jakkra]

Hi, we have now upgraded to arm-none-eabi 8.3 and still have the exact same problems. So the issue is not with the compiler version.

[Einar Thorsrud]

Hi,

Can you check if you see the same issue with GCC ARM Embedded 6.3 2017-q2-update, which is the tested/supported toolchain for SDK 15.2?

[pellepl]

Hi Einar,

tried with the 6.3 q2 toolchain without any luck.

I also tried removing `static` statement from the functions cc310_backend_init and cc310_backend_uninit without any result.

And also tried adding __attribute__((used)) to the same functions - still they do not show up in the .crypto_data section.

Tried removing gc-sections from linker flags just to try, but unfortunately this made sections overflow and could not get any leads from this.

Edit:

Further investigation indicates that the problem boils down to some issue in the libnrf_cc310_0.9.1.a.

If I add print-gc-sections to our linker, and diff the output with and without jakkras fix, it turns out that following symbols are garbage collected by the linker without the fix:

'.text.SaSi_PalInit'
'.text.SaSi_PalTerminate'
'.rodata.SaSi_PalInit.str1.4'
'.text.SaSi_PalDmaInit'
'.text.SaSi_PalDmaTerminate'
'.text.SaSi_PalMutexCreate'
'.text.SaSi_PalMutexDestroy'
'.rodata.SaSi_PalMutexCreate.str1.4'
'.text.SaSi_HalInit
'.text.SaSi_HalTerminate'

According to docs SaSi_PalInit should be called by SaSi_LibInit. SaSi_LibInit is called from

static ret_code_t cc310_backend_init(void)

which in turn is registered as

CRYPTO_BACKEND_REGISTER(nrf_crypto_backend_info_t const cc310_backend) =
{
    .init_fn    = cc310_backend_init,
    .uninit_fn  = cc310_backend_uninit,
};

in your cc310_backend_init.c.

It is weird that SaSi_LibInit is not gc'ed but SaSi_PalInit is (without the fix). Again, it suggests something odd in libnrf_cc310_0.9.1.a.

Cheers / Peter

[Einar Thorsrud]

Hi Peter,

Do you by any chance build the SDK as a library, and then compile your application separately, linking it to the SDK? If so, that could explain this, as section variables require that everything is compiled together, if not they will not be populated.

[pellepl]

Ah, yes, you're right. The SDK is built and linked as a static library.

This might be of an esoteric nature, but do you have any clue at all why it then works by adding a call to the dummy function?

Thanks for all help! / Peter

[Einar Thorsrud]

That is probably the explanation, then. I recommend you stop building the SDK as a library, as that has been known to cause several problems. The dummy function probably works because it make the linker aware that the function is being used (it is missing from the section variable where it should have been), and that causes it to include it.