]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
Resolve a segfault/bus error when we try to map memory that falls on a page
authorSean Bright <sean@malleable.com>
Wed, 15 Jun 2011 15:15:30 +0000 (15:15 +0000)
committerSean Bright <sean@malleable.com>
Wed, 15 Jun 2011 15:15:30 +0000 (15:15 +0000)
boundary.

The fix for ASTERISK-15359 was incorrect in that it added 1 to the length of the
mmap'd region.  The problem with this is that reading/writing to that extra byte
outside of the bounds of the underlying fd causes a bus error.

The real issue is that we are working with both a FILE * and the raw fd
underneath it and not synchronizing between them.  The code that was removed in
ASTERISK-15359 was correct, but we weren't flushing the FILE * before mapping
the fd.

Looking at the manager code in 1.4 reveals that the FILE * in 'struct
mansession' is never used except to create a temporary file that we immediately
fdopen.  This means we just need to write a 0 byte to the fd and everything will
just work.  The other branches require a call to fflush() which, while not a
guaranteed fix, should reduce the likelihood of a crash.

This all makes sense in my head.

(closes issue ASTERISK-16460)
Reported by: Ravelomanantsoa Hoby (hoby)
Patches:
issue17747_1.4_svn_markII.patch uploaded by Sean Bright (license #5060)

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.4@323559 65c4cc65-6c06-0410-ace0-fbb531ad65f3

main/manager.c

index 6d4ab0a8ac1194a6fbc7ce8f5c3bbc9532c19029..4b343f4911a37a1f19d97e114c7746877585a898 100644 (file)
@@ -2895,6 +2895,7 @@ static char *generic_http_callback(int format, struct sockaddr_in *requestor, co
        char *c = workspace;
        char *retval = NULL;
        struct ast_variable *v;
+       char template[] = "/tmp/ast-http-XXXXXX";
 
        for (v = params; v; v = v->next) {
                if (!strcasecmp(v->name, "mansession_id")) {
@@ -2942,8 +2943,9 @@ static char *generic_http_callback(int format, struct sockaddr_in *requestor, co
        ss.session = s;
        ast_mutex_unlock(&s->__lock);
 
-       ss.f = tmpfile();
-       ss.fd = fileno(ss.f);
+       if ((ss.fd = mkstemp(template)) > -1) {
+               unlink(template);
+       }
 
        if (s) {
                struct message m = { 0 };
@@ -2989,13 +2991,21 @@ static char *generic_http_callback(int format, struct sockaddr_in *requestor, co
                if (ss.fd > -1) {
                        char *buf;
                        size_t l;
+                       ssize_t res;
+
+                       /* Make sure that our buffer is NULL terminated */
+                       while ((res = write(ss.fd, "", 1)) < 1) {
+                               if (res == -1) {
+                                       ast_log(LOG_ERROR, "Failed to terminate manager response output: %s\n", strerror(errno));
+                                       break;
+                               }
+                       }
 
-                       if ((l = lseek(ss.fd, 0, SEEK_END)) > 0) {
-                               if (MAP_FAILED == (buf = mmap(NULL, l + 1, PROT_READ | PROT_WRITE, MAP_SHARED, ss.fd, 0))) {
+                       if (res == 1 && (l = lseek(ss.fd, 0, SEEK_END)) > 0) {
+                               if (MAP_FAILED == (buf = mmap(NULL, l, PROT_READ | PROT_WRITE, MAP_SHARED, ss.fd, 0))) {
                                        ast_log(LOG_WARNING, "mmap failed.  Manager request output was not processed\n");
                                } else {
                                        char *tmpbuf;
-                                       buf[l] = '\0';
                                        if (format == FORMAT_XML)
                                                tmpbuf = xml_translate(buf, params);
                                        else if (format == FORMAT_HTML)
@@ -3016,11 +3026,10 @@ static char *generic_http_callback(int format, struct sockaddr_in *requestor, co
                                                free(tmpbuf);
                                        free(s->outputstr);
                                        s->outputstr = NULL;
-                                       munmap(buf, l + 1);
+                                       munmap(buf, l);
                                }
                        }
-                       fclose(ss.f);
-                       ss.f = NULL;
+                       close(ss.fd);
                        ss.fd = -1;
                } else if (s->outputstr) {
                        char *tmp;