Bug in the LwIP ethernet driver

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Bug in the LwIP ethernet driver

2,008 Views
rojer
Contributor II

ethernetif.c has this code to deal with an incoming frame:

 

        /* Call ENET_ReadFrame when there is a received frame. */
        if (length != 0)
        {
            uint8_t *receivedDataBuffer;
            struct pbuf *packetBuffer;
            packetBuffer = pbuf_alloc(PBUF_RAW, length, PBUF_POOL);
            /* Received valid frame. Deliver the rx buffer with the size equal to length. */
            if ((packetBuffer == NULL) || (packetBuffer->payload == NULL))
            {
                LWIP_ASSERT("Fail to allocate new memory space", 0);
                /* Should not reach this statement if error occurs */
                return;
            }
            receivedDataBuffer = packetBuffer->payload;
            packetBuffer->len = (u16_t)length;
            ENET_ReadFrame(ENET, handle, receivedDataBuffer, packetBuffer->len);

 

the problem with this code is that it assumes that all length bytes will be allocated in one contiguous buffer, which may not be the case: pbuf_alloc(PBUF_POOL) can return a chain of pbufs adding up to the requested length.

in that case packetBuffer->len is not the same as length - instead, length of the entire chain is held in packetBuffer->tot_len

 

to illustrate, a concrete example i've just spent considerable time debugging: 704 bytes are allocated as a chain of 2 pbufs, 636 and 68 bytes. packetBuffer points at the first buffer, packetBuffer->len is 636, packetBuffer->tot_len is 704 and packetBuffer->next is non-NULL, pointing to the second 64 pbuf. however, at line 15 packetBuffer->len is rather unceremoniously stomped with 704 and we then proceed to call ENET_ReadFrame which will then copy 704 bytes to packetBuffer->payload, but which is not large enough to receive that amount - its size is 636 bytes... BANG! memory is stomped, all sorts of hell breaks loose and nuclear missiles are launched (maybe). 

 

it looks like whoever was writing this code is not familiar with lwip's pbuf chains, as evidenced by stomping of pbuf->len - that should never be done!

Labels (1)
0 Kudos
Reply
2 Replies

1,102 Views
manfredschnell
Contributor IV

Hi Deomid,

this LWIP issue is discussed in FreeRTOS LwIP SDK v2.0.0 tpcipecho demo Crash.

Please have a look on my function "ethernetif_Input()". There are the LWIP memory buffer chains handled correct.

Best regards

Manfred

0 Kudos
Reply

1,102 Views
rojer
Contributor II

as a quick fix, i changed pbuf type to PBUF_RAM, which is allocated on the heap and is guaranteed to be contiguous.

0 Kudos
Reply