<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7661">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">CLI: Address multiple issues.<br><br>* listen uses the variable `s` for the result from ast_poll() then<br> overwrites it with the result of accept(). Create a separate variable<br> poll_result to avoid confusion since ast_poll does not return a file<br> descriptor.<br>* Resolve fd leak that would occur if setsockopt failed in listen.<br>* Reserve an extra byte while processing completion results from remote<br> daemon. This fixes a bug where completion processing used strstr() on<br> a string that was not '\0' terminated. This was no risk to the Asterisk<br> daemon, the bug was only reachable the remote console process.<br>* Resolve leak in handle_showchan when the channel is not found.<br>* Multiple leaks and a deadlock in pbx_config CLI completion.<br>* Fix leaks in "manager show command".<br><br>Change-Id: I8f633ceb1714867ae30ef4e421858f77c14485a9<br>---<br>M main/asterisk.c<br>M main/cli.c<br>M main/manager.c<br>M pbx/pbx_config.c<br>4 files changed, 59 insertions(+), 29 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/asterisk.c b/main/asterisk.c<br>index 75d611f..73c03a2 100644<br>--- a/main/asterisk.c<br>+++ b/main/asterisk.c<br>@@ -1573,17 +1573,21 @@<br> int s;<br> socklen_t len;<br> int x;<br>+ int poll_result;<br> struct pollfd fds[1];<br>+<br> for (;;) {<br>- if (ast_socket < 0)<br>+ if (ast_socket < 0) {<br> return NULL;<br>+ }<br> fds[0].fd = ast_socket;<br> fds[0].events = POLLIN;<br>- s = ast_poll(fds, 1, -1);<br>+ poll_result = ast_poll(fds, 1, -1);<br> pthread_testcancel();<br>- if (s < 0) {<br>- if (errno != EINTR)<br>+ if (poll_result < 0) {<br>+ if (errno != EINTR) {<br> ast_log(LOG_WARNING, "poll returned error: %s\n", strerror(errno));<br>+ }<br> continue;<br> }<br> len = sizeof(sunaddr);<br>@@ -1597,6 +1601,7 @@<br> /* turn on socket credentials passing. */<br> if (setsockopt(s, SOL_SOCKET, SO_PASSCRED, &sckopt, sizeof(sckopt)) < 0) {<br> ast_log(LOG_WARNING, "Unable to turn on socket credentials passing\n");<br>+ close(s);<br> } else<br> #endif<br> {<br>@@ -3051,20 +3056,10 @@<br> #define CMD_MATCHESARRAY "_COMMAND MATCHESARRAY \"%s\" \"%s\""<br> char *mbuf;<br> char *new_mbuf;<br>- int mlen = 0, maxmbuf = 2048;<br>+ int mlen = 0;<br>+ int maxmbuf = ast_asprintf(&mbuf, CMD_MATCHESARRAY, lf->buffer, ptr);<br> <br>- /* Start with a 2048 byte buffer */<br>- mbuf = ast_malloc(maxmbuf);<br>-<br>- /* This will run snprintf twice at most. */<br>- while (mbuf && (mlen = snprintf(mbuf, maxmbuf, CMD_MATCHESARRAY, lf->buffer, ptr)) > maxmbuf) {<br>- /* Return value does not include space for NULL terminator. */<br>- maxmbuf = mlen + 1;<br>- ast_free(mbuf);<br>- mbuf = ast_malloc(maxmbuf);<br>- }<br>-<br>- if (!mbuf) {<br>+ if (maxmbuf == -1) {<br> *((char *) lf->cursor) = savechr;<br> <br> return (char *)(CC_ERROR);<br>@@ -3077,9 +3072,9 @@<br> <br> while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) {<br> if (mlen + 1024 > maxmbuf) {<br>- /* Expand buffer to the next 1024 byte increment. */<br>+ /* Expand buffer to the next 1024 byte increment plus a NULL terminator. */<br> maxmbuf = mlen + 1024;<br>- new_mbuf = ast_realloc(mbuf, maxmbuf);<br>+ new_mbuf = ast_realloc(mbuf, maxmbuf + 1);<br> if (!new_mbuf) {<br> ast_free(mbuf);<br> *((char *) lf->cursor) = savechr;<br>@@ -3092,6 +3087,7 @@<br> res = read(ast_consock, mbuf + mlen, 1024);<br> if (res > 0) {<br> mlen += res;<br>+ mbuf[mlen] = '\0';<br> }<br> }<br> mbuf[mlen] = '\0';<br>diff --git a/main/cli.c b/main/cli.c<br>index 207d1f8..1299c2a 100644<br>--- a/main/cli.c<br>+++ b/main/cli.c<br>@@ -1496,17 +1496,20 @@<br> return CLI_FAILURE;<br> }<br> <br>- output = ast_str_create(8192);<br>- if (!output) {<br>- return CLI_FAILURE;<br>- }<br>-<br> chan = ast_channel_get_by_name(a->argv[3]);<br> if (!chan) {<br> ast_cli(a->fd, "%s is not a known channel\n", a->argv[3]);<br>+<br> return CLI_SUCCESS;<br> }<br> <br>+ output = ast_str_create(8192);<br>+ if (!output) {<br>+ ast_channel_unref(chan);<br>+<br>+ return CLI_FAILURE;<br>+ }<br>+<br> now = ast_tvnow();<br> ast_channel_lock(chan);<br> <br>diff --git a/main/manager.c b/main/manager.c<br>index 079dab7..71d7432 100644<br>--- a/main/manager.c<br>+++ b/main/manager.c<br>@@ -2343,10 +2343,11 @@<br> AST_RWLIST_UNLOCK(&actions);<br> return ret;<br> }<br>- authority = ast_str_alloca(MAX_AUTH_PERM_STRING);<br> if (a->argc < 4) {<br> return CLI_SHOWUSAGE;<br> }<br>+<br>+ authority = ast_str_alloca(MAX_AUTH_PERM_STRING);<br> <br> #ifdef AST_XML_DOCS<br> /* setup the titles */<br>@@ -2375,6 +2376,22 @@<br> char *seealso = ast_xmldoc_printable(S_OR(cur->seealso, "Not available"), 1);<br> char *privilege = ast_xmldoc_printable(S_OR(auth_str, "Not available"), 1);<br> char *responses = ast_xmldoc_printable("None", 1);<br>+<br>+ if (!syntax || !synopsis || !description || !arguments<br>+ || !seealso || !privilege || !responses) {<br>+ ast_free(syntax);<br>+ ast_free(synopsis);<br>+ ast_free(description);<br>+ ast_free(arguments);<br>+ ast_free(seealso);<br>+ ast_free(privilege);<br>+ ast_free(responses);<br>+ ast_cli(a->fd, "Allocation failure.\n");<br>+ AST_RWLIST_UNLOCK(&actions);<br>+<br>+ return CLI_FAILURE;<br>+ }<br>+<br> ast_cli(a->fd, "%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s",<br> syntax_title, syntax,<br> synopsis_title, synopsis,<br>@@ -2402,6 +2419,14 @@<br> ast_cli(a->fd, "Event: %s\n", cur->final_response->name);<br> print_event_instance(a, cur->final_response);<br> }<br>+<br>+ ast_free(syntax);<br>+ ast_free(synopsis);<br>+ ast_free(description);<br>+ ast_free(arguments);<br>+ ast_free(seealso);<br>+ ast_free(privilege);<br>+ ast_free(responses);<br> } else<br> #endif<br> {<br>diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c<br>index b96914d..72e9f57 100644<br>--- a/pbx/pbx_config.c<br>+++ b/pbx/pbx_config.c<br>@@ -322,8 +322,10 @@<br> while ( (nc = ast_walk_contexts(nc)) && nc != c && !already_served)<br> already_served = lookup_ci(nc, i_name);<br> <br>- if (!already_served && ++which > a->n)<br>+ if (!already_served && ++which > a->n) {<br> res = ast_strdup(i_name);<br>+ break;<br>+ }<br> }<br> ast_unlock_context(c);<br> }<br>@@ -1549,17 +1551,21 @@<br> }<br> <br> for (c = NULL; !ret && (c = ast_walk_contexts(c)); ) {<br>- if (ast_rdlock_context(c)) /* fail, skip it */<br>+ if (ast_rdlock_context(c)) {<br>+ /* fail, skip it */<br> continue;<br>- if (!partial_match(ast_get_context_name(c), a->word, len))<br>+ }<br>+ if (!partial_match(ast_get_context_name(c), a->word, len)) {<br>+ ast_unlock_context(c);<br> continue;<br>+ }<br> if (lookup_c_ip(c, ignorepat) && ++which > a->n)<br> ret = ast_strdup(ast_get_context_name(c));<br> ast_unlock_context(c);<br> }<br> ast_unlock_contexts();<br> ast_free(dupline);<br>- return NULL;<br>+ return ret;<br> }<br> <br> return NULL;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7661">change 7661</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7661"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I8f633ceb1714867ae30ef4e421858f77c14485a9 </div>
<div style="display:none"> Gerrit-Change-Number: 7661 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>