[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
URL:
  <https://savannah.nongnu.org/bugs/?55607>

                 Summary: MQTT assert when length of received message >
MQTT_VAR_HEADER_BUFFER_LEN
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: dgirault
            Submitted on: mer. 30 janv. 2019 10:16:41 UTC
                Category: apps
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: git head

    _______________________________________________________

Details:

The following assert in mqtt.c line 684, added by commit
eea95459c9b0f150e9b9ab24e80e27e09c2612e5, completely broke MQTT reception.

LWIP_ASSERT("client->msg_idx < MQTT_VAR_HEADER_BUFFER_LEN", client->msg_idx <
MQTT_VAR_HEADER_BUFFER_LEN);


MQTT had always the ability to receive message longer than
MQTT_VAR_HEADER_BUFFER_LEN. Receive callback is called many times.






    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Additional Item Attachment, bug #55607 (project lwip):

File name: 0001-mqtt-remove-bad-assert-in-mqtt_message_received.patch Size:6
KB
   
<https://savannah.nongnu.org/file/0001-mqtt-remove-bad-assert-in-mqtt_message_received.patch?file_id=46133>



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Update of bug #55607 (project lwip):

                  Status:                    None => Fixed                  
             Assigned to:                    None => goldsimon              
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #1:

Pushed, thanks for the patch!

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Follow-up Comment #2, bug #55607 (project lwip):

Seen another problem with mqtt_parse_incoming() that may not call
mqtt_message_received() if remaining part of current buffer isn't equal to
buffer_space!
This is almost all time the case when called from altcp_tls socket.

The second patch address this and additionally allow working mostly in
zero-copy mode.

Using a MQTT_VAR_HEADER_BUFFER_LEN of 1520 will allow zero-copy for all
received frame (when connected through TLS).

In our architecture, working on pbuf is faster than in `client->rx_buffer`
because not in same memory space (SRAM vs. SDRAM).


(file #46134)
    _______________________________________________________

Additional Item Attachment:

File name: 0002-mqtt-support-mostly-zero-copy-message-analysis.patch Size:4 KB
   
<https://savannah.nongnu.org/file/0002-mqtt-support-mostly-zero-copy-message-analysis.patch?file_id=46134>



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Follow-up Comment #3, bug #55607 (project lwip):

Arg, I didn't see that you had pushed this bugfix when a added the comment
#2.

Do you want I create another bug report for the second patch?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Update of bug #55607 (project lwip):

                  Status:                   Fixed => In Progress            
             Open/Closed:                  Closed => Open                  


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Follow-up Comment #4, bug #55607 (project lwip):

I'd apply it, but that doesn't work. Can you pull and rebase 0002?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Follow-up Comment #5, bug #55607 (project lwip):

No problem, I'll do that.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Follow-up Comment #6, bug #55607 (project lwip):

Hi Simon,

Just had time to rebase over master (ec11b289) and include an additional fix
in MQTT for a corner case that appears in our configuration.

See new attached patches.

Regards,
David

(file #46816, file #46817)
    _______________________________________________________

Additional Item Attachment:

File name: 0001-mqtt-support-mostly-zero-copy-message-analysis.patch Size:4 KB
   
<https://savannah.nongnu.org/file/0001-mqtt-support-mostly-zero-copy-message-analysis.patch?file_id=46816>

File name: 0002-mqtt-fix-first-packet-checking-which-fail-if-MQTT_VA.patch
Size:2 KB
   
<https://savannah.nongnu.org/file/0002-mqtt-fix-first-packet-checking-which-fail-if-MQTT_VA.patch?file_id=46817>



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[bug #55607] MQTT assert when length of received message > MQTT_VAR_HEADER_BUFFER_LEN

Wilfred
Update of bug #55607 (project lwip):

                  Status:             In Progress => Fixed                  
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #7:

Applied, thanks!

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?55607>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel