From 2a5c3db244708d528f357adeb7e6ec947161fbad Mon Sep 17 00:00:00 2001 From: defanor Date: Sun, 22 Oct 2023 13:46:03 +0300 Subject: Fix a couple of bugs, refactor a little Found a couple of issues with -fanalyzer, though CWE-401 (analyzer-malloc-leak) appears to produce false positives still. --- src/rexmpp.c | 72 +++++++++++++++++++++++++++-------------------------- src/rexmpp_jingle.c | 40 ++++++++++++++++++----------- src/rexmpp_tls.c | 4 +-- src/rexmpp_xml.c | 8 +++--- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/src/rexmpp.c b/src/rexmpp.c index e9d20c0..7e4a212 100644 --- a/src/rexmpp.c +++ b/src/rexmpp.c @@ -912,62 +912,64 @@ rexmpp_err_t rexmpp_send_continue (rexmpp_t *s) } ssize_t ret; rexmpp_tls_err_t err; - int tls_was_active; while (1) { - tls_was_active = (s->tls_state == REXMPP_TLS_ACTIVE); - if (tls_was_active) { + if (s->tls_state == REXMPP_TLS_ACTIVE) { err = rexmpp_tls_send (s, s->tls, s->send_buffer, s->send_buffer_len, &ret); - } else { - ret = send (s->server_socket, - s->send_buffer + s->send_buffer_sent, - s->send_buffer_len - s->send_buffer_sent, - 0); - } - if (ret > 0) { - clock_gettime(CLOCK_MONOTONIC, &(s->last_network_activity)); - s->send_buffer_sent += ret; - if (s->send_buffer_sent == s->send_buffer_len) { - free(s->send_buffer); - s->send_buffer = NULL; - if (s->send_queue != NULL) { - rexmpp_xml_t *node = s->send_queue; - char *buf = rexmpp_xml_serialize(node, 0); - ret = rexmpp_send_start(s, buf, strlen(buf)); - free(buf); - if (ret != REXMPP_SUCCESS) { - return ret; - } - s->send_queue = s->send_queue->next; - rexmpp_xml_free(node); - } else { - return REXMPP_SUCCESS; - } - } - } else { - if (tls_was_active) { + if (ret <= 0) { if (err != REXMPP_TLS_E_AGAIN) { s->tls_state = REXMPP_TLS_ERROR; /* Assume a TCP error for now as well. */ rexmpp_cleanup(s); s->tcp_state = REXMPP_TCP_ERROR; rexmpp_schedule_reconnect(s); - return REXMPP_E_AGAIN; } - } else { + /* Returning E_AGAIN for now, since the error is potentially + recoverable after the scheduled reconnect. */ + return REXMPP_E_AGAIN; + } + } else { + ret = send (s->server_socket, + s->send_buffer + s->send_buffer_sent, + s->send_buffer_len - s->send_buffer_sent, + 0); + if (ret <= 0) { if (errno != EAGAIN) { rexmpp_log(s, LOG_ERR, "TCP send error: %s", strerror(errno)); rexmpp_cleanup(s); s->tcp_state = REXMPP_TCP_ERROR; rexmpp_schedule_reconnect(s); - return REXMPP_E_AGAIN; } + /* E_AGAIN, similarly to the TLS case. */ + return REXMPP_E_AGAIN; } - return REXMPP_E_AGAIN; } + + assert(ret > 0); + + clock_gettime(CLOCK_MONOTONIC, &(s->last_network_activity)); + s->send_buffer_sent += ret; + if (s->send_buffer_sent == s->send_buffer_len) { + free(s->send_buffer); + s->send_buffer = NULL; + if (s->send_queue != NULL) { + rexmpp_xml_t *node = s->send_queue; + char *buf = rexmpp_xml_serialize(node, 0); + ret = rexmpp_send_start(s, buf, strlen(buf)); + free(buf); + if (ret != REXMPP_SUCCESS) { + return ret; + } + s->send_queue = s->send_queue->next; + rexmpp_xml_free(node); + } else { + return REXMPP_SUCCESS; + } + } + } } diff --git a/src/rexmpp_jingle.c b/src/rexmpp_jingle.c index 9e16d59..3c73bc0 100644 --- a/src/rexmpp_jingle.c +++ b/src/rexmpp_jingle.c @@ -437,17 +437,24 @@ rexmpp_jingle_session_create (rexmpp_t *s, { rexmpp_jingle_session_t *sess = malloc(sizeof(rexmpp_jingle_session_t)); if (sess != NULL) { - sess->s = s; sess->jid = jid; sess->sid = sid; - sess->type = type; - sess->initiator = initiator; sess->initiate = NULL; sess->accept = NULL; + sess->s = s; + sess->initiator = initiator; + sess->type = type; sess->ibb_fh = NULL; sess->ibb_sid = NULL; sess->ibb_seq = 0; #ifdef ENABLE_CALLS + sess->stun_host = NULL; + sess->stun_port = 0; + sess->turn_host = NULL; + sess->turn_port = 0; + sess->turn_username = NULL; + sess->turn_password = NULL; + int i; for (i = 0; i < 2; i++) { sess->component[i].component_id = i + 1; @@ -458,15 +465,11 @@ rexmpp_jingle_session_create (rexmpp_t *s, sess->component[i].srtp_out = NULL; sess->component[i].srtp_in = NULL; } - sess->ice_agent = NULL; + sess->rtcp_mux = s->jingle_prefer_rtcp_mux; + sess->ice_agent = NULL; - sess->stun_host = NULL; - sess->stun_port = 0; - sess->turn_host = NULL; - sess->turn_port = 0; - sess->turn_username = NULL; - sess->turn_password = NULL; + sess->pa_stream = NULL; sess->ring_buffers.capture.read_pos = 0; sess->ring_buffers.capture.write_pos = 0; @@ -474,6 +477,7 @@ rexmpp_jingle_session_create (rexmpp_t *s, sess->ring_buffers.playback.write_pos = 0; sess->codec = REXMPP_CODEC_UNDEFINED; sess->payload_type = 0; + #ifdef HAVE_OPUS sess->opus_enc = NULL; sess->opus_dec = NULL; @@ -481,12 +485,13 @@ rexmpp_jingle_session_create (rexmpp_t *s, #endif /* ENABLE_CALLS */ if (! rexmpp_jingle_session_add(s, sess)) { /* The session is destroyed by rexmpp_jingle_session_add */ - sess = NULL; + return NULL; } + return sess; } else { rexmpp_log(s, LOG_ERR, "Failed to allocate memory for a Jingle session"); + return NULL; } - return sess; } rexmpp_jingle_session_t * @@ -1769,9 +1774,14 @@ rexmpp_jingle_call (rexmpp_t *s, rexmpp_jingle_session_t *sess = rexmpp_jingle_session_create(s, strdup(jid), rexmpp_random_id(), REXMPP_JINGLE_SESSION_MEDIA, 1); - rexmpp_jingle_ice_agent_init(sess); - rexmpp_jingle_discover_turn(s, sess); - return REXMPP_SUCCESS; + if (sess != NULL) { + rexmpp_jingle_ice_agent_init(sess); + rexmpp_jingle_discover_turn(s, sess); + return REXMPP_SUCCESS; + } else { + rexmpp_log(s, LOG_ERR, "Failed to create a Jingle session for a call"); + return REXMPP_E_OTHER; + } } rexmpp_err_t diff --git a/src/rexmpp_tls.c b/src/rexmpp_tls.c index e483a2c..2a7c903 100644 --- a/src/rexmpp_tls.c +++ b/src/rexmpp_tls.c @@ -582,8 +582,8 @@ rexmpp_tls_send (rexmpp_t *s, size_t data_size, ssize_t *written) { -#if defined(USE_GNUTLS) *written = -1; +#if defined(USE_GNUTLS) ssize_t ret = gnutls_record_send(tls_ctx->gnutls_session, data, data_size); @@ -597,7 +597,6 @@ rexmpp_tls_send (rexmpp_t *s, return REXMPP_TLS_E_OTHER; } #elif defined(USE_OPENSSL) - *written = -1; int ret = SSL_write_ex(tls_ctx->openssl_conn, data, data_size, (size_t*)written); if (ret > 0) { @@ -608,7 +607,6 @@ rexmpp_tls_send (rexmpp_t *s, #else (void)data; (void)data_size; - (void)written; (void)tls_ctx; rexmpp_log(s, LOG_ERR, "rexmpp is compiled without TLS support"); return REXMPP_TLS_E_OTHER; diff --git a/src/rexmpp_xml.c b/src/rexmpp_xml.c index 442a49e..9c3dd23 100644 --- a/src/rexmpp_xml.c +++ b/src/rexmpp_xml.c @@ -250,7 +250,7 @@ int rexmpp_xml_remove_attr (rexmpp_xml_t *node, } /* Adds a character, grows the string as needed. */ -inline char *rexmpp_str_putc (char *str, size_t *len, char c) { +static inline char *rexmpp_str_putc (char *str, size_t *len, char c) { char *ret = str; if ((*len) % 1024 == 0) { ret = realloc(str, (*len) + 1024); @@ -372,9 +372,9 @@ char *rexmpp_xml_print_raw (char *str, size_t *len, const char *text) { return ret; } -inline char *rexmpp_xml_print_indent (char *str, - size_t *len, - int indent) { +static inline char *rexmpp_xml_print_indent (char *str, + size_t *len, + int indent) { if (indent <= 0) { return str; } -- cgit v1.2.3