bytecode_watchdog: fix use of unaddressable data

If the watchdog thread is sleeping on a watchdog_item and
vm_execute_jit calls watchdog_disarm and returns before the watchdog wakes up,
then the watchdog thread will wake up and try to use the now uninitialized
item->abs_timeout.

Fix this by adding an in_use flag, and another condition variable.
watchdog_disarm now wakes the watchdog thread,
and waits till it releases the item.

Thanks to Michael Scheidell for providing feedback on this bug.
remotes/push_mirror/valgrind-mpool
Török Edvin 15 years ago
parent 7c25432931
commit bb5572cbe1
  1. 48
      libclamav/c++/bytecode2llvm.cpp

@ -1876,25 +1876,28 @@ INITIALIZE_PASS_END(RuntimeLimits, "rl" ,"Runtime Limits", false, false)
static pthread_mutex_t watchdog_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t watchdog_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t watchdog_cond = PTHREAD_COND_INITIALIZER; static pthread_cond_t watchdog_cond = PTHREAD_COND_INITIALIZER;
static pthread_cond_t watchdog_cond2 = PTHREAD_COND_INITIALIZER;
static int watchdog_running = 0; static int watchdog_running = 0;
struct watchdog_item { struct watchdog_item {
volatile uint8_t* timeout; volatile uint8_t* timeout;
struct timespec abstimeout; struct timespec abstimeout;
struct watchdog_item *next; struct watchdog_item *next;
int in_use;
}; };
static struct watchdog_item* watchdog_head = NULL; static struct watchdog_item* watchdog_head = NULL;
static struct watchdog_item* watchdog_tail = NULL; static struct watchdog_item* watchdog_tail = NULL;
extern "C" const char *cli_strerror(int errnum, char* buf, size_t len);
#define WATCHDOG_IDLE 10 #define WATCHDOG_IDLE 10
static void *bytecode_watchdog(void *arg) static void *bytecode_watchdog(void *arg)
{ {
struct timeval tv; struct timeval tv;
struct timespec out; struct timespec out;
int ret;
char err[128];
pthread_mutex_lock(&watchdog_mutex); pthread_mutex_lock(&watchdog_mutex);
watchdog_running = 1;
if (cli_debug_flag) if (cli_debug_flag)
cli_dbgmsg_internal("bytecode watchdog is running\n"); cli_dbgmsg_internal("bytecode watchdog is running\n");
do { do {
@ -1903,16 +1906,35 @@ static void *bytecode_watchdog(void *arg)
out.tv_sec = tv.tv_sec + WATCHDOG_IDLE; out.tv_sec = tv.tv_sec + WATCHDOG_IDLE;
out.tv_nsec = tv.tv_usec*1000; out.tv_nsec = tv.tv_usec*1000;
/* wait for some work, up to WATCHDOG_IDLE time */ /* wait for some work, up to WATCHDOG_IDLE time */
while (watchdog_head == NULL && while (watchdog_head == NULL) {
pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex, ret = pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
&out) != ETIMEDOUT) {} &out);
if (ret == ETIMEDOUT)
break;
if (ret) {
cli_warnmsg("bytecode_watchdog: cond_timedwait(1) failed: %s\n",
cli_strerror(ret, err, sizeof(err)));
break;
}
}
if (watchdog_head == NULL) if (watchdog_head == NULL)
break; break;
/* wait till timeout is reached on this item */ /* wait till timeout is reached on this item */
item = watchdog_head; item = watchdog_head;
while (item == watchdog_head && while (item == watchdog_head) {
pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex, item->in_use = 1;
&item->abstimeout) != ETIMEDOUT) {} ret = pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
&item->abstimeout);
if (ret == ETIMEDOUT)
break;
if (ret) {
cli_warnmsg("bytecode_watchdog: cond_timedwait(2) failed: %s\n",
cli_strerror(ret, err, sizeof(err)));
break;
}
}
item->in_use = 0;
pthread_cond_signal(&watchdog_cond2);
if (item != watchdog_head) if (item != watchdog_head)
continue;/* got removed meanwhile */ continue;/* got removed meanwhile */
/* timeout reached, signal it to bytecode */ /* timeout reached, signal it to bytecode */
@ -1929,7 +1951,6 @@ static void *bytecode_watchdog(void *arg)
return NULL; return NULL;
} }
extern "C" const char *cli_strerror(int errnum, char* buf, size_t len);
static void watchdog_disarm(struct watchdog_item *item) static void watchdog_disarm(struct watchdog_item *item)
{ {
struct watchdog_item *q, *p = NULL; struct watchdog_item *q, *p = NULL;
@ -1945,6 +1966,12 @@ static void watchdog_disarm(struct watchdog_item *item)
if (q == watchdog_tail) if (q == watchdog_tail)
watchdog_tail = p; watchdog_tail = p;
} }
/* don't remove the item from the list until the watchdog is sleeping on
* item, or it'll wake up on uninit data */
while (item->in_use) {
pthread_cond_signal(&watchdog_cond);
pthread_cond_wait(&watchdog_cond2, &watchdog_mutex);
}
pthread_mutex_unlock(&watchdog_mutex); pthread_mutex_unlock(&watchdog_mutex);
} }
@ -1956,6 +1983,7 @@ static int watchdog_arm(struct watchdog_item *item, int ms, volatile uint8_t *ti
*timeout = 0; *timeout = 0;
item->timeout = timeout; item->timeout = timeout;
item->next = NULL; item->next = NULL;
item->in_use = 0;
gettimeofday(&tv0, NULL); gettimeofday(&tv0, NULL);
tv0.tv_usec += ms * 1000; tv0.tv_usec += ms * 1000;
@ -1973,6 +2001,8 @@ static int watchdog_arm(struct watchdog_item *item, int ms, volatile uint8_t *ti
char buf[256]; char buf[256];
cli_errmsg("(watchdog) pthread_create failed: %s\n", cli_strerror(rc, buf, sizeof(buf))); cli_errmsg("(watchdog) pthread_create failed: %s\n", cli_strerror(rc, buf, sizeof(buf)));
} }
if (!rc)
watchdog_running = 1;
pthread_attr_destroy(&attr); pthread_attr_destroy(&attr);
} }
if (!rc) { if (!rc) {

Loading…
Cancel
Save