From: W.C.A. Wijngaards Date: Wed, 17 Jun 2026 14:05:45 +0000 (+0200) Subject: - Fix after malloc failure for stats, then it drains the pipe X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=55e9532d16835185a18bbf71831e4b767221bf22;p=thirdparty%2Funbound.git - Fix after malloc failure for stats, then it drains the pipe so the internal messaging stays correct. Also it does not exit the server if stats pipe communication fails. Thanks to Qifan Zhang, Palo Alto Networks, for the report. --- diff --git a/daemon/stats.c b/daemon/stats.c index 43a9f7092..64a31ae39 100644 --- a/daemon/stats.c +++ b/daemon/stats.c @@ -422,12 +422,28 @@ void server_stats_obtain(struct worker* worker, struct worker* who, # endif #endif ); + log_err("server_stats_obtain: no response from worker %d " + "(stats timeout); returning zero stats for this worker", + who->thread_num); + /* A later reply from the worker, would be sizeof stats reply, + * and the worker_handle_control_cmd routine discards if + * it is not a 4byte command, when that is received here. */ + memset(s, 0, sizeof(*s)); + return; + } + if(!tube_read_msg(worker->cmd, &reply, &len, 0)) { + log_err("server_stats_obtain: failed to read stats from worker " + "(tube read error); returning zero stats for this worker"); + memset(s, 0, sizeof(*s)); + return; + } + if(len != (uint32_t)sizeof(*s)) { + log_err("server_stats_obtain: wrong stats length %d (expected %d); " + "discarding", (int)len, (int)sizeof(*s)); + free(reply); + memset(s, 0, sizeof(*s)); + return; } - if(!tube_read_msg(worker->cmd, &reply, &len, 0)) - fatal_exit("failed to read stats over cmd channel"); - if(len != (uint32_t)sizeof(*s)) - fatal_exit("stats on cmd channel wrong length %d %d", - (int)len, (int)sizeof(*s)); memcpy(s, reply, (size_t)len); free(reply); } @@ -439,7 +455,7 @@ void server_stats_reply(struct worker* worker, int reset) verbose(VERB_ALGO, "write stats replymsg"); if(!tube_write_msg(worker->daemon->workers[0]->cmd, (uint8_t*)&s, sizeof(s), 0)) - fatal_exit("could not write stat values over cmd channel"); + log_err("could not write stat values over cmd channel"); } void server_stats_add(struct ub_stats_info* total, struct ub_stats_info* a) diff --git a/daemon/worker.c b/daemon/worker.c index 58ef01944..5da8f76a7 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -501,7 +501,9 @@ worker_handle_control_cmd(struct tube* ATTR_UNUSED(tube), uint8_t* msg, return; } if(len != sizeof(uint32_t)) { - fatal_exit("bad control msg length %d", (int)len); + verbose(VERB_ALGO, "bad control msg length %d", (int)len); + free(msg); + return; } cmd = sldns_read_uint32(msg); free(msg); diff --git a/doc/Changelog b/doc/Changelog index 3c7f1961c..7d59a265f 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -51,6 +51,10 @@ - Fix that fast_reload does not terminate the server on malloc failure for dnstap, or if gethostname fails. Thanks to Qifan Zhang, Palo Alto Networks, for the report. + - Fix after malloc failure for stats, then it drains the pipe + so the internal messaging stays correct. Also it does + not exit the server if stats pipe communication fails. + Thanks to Qifan Zhang, Palo Alto Networks, for the report. 16 June 2026: Wouter - Fix to disallow $INCLUDE for secondary zones. Start up diff --git a/util/tube.c b/util/tube.c index 96187e134..135ef130a 100644 --- a/util/tube.c +++ b/util/tube.c @@ -145,6 +145,20 @@ void tube_remove_bg_write(struct tube* tube) } } +/** Drain the pipe of bytes. */ +static void +fd_drain(int fd, uint32_t len) +{ + uint8_t discard[256]; + uint32_t remaining = len; + while(remaining > 0) { + ssize_t n = read(fd, discard, + remaining < sizeof(discard) ? remaining : sizeof(discard)); + if(n <= 0) break; + remaining -= (uint32_t)n; + } +} + int tube_handle_listen(struct comm_point* c, void* arg, int error, struct comm_reply* ATTR_UNUSED(reply_info)) @@ -184,6 +198,9 @@ tube_handle_listen(struct comm_point* c, void* arg, int error, tube->cmd_msg = (uint8_t*)calloc(1, tube->cmd_len); if(!tube->cmd_msg) { log_err("malloc failure"); + /* Drain the remaining bytes, since they belong to this + * message. The next message starts after it. */ + fd_drain(c->fd, tube->cmd_len); tube->cmd_read = 0; return 0; } @@ -374,6 +391,9 @@ int tube_read_msg(struct tube* tube, uint8_t** buf, uint32_t* len, *buf = (uint8_t*)malloc(*len); if(!*buf) { log_err("tube read out of memory"); + /* Drain the remaining bytes, since they belong to this + * message. The next message starts after it. */ + fd_drain(fd, *len); (void)fd_set_nonblock(fd); return 0; }