Lora-net/LoRaMac-node

TimerInit called multiple times on the same TimerEvent_t struct

Opened this issue · 1 comments

Hi,

On a Class B device, calling LmHandlerJoin will call LmHandlerJoinRequest, which calls LoRaMacMlmeRequest, which calls ResetMacParameters, which calls LoRaMacClassBInit.

The issue is that the 3 timers in that function will get TimerInit called on them multiple times if there's multiple join requests. TimerInit will set a TimerEvent_t's Next pointer to NULL, which can mess up the timer linked list. Our internal fix was to check if the timers were initialized prior to calling TimerInit:

void LoRaMacClassBInit( LoRaMacClassBParams_t *classBParams, LoRaMacClassBCallback_t *callbacks, LoRaMacClassBNvmData_t* nvm )
{
#ifdef LORAMAC_CLASSB_ENABLED
    static bool timersInitialized = false;

    // Assign non-volatile context
    if( nvm == NULL )
    {
        return;
    }
    ClassBNvm = nvm;

    // Store callbacks
    Ctx.LoRaMacClassBCallbacks = *callbacks;

    // Store parameter pointers
    Ctx.LoRaMacClassBParams = *classBParams;

    // Initialize timers
    if ( !timersInitialized )
    {
        timersInitialized = true;
        TimerInit( &Ctx.BeaconTimer, LoRaMacClassBBeaconTimerEvent );
        TimerInit( &Ctx.PingSlotTimer, LoRaMacClassBPingSlotTimerEvent );
        TimerInit( &Ctx.MulticastSlotTimer, LoRaMacClassBMulticastSlotTimerEvent );
    }

    InitClassB( );
#endif // LORAMAC_CLASSB_ENABLED
}

I can submit a PR if you think that's an acceptable fix.

This might be a better fix (not sure if stopping the timers is necessary but it makes me feel better)

void LoRaMacClassBInit( LoRaMacClassBParams_t *classBParams, LoRaMacClassBCallback_t *callbacks, LoRaMacClassBNvmData_t* nvm )
{
#ifdef LORAMAC_CLASSB_ENABLED
    static bool timersInitialized = false;

    // Assign non-volatile context
    if( nvm == NULL )
    {
        return;
    }
    ClassBNvm = nvm;

    // Store callbacks
    Ctx.LoRaMacClassBCallbacks = *callbacks;

    // Store parameter pointers
    Ctx.LoRaMacClassBParams = *classBParams;

    // Initialize timers
    if ( !timersInitialized )
    {
        timersInitialized = true;
        TimerInit( &Ctx.BeaconTimer, LoRaMacClassBBeaconTimerEvent );
        TimerInit( &Ctx.PingSlotTimer, LoRaMacClassBPingSlotTimerEvent );
        TimerInit( &Ctx.MulticastSlotTimer, LoRaMacClassBMulticastSlotTimerEvent );
    }
    else
    {
        if (Ctx.BeaconTimer.IsStarted)        { TimerStop(&Ctx.BeaconTimer); }
        if (Ctx.PingSlotTimer.IsStarted)      { TimerStop(&Ctx.PingSlotTimer); }
        if (Ctx.MulticastSlotTimer.IsStarted) { TimerStop(&Ctx.MulticastSlotTimer); }
    }

    InitClassB( );
#endif // LORAMAC_CLASSB_ENABLED
}