From f94a1d47d67090939497dfd4b004e637cd1a9cc7 Mon Sep 17 00:00:00 2001
From: "Kevin P. Fleming" <kpfleming@digium.com>
Date: Wed, 14 Sep 2005 23:56:50 +0000
Subject: [PATCH] clean up ast_verbose(), don't hold the lock any longer than
needed
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@6606 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---
logger.c | 74 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 32 deletions(-)
diff --git a/logger.c b/logger.c
index 1727a0ffad..1d5bacee4d 100755
--- a/logger.c
+++ b/logger.c
@@ -812,46 +812,60 @@ void ast_log(int level, const char *file, int line, const char *function, const
}
}
-extern void ast_verbose(const char *fmt, ...)
+void ast_verbose(const char *fmt, ...)
{
static char stuff[4096];
- static int pos = 0, opos;
- static int replacelast = 0, complete;
+ static int len = 0;
+ static int replacelast = 0;
+
+ int complete;
+ int olen;
struct msglist *m;
struct verb *v;
- time_t t;
- struct tm tm;
- char date[40];
- char *datefmt;
va_list ap;
va_start(ap, fmt);
- ast_mutex_lock(&msglist_lock);
- time(&t);
- localtime_r(&t, &tm);
- strftime(date, sizeof(date), dateformat, &tm);
if (option_timestamp) {
+ time_t t;
+ struct tm tm;
+ char date[40];
+ char *datefmt;
+
+ time(&t);
+ localtime_r(&t, &tm);
+ strftime(date, sizeof(date), dateformat, &tm);
datefmt = alloca(strlen(date) + 3 + strlen(fmt) + 1);
if (datefmt) {
sprintf(datefmt, "[%s] %s", date, fmt);
fmt = datefmt;
}
}
- vsnprintf(stuff + pos, sizeof(stuff) - pos, fmt, ap);
- opos = pos;
- pos = strlen(stuff);
+ /* this lock is also protecting against multiple threads
+ being in this function at the same time, so it must be
+ held before any of the static variables are accessed
+ */
+ ast_mutex_lock(&msglist_lock);
+
+ /* there is a potential security problem here: if formatting
+ the current date using 'dateformat' results in a string
+ containing '%', then the vsnprintf() call below will
+ probably try to access random memory
+ */
+ vsnprintf(stuff + len, sizeof(stuff) - len, fmt, ap);
+ va_end(ap);
+
+ olen = len;
+ len = strlen(stuff);
+
+ complete = (stuff[len - 1] == '\n') ? 1 : 0;
- if (stuff[strlen(stuff)-1] == '\n')
- complete = 1;
- else
- complete=0;
if (complete) {
if (msgcnt < MAX_MSG_QUEUE) {
/* Allocate new structure */
- m = malloc(sizeof(struct msglist));
- msgcnt++;
+ if ((m = malloc(sizeof(*m))))
+ msgcnt++;
} else {
/* Recycle the oldest entry */
m = list;
@@ -874,22 +888,18 @@ extern void ast_verbose(const char *fmt, ...)
}
}
}
- if (verboser) {
- v = verboser;
- while(v) {
- v->verboser(stuff, opos, replacelast, complete);
- v = v->next;
- }
- } /* else
- fprintf(stdout, stuff + opos); */
+
+ for (v = verboser; v; v = v->next)
+ v->verboser(stuff, olen, replacelast, complete);
+
ast_log(LOG_VERBOSE, "%s", stuff);
- if (strlen(stuff)) {
- if (stuff[strlen(stuff)-1] != '\n')
+
+ if (len) {
+ if (complete)
replacelast = 1;
else
- replacelast = pos = 0;
+ replacelast = len = 0;
}
- va_end(ap);
ast_mutex_unlock(&msglist_lock);
}
--
GitLab