Re: [lwip-users] Memory leak due to synthesized DDoS

[Stephen Cowell]

I found my answer... forgive the top-post.

Turns out that, curiously enough, TCP is not HTTP... you probably knew 
that.  When carving out a Modbus TCP solution I used the HTTP code in 
lwIP 1.4.1... and got it working... but I left in the http_close_conn() 
at the end of tcpModBus_recv() (my hack of http_recv()).  This messed up 
the handling of pbufs etc... TCP is a session-oriented thing, HTTP is 
single transactions, hence the close() at the end of each.

I took out all the 2.1 stuff I'd kludged in, not necessary, had nothing 
to do with the solution.  Went back to lwIP pool handling etc.  Learned 
a lot.

Here's a skeleton for Modbus TCP to replace http_recv().

<>

static err_t tcpModBus_recv(void *arg, struct tcp_pcb *pcb, struct pbuf 
*p, err_t err)
{
    int i;
    char *data;
    char out[64];
    uint8_t *ui_Ptr;
    int size,function;

    if (err == ERR_OK && p != NULL)
    {
        /* Inform TCP that we have taken the data. */
        tcp_recved(pcb, p->tot_len);

        if (1)// (hs->file == NULL)
        {
            data = p->payload;
            size = data[11];// this is a word count of the expected payload
            function = data[7];// here is the FC command byte

            for (i=0;i<13;i++)
                out[i] = data[i];// the rest of this will be 
overwritten if needed.

            int len = data[11];// how many inputs
            int start = data[8];// starting channel address
            start <<=8;
            start +=  data[9];// probably high byte of this never used!

            int dataVal = data[10];
            dataVal <<= 8;
            dataVal += data[11];// this is used for some writes to us

            if ((start < MODBUS_START_ADDRESS)||(start > 
MODBUS_END_ADDRESS))// define these by your desired register map
            {
                out[5] = 3;// bytes following
                out[7] |= 128;// set high bit for exception, this is 
modbus function code, modified for error
                out[8] = 2;// illegal address

                pbuf_free(p);
                tcp_write(pcb, out, 9, TCP_WRITE_FLAG_COPY);
            }
            else// rest of channel test ifs... here I include a simple 
read of integer FW version
            if ((start >= 6000)&&(start<6500))// read accessory 
channels (version etc)
            {
                // test function code
                if (out[7] != 4)// not FC4 Read Input Registers
                {
                    out[5] = 3;// bytes following
                    out[7] |= 128;// set high bit for exception, this 
is modbus function code, modified for error
                    out[8] = 1;// illegal FC

                    pbuf_free(p);
                    tcp_write(pcb, out, 9, TCP_WRITE_FLAG_COPY);
                }
                else
                {
                    int channel = start - 6000;// starting channel 
address    NOT USED HERE... only decoding one channel, 6000

                    out[5] = 3+(size*2);// byte count including header
                    out[8] = size*2;// they ask for words... we send 
back bytes... this is why size*2 was originally used

                    out[9] = INT_HEADER>>8;
                    out[10] = INT_HEADER;// this is the two-byte BCD FW 
version

                    pbuf_free(p);
                    tcp_write(pcb, out, 9+size*2, 
TCP_WRITE_FLAG_COPY);// header ends at 9
                }
            }
            else
            {
                pbuf_free(p);// must always do this
            }
        }// if 1
    }//if (err == ERR_OK

// here's where I removed the http_close_conn()

    return ERR_OK;
}

</>

You'll have to set up your callbacks etc.  Working very nicely now. 
Thanks for your help folks.
__
Steve
.


Stephen Cowell
Project Manager/Engineer
Plasmability LLC
Office (512) 267-7087
Cell  (512) 632-8593 (best)
www.plasmability.com

On 10/25/2022 8:46 PM, Stephen Cowell wrote:
> My product is a modbus TCPIP board, using the Atmel SAM4E16E based off 
> of the SAM4E-EK.  It is using LWiP 1.4.1 with pbuf.c and pbuf.h from 
> version 2.1.3.  I know... cringe... but I did this recently, during 
> troubleshooting... helped some, see below.
>
> I'm banging it real hard using DaqFactory to create two simultaneous 
> threads sending modbus requests to the same port/address.  As I do 
> this I can see the pbuf_pool pbufs being created higher and higher up 
> the pool... I can debug on the pbuf deallocation and see that the 
> ->next is NULL, so there is no connection to the rest of the chain.  
> If I let this go on for long enough the processor will hard fault.
>
> I had this behavior happen very quickly before I moved from LWiP mem 
> management to GNU C library malloc()... now it takes a long time to 
> die.  Also, after I spliced in the pbuf code from the latest-greatest 
> it did seem to become more robust... but I can still watch the 
> allocated pbufs climb up the pool, leaving behind orphan pbufs as a 
> memory leak.
>
> I guess my main question is... should LWiP be able to survive this 
> kind of abuse?  I'm able to hit it with requests about every two 
> milliseconds... and it takes about an hour before it runs out of 
> memory.  Would I notice any improvement by completing the port to 
> 2.1.3 (a significant effort)?  Is LWiP tested in this way?  It's worth 
> noting that DaqFactory is glitching the inputs it receives 
> sometimes... but Wireshark shows them to be rock solid (when there's 
> not a timeout or port locked complaint).  Am I abusing it too hard? 
> Thanks for your time... Steve.
>
>
> _______________________________________________
> lwip-users mailing list
> lwip-users@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/lwip-users