[svn-commits] seanbright: branch 1.4 r323559 - /branches/1.4/main/manager.c

[SVN commits to the Digium repositories]

Author: seanbright
Date: Wed Jun 15 10:15:30 2011
New Revision: 323559

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=323559
Log:
Resolve a segfault/bus error when we try to map memory that falls on a page
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)

Modified:
    branches/1.4/main/manager.c

Modified: branches/1.4/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/main/manager.c?view=diff&rev=323559&r1=323558&r2=323559
==============================================================================
--- branches/1.4/main/manager.c (original)
+++ branches/1.4/main/manager.c Wed Jun 15 10:15:30 2011
@@ -2895,6 +2895,7 @@
 	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 @@
 	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 @@
 		if (ss.fd > -1) {
 			char *buf;
 			size_t l;
-
-			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))) {
+			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 (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 @@
 						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;