[gtk-vnc] Fix crashes in mouse event, tls shutdown & framebuffer update handlers

Daniel P. Berrange berrange at fedoraproject.org
Mon Nov 29 12:04:30 UTC 2010


commit a2b6401838bd6ead738d3dce4f35ac59da1c8f9c
Author: Daniel P. Berrange <berrange at redhat.com>
Date:   Mon Nov 29 11:58:21 2010 +0000

    Fix crashes in mouse event, tls shutdown & framebuffer update handlers

 ...vnc-0.4.2-framebuffer-update-bounds-check.patch |   77 ++++++++++++++++++++
 gtk-vnc-0.4.2-motion-event-crash.patch             |   35 +++++++++
 gtk-vnc-0.4.2-tls-shutdown-crash.patch             |   61 ++++++++++++++++
 gtk-vnc.spec                                       |   15 ++++-
 4 files changed, 187 insertions(+), 1 deletions(-)
---
diff --git a/gtk-vnc-0.4.2-framebuffer-update-bounds-check.patch b/gtk-vnc-0.4.2-framebuffer-update-bounds-check.patch
new file mode 100644
index 0000000..615c411
--- /dev/null
+++ b/gtk-vnc-0.4.2-framebuffer-update-bounds-check.patch
@@ -0,0 +1,77 @@
+commit f3fc5e57a78d4be9872f1394f697b9929873a737
+Author: Daniel P. Berrange <dan at berrange.com>
+Date:   Tue Nov 23 22:59:37 2010 +0000
+
+    Fix framebuffer update boundary check
+    
+    Framebuffer boundary checks need to take into account offset,
+    in addition to width/height
+    
+    * src/vncconnection.c: Fix boundary check
+
+diff --git a/src/vncconnection.c b/src/vncconnection.c
+index 433256a..165a5f1 100644
+--- a/src/vncconnection.c
++++ b/src/vncconnection.c
+@@ -2653,13 +2653,14 @@ static void vnc_connection_ext_key_event(VncConnection *conn)
+ 
+ 
+ static gboolean vnc_connection_validate_boundary(VncConnection *conn,
++						 guint16 x, guint16 y,
+ 						 guint16 width, guint16 height)
+ {
+ 	VncConnectionPrivate *priv = conn->priv;
+ 
+-	if (width > priv->width || height > priv->height) {
+-		VNC_DEBUG("Framebuffer update %dx%d outside boundary %dx%d",
+-			  width, height, priv->width, priv->height);
++	if ((x + width) > priv->width || (y + height) > priv->height) {
++		VNC_DEBUG("Framebuffer update %dx%d at %d,%d outside boundary %dx%d",
++			  width, height, x, y, priv->width, priv->height);
+ 		priv->has_error = TRUE;
+ 	}
+ 
+@@ -2681,37 +2682,37 @@ static gboolean vnc_connection_framebuffer_update(VncConnection *conn, gint32 et
+ 
+ 	switch (etype) {
+ 	case VNC_CONNECTION_ENCODING_RAW:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_raw_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
+ 		break;
+ 	case VNC_CONNECTION_ENCODING_COPY_RECT:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_copyrect_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
+ 		break;
+ 	case VNC_CONNECTION_ENCODING_RRE:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_rre_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
+ 		break;
+ 	case VNC_CONNECTION_ENCODING_HEXTILE:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_hextile_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
+ 		break;
+ 	case VNC_CONNECTION_ENCODING_ZRLE:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_zrle_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
+ 		break;
+ 	case VNC_CONNECTION_ENCODING_TIGHT:
+-		if (!vnc_connection_validate_boundary(conn, width, height))
++		if (!vnc_connection_validate_boundary(conn, x, y, width, height))
+ 			break;
+ 		vnc_connection_tight_update(conn, x, y, width, height);
+ 		vnc_connection_update(conn, x, y, width, height);
diff --git a/gtk-vnc-0.4.2-motion-event-crash.patch b/gtk-vnc-0.4.2-motion-event-crash.patch
new file mode 100644
index 0000000..46fcc35
--- /dev/null
+++ b/gtk-vnc-0.4.2-motion-event-crash.patch
@@ -0,0 +1,35 @@
+commit f23f0ebf1b659208d5036e10ab1f32249a2e1a4c
+Author: Daniel P. Berrange <dan at berrange.com>
+Date:   Mon Nov 22 21:18:29 2010 +0000
+
+    Avoid crash in motion event & vnc_display_get_pixbuf
+    
+    If a mouse event occurs before a connection completes setup
+    priv->fb will be NULL and a crash can occur. Likewise if
+    vnc_display_get_pixbuf() is called before priv->fb is set,
+    then a crash occurs. Add checks for NULL in both cases
+
+diff --git a/src/vncdisplay.c b/src/vncdisplay.c
+index 55fbcf4..0b7e800 100644
+--- a/src/vncdisplay.c
++++ b/src/vncdisplay.c
+@@ -557,6 +557,9 @@ static gboolean motion_event(GtkWidget *widget, GdkEventMotion *motion)
+ 	if (priv->conn == NULL || !vnc_connection_is_initialized(priv->conn))
+ 		return FALSE;
+ 
++	if (!priv->fb)
++		return FALSE;
++
+ 	fbw = vnc_framebuffer_get_width(VNC_FRAMEBUFFER(priv->fb));
+ 	fbh = vnc_framebuffer_get_height(VNC_FRAMEBUFFER(priv->fb));
+ 
+@@ -2050,6 +2053,9 @@ GdkPixbuf *vnc_display_get_pixbuf(VncDisplay *obj)
+ 	    !vnc_connection_is_initialized(priv->conn))
+ 		return NULL;
+ 
++	if (!priv->fb)
++		return NULL;
++
+ 	fb = VNC_FRAMEBUFFER(priv->fb);
+ 	surface = vnc_cairo_framebuffer_get_surface(priv->fb);
+ 	content = cairo_surface_get_content(surface) | CAIRO_CONTENT_COLOR;
diff --git a/gtk-vnc-0.4.2-tls-shutdown-crash.patch b/gtk-vnc-0.4.2-tls-shutdown-crash.patch
new file mode 100644
index 0000000..4a087e4
--- /dev/null
+++ b/gtk-vnc-0.4.2-tls-shutdown-crash.patch
@@ -0,0 +1,61 @@
+commit 5760a2a28d85cb79e39063cfd8ee8aee975caf24
+Author: Daniel P. Berrange <dan at berrange.com>
+Date:   Mon Nov 22 21:44:56 2010 +0000
+
+    Avoid crash in TLS cleanup code on shutdown
+    
+    The gnutls_bye() method may try to send data on the socket todo
+    graceful TLS shutdown. The priv->sock variable is possibly
+    already NULL at this point if the close was triggered via the
+    vnc_connection_shutdown() method. Change the latter so that
+    it only calls g_socket_close, not actually free'ing the
+    priv->sock object immediately. Also put sanity check code in
+    the TLS push/pull functions to catch future bugs in this area
+
+diff --git a/src/vncconnection.c b/src/vncconnection.c
+index 4a0c53c..433256a 100644
+--- a/src/vncconnection.c
++++ b/src/vncconnection.c
+@@ -939,6 +939,12 @@ static ssize_t vnc_connection_tls_push(gnutls_transport_ptr_t transport,
+ 	int ret;
+ 	GError *error = NULL;
+ 
++	if (!priv->sock) {
++		VNC_DEBUG("Unexpected TLS push on closed socket");
++		errno = EBADF;
++		return -1;
++	}
++
+ 	ret = g_socket_send(priv->sock, data, len, NULL, &error);
+ 	if (ret < 0) {
+ 		if (error) {
+@@ -962,6 +968,12 @@ static ssize_t vnc_connection_tls_pull(gnutls_transport_ptr_t transport,
+ 	int ret;
+ 	GError *error = NULL;
+ 
++	if (!priv->sock) {
++		VNC_DEBUG("Unexpected TLS pull on closed socket");
++		errno = EBADF;
++		return -1;
++	}
++
+ 	ret = g_socket_receive(priv->sock, data, len, NULL, &error);
+ 	if (ret < 0) {
+ 		if (error) {
+@@ -4461,11 +4473,12 @@ void vnc_connection_shutdown(VncConnection *conn)
+ 	VNC_DEBUG("Waking up couroutine to shutdown gracefully");
+ 	g_io_wakeup(&priv->wait);
+ 
+-	if (priv->sock) {
++	/* Closing the socket triggers an I/O error in the
++	 * event loop resulting...eventually.. in a call
++	 * to vnc_connection_close for full cleanup
++	 */
++	if (priv->sock)
+ 		g_socket_close(priv->sock, NULL);
+-		g_object_unref(priv->sock);
+-		priv->sock = NULL;
+-	}
+ }
+ 
+ gboolean vnc_connection_is_open(VncConnection *conn)
diff --git a/gtk-vnc.spec b/gtk-vnc.spec
index 5939a1e..2f8cd06 100644
--- a/gtk-vnc.spec
+++ b/gtk-vnc.spec
@@ -18,10 +18,13 @@
 Summary: A GTK2 widget for VNC clients
 Name: gtk-vnc
 Version: 0.4.2
-Release: 1%{?dist}%{?extra_release}
+Release: 2%{?dist}%{?extra_release}
 License: LGPLv2+
 Group: Development/Libraries
 Source: http://ftp.gnome.org/pub/GNOME/sources/%{name}/0.4/%{name}-%{version}.tar.bz2
+Patch1: %{name}-%{version}-motion-event-crash.patch
+Patch2: %{name}-%{version}-tls-shutdown-crash.patch
+Patch3: %{name}-%{version}-framebuffer-update-bounds-check.patch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 URL: http://live.gnome.org/gtk-vnc
 BuildRequires: gtk2-devel >= 2.14
@@ -145,6 +148,11 @@ Libraries, includes, etc. to compile with the gtk-vnc library
 
 %prep
 %setup -q -n gtk-vnc-%{version} -c
+cd %{name}-%{version}
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
+cd ..
 %if %{with_gtk3}
 cp -a gtk-vnc-%{version} gtk-vnc2-%{version}
 %endif
@@ -289,6 +297,11 @@ rm -fr %{buildroot}
 
 
 %changelog
+* Mon Nov 29 2010 Daniel P. Berrange <berrange at redhat.com> - 0.4.2-2
+- Fix crash in TLS shutdown code (rhbz #650601)
+- Fix crash in motion event handler (rhbz #650104)
+- Fix framebuffer update bounds checking (rhbz #655630)
+
 * Fri Nov  5 2010 Daniel P. Berrange <berrange at redhat.com> - 0.4.2-1
 - Update to 0.4.2 release.
 - Enable experimental GTK3 build


More information about the scm-commits mailing list