[MPlayer-dev-eng] [PATCH] fix for segmentation fault in ao_nas

[Erik Auerswald]

Hi,

audio output using ao_nas sometimes crashes mplayer with a segmentation
fault. This is caused by NAS server messages requesting large amounts of
data. One error in libao2/ao_nas.c is subtracting an AuUint32 from an int
value. The other is not checking if enough data is in the buffer to
fulfill the request.

The latter problem is solved by limiting the sent data to the available
amount of data. The former can be solved by either using 64bit integer
types or checking the AuUint32 for values that overflow an int in the
current implementation.

I've attached 2 alternative patches, one for each of the overflow fixes,
because I'm not sure which way is "better".

Erik

P.S. I've already sent alternative 1 to this list, but I didn't use the
[PATCH] subject line, so it probably was overlooked.
-------------- next part --------------
diff -Naur main.orig/libao2/ao_nas.c main.new/libao2/ao_nas.c
--- main.orig/libao2/ao_nas.c	2005-02-28 00:06:32.000000000 +0100
+++ main.new/libao2/ao_nas.c	2005-05-17 11:05:39.000000000 +0200
@@ -254,6 +254,23 @@
 		event->num_bytes,
 		nas_data->expect_underrun);
 
+	/* fix for segmentation faults in this code
+	 *
+	 * obviously a AuUint32 is never negative, but used as an int it
+	 * could appear so and cause unexpected results on 32 bit systems
+	 * since the AuUint32 event->num_bytes is subtracted from the int value
+	 * nas_data->server_buffer_used any "negative" int value should be
+	 * ignored (i.e. set to 0) */
+	if(((int)event->num_bytes) < 0) {
+		mp_msg(MSGT_AO, MSGL_V, "ao_nas: event_handler(): NAS event with negative (int)event->num_bytes=%d\n", (int)event->num_bytes);
+		mp_msg(MSGT_AO, MSGL_V, "ao_nas: event_handler(): setting event->num_bytes to 0\n");
+		event->num_bytes = 0;
+	}
+	/* values of event->num_bytes > nas_data->server_buffer_used can result
+	 * in a segmentation fault as well */
+	if(event->num_bytes > nas_data->server_buffer_used)
+		event->num_bytes = nas_data->server_buffer_used;
+
 	nas_data->server_buffer_used -= event->num_bytes;
 	if (nas_data->server_buffer_used < 0)
 		nas_data->server_buffer_used = 0;
-------------- next part --------------
diff -Naur main.orig/libao2/ao_nas.c main.new/libao2/ao_nas.c
--- main.orig/libao2/ao_nas.c	2005-02-28 00:06:32.000000000 +0100
+++ main.new/libao2/ao_nas.c	2005-05-27 16:11:14.635965313 +0200
@@ -117,10 +117,10 @@
 
 	void *client_buffer;
 	void *server_buffer;
-	int client_buffer_size;
-	int client_buffer_used;
-	int server_buffer_size;
-	int server_buffer_used;
+	int64_t client_buffer_size;
+	int64_t client_buffer_used;
+	int64_t server_buffer_size;
+	int64_t server_buffer_used;
 	pthread_mutex_t buffer_mutex;
 
 	pthread_t event_thread;
@@ -254,6 +254,11 @@
 		event->num_bytes,
 		nas_data->expect_underrun);
 
+	/* values of event->num_bytes > nas_data->server_buffer_used can result
+	 * in a segmentation fault */
+	if(event->num_bytes > nas_data->server_buffer_used)
+		event->num_bytes = nas_data->server_buffer_used;
+
 	nas_data->server_buffer_used -= event->num_bytes;
 	if (nas_data->server_buffer_used < 0)
 		nas_data->server_buffer_used = 0;