Browse code

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.

Török Edvin authored on 2011/06/30 20:24:34
Showing 1 changed files
... ...
@@ -1876,25 +1876,28 @@ INITIALIZE_PASS_END(RuntimeLimits, "rl" ,"Runtime Limits", false, false)
1876 1876
 
1877 1877
 static pthread_mutex_t watchdog_mutex = PTHREAD_MUTEX_INITIALIZER;
1878 1878
 static pthread_cond_t watchdog_cond = PTHREAD_COND_INITIALIZER;
1879
+static pthread_cond_t watchdog_cond2 = PTHREAD_COND_INITIALIZER;
1879 1880
 static int watchdog_running = 0;
1880 1881
 
1881 1882
 struct watchdog_item {
1882 1883
     volatile uint8_t* timeout;
1883 1884
     struct timespec abstimeout;
1884 1885
     struct watchdog_item *next;
1886
+    int in_use;
1885 1887
 };
1886 1888
 
1887 1889
 static struct watchdog_item* watchdog_head = NULL;
1888 1890
 static struct watchdog_item* watchdog_tail = NULL;
1889 1891
 
1892
+extern "C" const char *cli_strerror(int errnum, char* buf, size_t len);
1890 1893
 #define WATCHDOG_IDLE 10
1891 1894
 static void *bytecode_watchdog(void *arg)
1892 1895
 {
1893 1896
     struct timeval tv;
1894 1897
     struct timespec out;
1895
-
1898
+    int ret;
1899
+    char err[128];
1896 1900
     pthread_mutex_lock(&watchdog_mutex);
1897
-    watchdog_running = 1;
1898 1901
     if (cli_debug_flag)
1899 1902
 	cli_dbgmsg_internal("bytecode watchdog is running\n");
1900 1903
     do {
... ...
@@ -1903,16 +1906,35 @@ static void *bytecode_watchdog(void *arg)
1903 1903
 	out.tv_sec = tv.tv_sec + WATCHDOG_IDLE;
1904 1904
 	out.tv_nsec = tv.tv_usec*1000;
1905 1905
 	/* wait for some work, up to WATCHDOG_IDLE time */
1906
-	while (watchdog_head == NULL &&
1907
-	       pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
1908
-				      &out) != ETIMEDOUT) {}
1906
+	while (watchdog_head == NULL) {
1907
+	    ret = pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
1908
+					 &out);
1909
+	    if (ret == ETIMEDOUT)
1910
+		break;
1911
+	    if (ret) {
1912
+		cli_warnmsg("bytecode_watchdog: cond_timedwait(1) failed: %s\n",
1913
+			    cli_strerror(ret, err, sizeof(err)));
1914
+		break;
1915
+	    }
1916
+	}
1909 1917
 	if (watchdog_head == NULL)
1910 1918
 	    break;
1911 1919
 	/* wait till timeout is reached on this item */
1912 1920
 	item = watchdog_head;
1913
-	while (item == watchdog_head &&
1914
-	       pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
1915
-				      &item->abstimeout) != ETIMEDOUT) {}
1921
+	while (item == watchdog_head) {
1922
+	    item->in_use = 1;
1923
+	    ret = pthread_cond_timedwait(&watchdog_cond, &watchdog_mutex,
1924
+					 &item->abstimeout);
1925
+	    if (ret == ETIMEDOUT)
1926
+		break;
1927
+	    if (ret) {
1928
+		cli_warnmsg("bytecode_watchdog: cond_timedwait(2) failed: %s\n",
1929
+			    cli_strerror(ret, err, sizeof(err)));
1930
+		break;
1931
+	    }
1932
+	}
1933
+	item->in_use = 0;
1934
+	pthread_cond_signal(&watchdog_cond2);
1916 1935
 	if (item != watchdog_head)
1917 1936
 	    continue;/* got removed meanwhile */
1918 1937
 	/* timeout reached, signal it to bytecode */
... ...
@@ -1929,7 +1951,6 @@ static void *bytecode_watchdog(void *arg)
1929 1929
     return NULL;
1930 1930
 }
1931 1931
 
1932
-extern "C" const char *cli_strerror(int errnum, char* buf, size_t len);
1933 1932
 static void watchdog_disarm(struct watchdog_item *item)
1934 1933
 {
1935 1934
     struct watchdog_item *q, *p = NULL;
... ...
@@ -1945,6 +1966,12 @@ static void watchdog_disarm(struct watchdog_item *item)
1945 1945
 	if (q == watchdog_tail)
1946 1946
 	    watchdog_tail = p;
1947 1947
     }
1948
+    /* don't remove the item from the list until the watchdog is sleeping on
1949
+     * item, or it'll wake up on uninit data */
1950
+    while (item->in_use) {
1951
+	pthread_cond_signal(&watchdog_cond);
1952
+	pthread_cond_wait(&watchdog_cond2, &watchdog_mutex);
1953
+    }
1948 1954
     pthread_mutex_unlock(&watchdog_mutex);
1949 1955
 }
1950 1956
 
... ...
@@ -1956,6 +1983,7 @@ static int watchdog_arm(struct watchdog_item *item, int ms, volatile uint8_t *ti
1956 1956
     *timeout = 0;
1957 1957
     item->timeout = timeout;
1958 1958
     item->next = NULL;
1959
+    item->in_use = 0;
1959 1960
 
1960 1961
     gettimeofday(&tv0, NULL);
1961 1962
     tv0.tv_usec += ms * 1000;
... ...
@@ -1973,6 +2001,8 @@ static int watchdog_arm(struct watchdog_item *item, int ms, volatile uint8_t *ti
1973 1973
 	    char buf[256];
1974 1974
 	    cli_errmsg("(watchdog) pthread_create failed: %s\n", cli_strerror(rc, buf, sizeof(buf)));
1975 1975
 	}
1976
+	if (!rc)
1977
+	    watchdog_running = 1;
1976 1978
 	pthread_attr_destroy(&attr);
1977 1979
     }
1978 1980
     if (!rc) {