[Tinyos-devel] CRC bug

[Razvan Musaloiu-E.]

Hi!

I think I might spotted one case in which can a bad packet can hit the 
upper layer. This is also from CC2420ReceiveP:

    209          if(rxFrameLength <= MAC_PACKET_SIZE) {
    210            if(rxFrameLength > 0) {
    211              if(rxFrameLength > SACK_HEADER_LENGTH) {
    212                // This packet has an FCF byte plus at least one more byte to read
    213                call RXFIFO.continueRead(buf + 1, SACK_HEADER_LENGTH);
    214
    215              } else {
    216                // This is really a bad packet, skip FCF and get it out of here.
    217                m_state = S_RX_PAYLOAD;
    218                call RXFIFO.continueRead(buf + 1, rxFrameLength);
    219              }
    220
    221            } else {
    222              // Length == 0; start reading the next packet
    223              atomic receivingPacket = FALSE;
    224              call CSN.set();
    225              call SpiResource.release();
    226              waitForNextPacket();
    227            }
    228

The transition to S_RX_PAYLOAD from line 217 would allow the control to 
reach the line 290 and potentially get to the signal of receive. Is the 
rx_buf suppose to prevent this?

All the best in the new year to everyone!
Razvan ME

On Mon, 31 Dec 2007, Razvan Musaloiu-E. wrote:

> Hi!
>
> This is surprising. After adding the filtering I mentioned in my previous 
> email I stopped seeing corrupted packets at the application level.
>
> --
> Razvan ME
>
> On Mon, 31 Dec 2007, David Moss wrote:
>
>> I know it's obscure (HIL level after all), but take a look at
>> CC2420ReceiveP, line 290:
>>
>>      // buf[rxFrameLength] >> 7 checks the CRC
>>      if ( ( buf[ rxFrameLength ] >> 7 ) && rx_buf ) {
>>        uint8_t type = ( header->fcf >> IEEE154_FCF_FRAME_TYPE ) & 7;
>>        signal CC2420Receive.receive( type, m_p_rx_buf );
>>        if ( type == IEEE154_TYPE_DATA ) {
>>          post receiveDone_task();
>>          return;
>>        }
>>      }
>> 
>> -David
>> 
>> 
>> 
>> -----Original Message-----
>> From: Razvan Musaloiu-E. [mailto:razvanm at cs.jhu.edu]
>> Sent: Wednesday, December 26, 2007 5:16 PM
>> To: David Moss
>> Cc: Vlado Handziski; Kevin Klues; TinyOS Development
>> Subject: Re: [Tinyos-devel] CRC bug
>> 
>> Hi!
>> 
>> On Fri, 7 Dec 2007, Vlado Handziski wrote:
>> 
>>> My recollection is that his has been done in HW on the CC2420, but this is
>>> before the new stack was introduced.
>> 
>> The CC2420 chip verifies the CRC but it doesn't do any filtering based on
>> this. David, can you please confirm this? :-)
>> 
>> Below is a patch that does the filtering in CC2420ActiveMessageP.nc.
>> 
>> diff --git a/tos/chips/cc2420/CC2420ActiveMessageP.nc b/tos/chips/cc2420/CC2420ActiveMessageP.nc
>> index 2cd35c8..4e1a452 100644
>> --- a/tos/chips/cc2420/CC2420ActiveMessageP.nc
>> +++ b/tos/chips/cc2420/CC2420ActiveMessageP.nc
>> @@ -164,6 +164,10 @@ implementation {
>>
>>    /***************** SubReceive Events ****************/
>>    event message_t* SubReceive.receive(message_t* msg, void* payload, uint8_t len) {
>> +    cc2420_metadata_t* header = call CC2420PacketBody.getMetadata(msg);
>> +    if (header->crc == 0) {
>> +      return msg;
>> +    }
>>      if (call AMPacket.isForMe(msg)) {
>>        return signal Receive.receive[call AMPacket.type(msg)](msg, payload, len - CC2420_SIZE);
>>      }
>> 
>> Happy holidays to everyone!
>> Razvan ME
>> 
>>> On Dec 7, 2007 10:31 PM, Kevin Klues <klueska at gmail.com> wrote:
>>> 
>>>> Does anyone know why the crc footer bit is never checked anywhere in
>>>> the entire tinyos-2.x tree?  In tinyos-1.x the radio stack passed the
>>>> packet up to the AM layer (AMPromiscuous.nc, AMStandard.nc) which
>>>> checked to see if  (footer->crc == 1) before passing the packet up the
>>>> stack.  The AM layer contains no such functionality in T2.
>>>> Because of this, a disconcerting number of packets (at least on motes
>>>> that use a cc1000) obtained by the application layer end up bing
>>>> corrupted.
>>>> 
>>>> This is obviously a bug, the question is where this filtering should
>>>> actually be done.  As I mentioned before, in tinyos-1.x it was done at
>>>> the AM layer.  Is that where we want to keep it, or should it be
>>>> pushed down into the radio stacks themselves?  In what cases will we
>>>> ever want to receive a packet if it has a bad crc?  I can think of
>>>> situations where this might be useful (i.e. doing forward error
>>>> correction or something), but if such functionality were implemented
>>>> it should exist below the AM layer.
>>>> I guess my real question then, is where should we put this filtering
>>>> NOW so taht corrupted packets dont get sent up to the application.
>>>> 
>>>> --
>>>> ~Kevin
>>>> 
>>>> 
>>> 
>> 
>> 
>