CLI: Address multiple issues.

* listen uses the variable `s` for the result from ast_poll() then
  overwrites it with the result of accept().  Create a separate variable
  poll_result to avoid confusion since ast_poll does not return a file
  descriptor.
* Resolve fd leak that would occur if setsockopt failed in listen.
* Reserve an extra byte while processing completion results from remote
  daemon.  This fixes a bug where completion processing used strstr() on
  a string that was not '\0' terminated.  This was no risk to the Asterisk
  daemon, the bug was only reachable the remote console process.
* Resolve leak in handle_showchan when the channel is not found.
* Multiple leaks and a deadlock in pbx_config CLI completion.
* Fix leaks in "manager show command".

Change-Id: I8f633ceb1714867ae30ef4e421858f77c14485a9

diff --git a/main/asterisk.c b/main/asterisk.c
index 015c9f35a1f4de6012709e1bee4916774023e611..f8e31d5a0361205783dc5cb68f8d9ee0e9d151ce 100644 (file)
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1695,17 +1695,21 @@ static void *listener(void *unused)
        int s;
        socklen_t len;
        int x;
+       int poll_result;
        struct pollfd fds[1];
+
        for (;;) {
-               if (ast_socket < 0)
+               if (ast_socket < 0) {
                        return NULL;
+               }
                fds[0].fd = ast_socket;
                fds[0].events = POLLIN;
-               s = ast_poll(fds, 1, -1);
+               poll_result = ast_poll(fds, 1, -1);
                pthread_testcancel();
-               if (s < 0) {
-                       if (errno != EINTR)
+               if (poll_result < 0) {
+                       if (errno != EINTR) {
                                ast_log(LOG_WARNING, "poll returned error: %s\n", strerror(errno));
+                       }
                        continue;
                }
                len = sizeof(sunaddr);
@@ -1719,6 +1723,7 @@ static void *listener(void *unused)
                        /* turn on socket credentials passing. */
                        if (setsockopt(s, SOL_SOCKET, SO_PASSCRED, &sckopt, sizeof(sckopt)) < 0) {
                                ast_log(LOG_WARNING, "Unable to turn on socket credentials passing\n");
+                               close(s);
                        } else
 #endif
                        {
@@ -3195,20 +3200,10 @@ static char *cli_complete(EditLine *editline, int ch)
 #define CMD_MATCHESARRAY "_COMMAND MATCHESARRAY \"%s\" \"%s\""
                char *mbuf;
                char *new_mbuf;
-               int mlen = 0, maxmbuf = 2048;
-
-               /* Start with a 2048 byte buffer */
-               mbuf = ast_malloc(maxmbuf);
-
-               /* This will run snprintf twice at most. */
-               while (mbuf && (mlen = snprintf(mbuf, maxmbuf, CMD_MATCHESARRAY, lf->buffer, ptr)) > maxmbuf) {
-                       /* Return value does not include space for NULL terminator. */
-                       maxmbuf = mlen + 1;
-                       ast_free(mbuf);
-                       mbuf = ast_malloc(maxmbuf);
-               }
+               int mlen = 0;
+               int maxmbuf = ast_asprintf(&mbuf, CMD_MATCHESARRAY, lf->buffer, ptr);
 
-               if (!mbuf) {
+               if (maxmbuf == -1) {
                        *((char *) lf->cursor) = savechr;
 
                        return (char *)(CC_ERROR);
@@ -3221,9 +3216,9 @@ static char *cli_complete(EditLine *editline, int ch)
 
                while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) {
                        if (mlen + 1024 > maxmbuf) {
-                               /* Expand buffer to the next 1024 byte increment. */
+                               /* Expand buffer to the next 1024 byte increment plus a NULL terminator. */
                                maxmbuf = mlen + 1024;
-                               new_mbuf = ast_realloc(mbuf, maxmbuf);
+                               new_mbuf = ast_realloc(mbuf, maxmbuf + 1);
                                if (!new_mbuf) {
                                        ast_free(mbuf);
                                        *((char *) lf->cursor) = savechr;
@@ -3236,6 +3231,7 @@ static char *cli_complete(EditLine *editline, int ch)
                        res = read(ast_consock, mbuf + mlen, 1024);
                        if (res > 0) {
                                mlen += res;
+                               mbuf[mlen] = '\0';
                        }
                }
                mbuf[mlen] = '\0';

diff --git a/main/cli.c b/main/cli.c
index 7039b72fcacaa16e7ff00614d3d475993f5c9648..fe20c340138f3d9653f00ed16a0c4068a8290341 100644 (file)
--- a/main/cli.c
+++ b/main/cli.c
@@ -1522,17 +1522,20 @@ static char *handle_showchan(struct ast_cli_entry *e, int cmd, struct ast_cli_ar
                return CLI_FAILURE;
        }
 
-       output = ast_str_create(8192);
-       if (!output) {
-               return CLI_FAILURE;
-       }
-
        chan = ast_channel_get_by_name(a->argv[3]);
        if (!chan) {
                ast_cli(a->fd, "%s is not a known channel\n", a->argv[3]);
+
                return CLI_SUCCESS;
        }
 
+       output = ast_str_create(8192);
+       if (!output) {
+               ast_channel_unref(chan);
+
+               return CLI_FAILURE;
+       }
+
        now = ast_tvnow();
        ast_channel_lock(chan);
 

diff --git a/main/manager.c b/main/manager.c
index 81052651e84e967fb36b7688dc80c9bdc02a4f3e..890a9754855a29c9b3714753b764d27bbae118f9 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -2344,11 +2344,12 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_
                AST_RWLIST_UNLOCK(&actions);
                return ret;
        }
-       authority = ast_str_alloca(MAX_AUTH_PERM_STRING);
        if (a->argc < 4) {
                return CLI_SHOWUSAGE;
        }
 
+       authority = ast_str_alloca(MAX_AUTH_PERM_STRING);
+
 #ifdef AST_XML_DOCS
        /* setup the titles */
        term_color(synopsis_title, "[Synopsis]\n", COLOR_MAGENTA, 0, 40);
@@ -2376,6 +2377,22 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_
                                        char *seealso = ast_xmldoc_printable(S_OR(cur->seealso, "Not available"), 1);
                                        char *privilege = ast_xmldoc_printable(S_OR(auth_str, "Not available"), 1);
                                        char *responses = ast_xmldoc_printable("None", 1);
+
+                                       if (!syntax || !synopsis || !description || !arguments
+                                                       || !seealso || !privilege || !responses) {
+                                               ast_free(syntax);
+                                               ast_free(synopsis);
+                                               ast_free(description);
+                                               ast_free(arguments);
+                                               ast_free(seealso);
+                                               ast_free(privilege);
+                                               ast_free(responses);
+                                               ast_cli(a->fd, "Allocation failure.\n");
+                                               AST_RWLIST_UNLOCK(&actions);
+
+                                               return CLI_FAILURE;
+                                       }
+
                                        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",
                                                syntax_title, syntax,
                                                synopsis_title, synopsis,
@@ -2403,6 +2420,14 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_
                                                ast_cli(a->fd, "Event: %s\n", cur->final_response->name);
                                                print_event_instance(a, cur->final_response);
                                        }
+
+                                       ast_free(syntax);
+                                       ast_free(synopsis);
+                                       ast_free(description);
+                                       ast_free(arguments);
+                                       ast_free(seealso);
+                                       ast_free(privilege);
+                                       ast_free(responses);
                                } else
 #endif
                                {

diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c
index c4a0e6c286b3854c55da0194e5b9dd286d24917b..1a1c73c64f29bd0b2967aa52530170077cbe7969 100644 (file)
--- a/pbx/pbx_config.c
+++ b/pbx/pbx_config.c
@@ -303,8 +303,10 @@ static char *complete_dialplan_remove_include(struct ast_cli_args *a)
                                while ( (nc = ast_walk_contexts(nc)) && nc != c && !already_served)
                                        already_served = lookup_ci(nc, i_name);
 
-                               if (!already_served && ++which > a->n)
+                               if (!already_served && ++which > a->n) {
                                        res = strdup(i_name);
+                                       break;
+                               }
                        }
                        ast_unlock_context(c);
                }
@@ -1523,17 +1525,21 @@ static char *complete_dialplan_remove_ignorepat(struct ast_cli_args *a)
                }
 
                for (c = NULL; !ret && (c = ast_walk_contexts(c)); ) {
-                       if (ast_rdlock_context(c))      /* fail, skip it */
+                       if (ast_rdlock_context(c)) {
+                               /* fail, skip it */
                                continue;
-                       if (!partial_match(ast_get_context_name(c), a->word, len))
+                       }
+                       if (!partial_match(ast_get_context_name(c), a->word, len)) {
+                               ast_unlock_context(c);
                                continue;
+                       }
                        if (lookup_c_ip(c, ignorepat) && ++which > a->n)
                                ret = strdup(ast_get_context_name(c));
                        ast_unlock_context(c);
                }
                ast_unlock_contexts();
                free(dupline);
-               return NULL;
+               return ret;
        }
 
        return NULL;