FreeRTOS / coreMQTT

Client implementation of the MQTT 3.1.1 specification for embedded devices
MIT License
140 stars 100 forks source link

sendBuffer may fail because of tick timer rollover #286

Closed dtaarondecker closed 4 months ago

dtaarondecker commented 4 months ago

In core_mqtt.c, sendBuffer a timeout value is calculated by reading the current time and adding some timeout to that value. https://github.com/FreeRTOS/coreMQTT/blob/58d626a25839ed20b19a57fdff34a41f36dacd05/source/core_mqtt.c#L877

Take the case where current time is very close to, but not yet rolled over. We then add some timeout value causing a rollover on the timeoutMs variable.

We then check if the current time is greater than or equal to the timeout time

https://github.com/FreeRTOS/coreMQTT/blob/58d626a25839ed20b19a57fdff34a41f36dacd05/source/core_mqtt.c#L912

This check will erroneously fail.

For Example:

current time = 4294957296
timeoutMs = current time + 20000 = 20000

*continue executing*

Current Time = current time + ΔT where ΔT is less than 2^32 - current time 

currentTime >= timeoutMs = true until current time rolls over.
paulbartell commented 4 months ago

@dtaarondecker Thanks for the bug report. If you would like, feel free to open a pull request fixing the issue. Otherwise, we will get to it soon.