Bug in the LwIP ethernet driver

rojer — Mon, 24 Oct 2016 15:12:49 GMT

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!

Re: Bug in the LwIP ethernet driver

rojer — Mon, 24 Oct 2016 16:32:02 GMT

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

Re: Bug in the LwIP ethernet driver

manfredschnell — Tue, 25 Oct 2016 05:33:23 GMT

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

topic Bug in the LwIP ethernet driver in Kinetis Software Development Kit

Bug in the LwIP ethernet driver

Re: Bug in the LwIP ethernet driver

Re: Bug in the LwIP ethernet driver