From 3190d292523d9af4bbc5c899759e54ddcf97e9e5 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 19 Aug 2020 14:25:14 +0100 Subject: [PATCH] Fix memory leak on handling QoS 2 PUBLISH. In some circumstances, Mosquitto could leak memory when handling PUBLISH messages. This is limited to incoming QoS 2 messages, and is related to the combination of the broker having persistence enabled, a clean session=false client, which was connected prior to the broker restarting, then has reconnected and has now sent messages at a sufficiently high rate that the incoming queue at the broker has filled up and hence messages are being dropped. This is more likely to have an effect where max_queued_messages is a small value. This has now been fixed. Closes #1793. --- ChangeLog.txt | 13 +++++++++++++ src/database.c | 4 ++-- src/handle_publish.c | 20 +++++++++++++++++--- src/mosquitto_broker_internal.h | 2 ++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 760d7414..eee6a85b 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,16 @@ +1.5.10 - 2020-08-19 +=================== + +Security: +- In some circumstances, Mosquitto could leak memory when handling PUBLISH + messages. This is limited to incoming QoS 2 messages, and is related + to the combination of the broker having persistence enabled, a clean + session=false client, which was connected prior to the broker restarting, + then has reconnected and has now sent messages at a sufficiently high rate + that the incoming queue at the broker has filled up and hence messages are + being dropped. This is more likely to have an effect where + max_queued_messages is a small value. This has now been fixed. Closes #1793. + 1.5.9 - 20190917 ================ diff --git a/src/database.c b/src/database.c index 9ef866e1..2c357cad 100644 --- a/src/database.c +++ b/src/database.c @@ -36,7 +36,7 @@ static unsigned long max_queued_bytes = 0; * @param qos qos for the packet of interest * @return true if more in flight are allowed. */ -static bool db__ready_for_flight(struct mosquitto *context, int qos) +bool db__ready_for_flight(struct mosquitto *context, int qos) { if(qos == 0 || (max_inflight == 0 && max_inflight_bytes == 0)){ return true; @@ -64,7 +64,7 @@ static bool db__ready_for_flight(struct mosquitto *context, int qos) * @param qos destination qos for the packet of interest * @return true if queuing is allowed, false if should be dropped */ -static bool db__ready_for_queue(struct mosquitto *context, int qos) +bool db__ready_for_queue(struct mosquitto *context, int qos) { if(max_queued == 0 && max_queued_bytes == 0){ return true; diff --git a/src/handle_publish.c b/src/handle_publish.c index b395cb33..e798afed 100644 --- a/src/handle_publish.c +++ b/src/handle_publish.c @@ -184,9 +184,23 @@ int handle__publish(struct mosquitto_db *db, struct mosquitto *context) db__message_store_find(context, mid, &stored); } if(!stored){ - dup = 0; - if(db__message_store(db, context, mid, topic, qos, payloadlen, &payload, retain, &stored, 0)){ - return 1; + if(qos == 0 + || db__ready_for_flight(context, qos) + || db__ready_for_queue(context, qos)){ + + dup = 0; + if(db__message_store(db, context, mid, topic, qos, payloadlen, &payload, retain, &stored, 0)){ + return 1; + } + }else{ + /* Client isn't allowed any more incoming messages, so fail early */ + mosquitto__free(topic); + UHPA_FREE(payload, payloadlen); + if(qos == 1){ + return send__puback(context, mid); + }else{ + return send__pubrec(context, mid); + } } }else{ mosquitto__free(topic); diff --git a/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 5c077cb4..75bbc28a 100644 --- a/src/mosquitto_broker_internal.h +++ b/src/mosquitto_broker_internal.h @@ -567,6 +567,8 @@ void db__msg_store_deref(struct mosquitto_db *db, struct mosquitto_msg_store **s void db__msg_store_clean(struct mosquitto_db *db); void db__msg_store_compact(struct mosquitto_db *db); int db__message_reconnect_reset(struct mosquitto_db *db, struct mosquitto *context); +bool db__ready_for_flight(struct mosquitto *context, int qos); +bool db__ready_for_queue(struct mosquitto *context, int qos); void sys_tree__init(struct mosquitto_db *db); void sys_tree__update(struct mosquitto_db *db, int interval, time_t start_time);