buglwIP - A Lightweight TCP/IP stack - Bugs: bug #53103, MQTT corrupted mesages

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #53103: MQTT corrupted mesages

Submitter:  David Bourgeois <jaguarondi>
Submitted:  Fri 09 Feb 2018 02:44:47 AM UTC
   
 
Category:  apps Severity:  3 - Normal
Item Group:  Crash Error Status:  Works For Me
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Jump to the original submission

Mon 21 Aug 2023 03:16:53 PM UTC, comment #8: 

Am I reading bug #57734 correctly to be open and unassigned?

Roger Sala <rsala>
Sat 19 Aug 2023 08:10:00 AM UTC, comment #7: 

This is similar to what we found when the MQTT topic is in segmented packets, this is described in bug #57734

David Bourgeois <jaguarondi>
Fri 18 Aug 2023 04:49:48 PM UTC, comment #6: 

... so I turned on the MQTT_DEBUG_TRACE and was able to establish that my problem was occurring when the ring buffer rolled over and a mqtt publish became fragmented which caused the TLS layer on the broker machine to issue a "bad record MAC" alert and then the mosquitto broker to subsequently close the connection.

Not sure if this is a LwIP problem or an MbedTLS problem. I'm integrating MbedTLS 2.16.2 with LwIp through the altcp_tls interface.

Right now I've implemented the following grotesque hack as a workaround, but it would be great to know what the correct fix is. I'm sure other developers must be running into this same problem?

static void
mqtt_ringbuf_advance_get_idx(struct mqtt_ringbuf_t *rb, u16_t len)
{
  LWIP_ASSERT("mqtt_ringbuf_advance_get_idx: len < MQTT_OUTPUT_RINGBUF_SIZE", len < MQTT_OUTPUT_RINGBUF_SIZE);

  rb->get += len;
  if(rb->get > (MQTT_OUTPUT_RINGBUF_SIZE-8192))
  {
  rb->get = rb->put = 0;
  }
  /*RCSTODO rb->get += len;
  if (rb->get >= MQTT_OUTPUT_RINGBUF_SIZE) {
    rb->get = rb->get - MQTT_OUTPUT_RINGBUF_SIZE;
  }*/
}

Roger Sala <rsala>
Wed 16 Aug 2023 05:25:18 PM UTC, comment #5: 

Same MQTT ring buffer corruption issue with LwIP 2.1.2, MbedTLS, FreeRTOS on STM32H7. In my case the publishes are not taking place in a tight loop and if I publish at a lower frequency, the crash always takes place after the exact same number of packets sent. The bigger the ring buffer the longer it runs, but it always crashes as in the attached pcap.

(file #55051)

Roger Sala <rsala>
Fri 16 Feb 2018 11:05:59 AM UTC, comment #4: 

Cool, I think that's the first bug found with the new  LWIP_ASSERT_CORE_LOCKED() code :-)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 16 Feb 2018 10:53:54 AM UTC, comment #3: 

You are right, it was indeed a threading issue on our side, not calling LOCK_TCPIP_CORE and LWIP_ASSERT_CORE_LOCKED() indeed revealed the problem. Sorry for the trouble.

David Bourgeois <jaguarondi>
Wed 14 Feb 2018 08:04:39 PM UTC, comment #2: 

I just tested this and it works for me.

Instead of a tight loop, I used tcpip_callback() to schedule the next publish call as fast as possible. This gives me many ERR_MEM results and some ERR_OK, but it continued for half an hour before I stopped the test.

Whatever is wrong here, I don't think it's the mqtt code.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 14 Feb 2018 11:34:06 AM UTC, comment #1: 

Are you 100% sure this is no threading issue? That code executing mqtt_publish() in a tight loop looks suspicous! When correctly following threading rules, you'd have no chance to process TCP ACKs with this code.

Have you checked the new LWIP_ASSERT_CORE_LOCKED() helper?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 09 Feb 2018 02:44:47 AM UTC, original submission:  

In the attached wireshark capture, we can see a series of corrupted messages in packet 454,  which trigger a disconnection by the broker.

It is difficult to debug, but from what we could gather, it seems that an ERR_MEM produced by the altcp_write is not handled, and it somehow corrupts the ring buffer.

The capture has been produced by the following code:


extern mqtt_client_t *mqtt_client;
int ip[4] = {10,192,55,55};
mqtt_connect(ip, "client_test");
char topic[64];
char payload[64];
int  counter=0;
while(1){
    sprintf(topic, "topic/test/%d", counter);
    sprintf(payload, "packet %d", counter);
    mqtt_publish(mqtt_client, topic, payload, strlen(payload), MQTT_QOS_1, 0, NULL, NULL);
    ++counter;
    if (counter%10 == 0){
        osDelay(10);
    }
}


David Bourgeois <jaguarondi>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #55051:  mqtt.pcapng added by rsala (43KiB - application/x-pcapng)
file #43211:  mqtt_garbage_capture.pcapng.gz added by jaguarondi (13KiB - application/x-gzip - Wireshark capture of the bug triggered by the example code)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rsala (Updated the item)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jaguarondi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-08-16 rsala Attached File- Added mqtt.pcapng, #55051
    2018-02-14 goldsimon StatusNone Works For Me
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2018-02-09 jaguarondi Attached File- Added mqtt_garbage_capture.pcapng.gz, #43211

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code