Skip to content

Commit

Permalink
Issue #752 - Applied proposed enhancement.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mluis1
mluis1 committed Jul 2, 2019
1 parent 895b6cd commit 1acc148
Showing 1 changed file with 2 additions and 14 deletions.
16 changes: 2 additions & 14 deletions src/mac/LoRaMacCommands.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ Maintainer: Miguel Luis ( Semtech ), Daniel Jaeckle ( STACKFORCE ), Johannes Bru
*/
#define CID_FIELD_SIZE 1

/*!
* List of all stick MAC command answers which will be deleted after a receiving downlink
*/
const uint8_t CIDsStickyAnsCmds[] = { MOTE_MAC_DL_CHANNEL_ANS, MOTE_MAC_RX_PARAM_SETUP_ANS, MOTE_MAC_RX_TIMING_SETUP_ANS };

/*!
* Mac Commands list structure
*/
Expand Down Expand Up @@ -460,16 +455,9 @@ LoRaMacCommandStatus_t LoRaMacCommandsRemoveStickyAnsCmds( void )
while( curElement != NULL )
{
nexElement = curElement->Next;
if( curElement->IsSticky == true )
if( IsSticky( curElement->CID ) == true )
{
for( uint8_t i = 0; i < sizeof( CIDsStickyAnsCmds ); i++ )
{
if( curElement->CID == CIDsStickyAnsCmds[i] )
{
LoRaMacCommandsRemoveCmd( curElement );
break;
}
}
LoRaMacCommandsRemoveCmd( curElement );
}
curElement = nexElement;
}
Expand Down

1 comment on commit 1acc148

@msmits-definium
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks V1.1:
After joining, the MAC command RekeyInd is sent as DataUp (CID 11). The server responds with RekeyConf. This is now removed from the mac commands list because there is no check with CIDsStickyAnsCmds[]. It is however also removed a second time in ProcessMacCommands, case SRV_MAC_REKEY_CONF.

        case SRV_MAC_REKEY_CONF:
        {
            uint8_t serverMinorVersion = payload[macIndex++];

            // Compare own LoRaWAN Version with server's
            if( MacCtx.NvmCtx->Version.Fields.Minor == serverMinorVersion )
            {
                // If they equal remove the sticky RekeyInd MAC-Command.
                LoRaMacCommandsGetCmd( MOTE_MAC_REKEY_IND, &macCmd);
                LoRaMacCommandsRemoveCmd( macCmd );
            }
            break;
        }

Because there is no check on the return value of LoRaMacCommandsGetCmd, the remove proceeds and results in an invalid deletion on the underlying linked list, It causes a hardfault on my LPC 54628.

See trace below which gives an idea of th flow:

[lorawan] initialisation
[lorawan] join;attempt=0
[lorawan] join=executing
[lorawan] lwn_network_task RECV
DEBUG - ProcessRadioTxDone
$ [lorawan] lwn_network_task RECV
DEBUG - ProcessRadioRxDone
LoRaMacCommandsAddCmd - cid: 11 <---- mac command added
DEBUG - ProcessRadioRxDone - exit
[lorawan] JOIN OK
[lorawan] MIB_DEV_ADDR: FC0023C4
[lorawan] join=success
[lorawan] session=get
[lorawan] SEND START
[lorawan] lwn_network_task RECV
DEBUG - ProcessRadioTxDone
[lorawan] lwn_network_task RECV
DEBUG - ProcessRadioRxDone
LoRaMacCommandsRemoveCmd - cid: 11 <---- mac command removed first time, OK
ProcessMacCommands - payload[macIndex]: 11
LoRaMacCommandsRemoveCmd - cid: 181 <----- mac command remove attempted 2nd time but on empty list, FAIL

Please sign in to comment.