From: Matthias Gerstner <matthias.gerstner@suse.de>
Date: Fri, 14 Jul 2017 15:11:54 +0200
Subject: only allow dynamic UnregisterHandler for external handlers, thereby
 fixing DoS
Git-commit: bb80e9c7a798f035768260ebdadffb6eb0786178
References: bsc#1049489

Trying to unregister an internal handler ended up in a SEGFAULT, because
the tcmur_handler->opaque was NULL. Way to reproduce:

dbus-send --system --print-reply --dest=org.kernel.TCMUService1 /org/kernel/TCMUService1/HandlerManager1 org.kernel.TCMUService1.HandlerManager1.UnregisterHandler string:qcow

we use a newly introduced boolean in struct tcmur_handler for keeping
track of external handlers. As suggested by mikechristie adjusting the
public data structure is acceptable.

Acked-by: Lee Duncan <lduncan@suse.com>
---
 main.c        |   32 +++++++++++++++++++++++++++++---
 tcmu-runner.h |    9 +++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

--- a/main.c
+++ b/main.c
@@ -84,6 +84,12 @@ int tcmur_register_handler(struct tcmur_
 	return 0;
 }
 
+static int tcmur_register_dbus_handler(struct tcmur_handler *handler)
+{
+	assert(handler->_is_dbus_handler == true);
+	return tcmur_register_handler(handler);
+}
+
 bool tcmur_unregister_handler(struct tcmur_handler *handler)
 {
 	int i;
@@ -96,6 +102,16 @@ bool tcmur_unregister_handler(struct tcm
 	return false;
 }
 
+static bool tcmur_unregister_dbus_handler(struct tcmur_handler *handler)
+{
+	bool ret = false;
+	assert(handler->_is_dbus_handler == true);
+
+	ret = tcmur_unregister_handler(handler);
+
+	return ret;
+}
+
 static int is_handler(const struct dirent *dirent)
 {
 	if (strncmp(dirent->d_name, "handler_", 8))
@@ -521,7 +537,7 @@ on_handler_appeared(GDBusConnection *con
 
 	if (info->register_invocation) {
 		info->connection = connection;
-		tcmur_register_handler(handler);
+		tcmur_register_dbus_handler(handler);
 		dbus_export_handler(handler, G_CALLBACK(on_dbus_check_config));
 		g_dbus_method_invocation_return_value(info->register_invocation,
 			    g_variant_new("(bs)", TRUE, "succeeded"));
@@ -546,7 +562,7 @@ on_handler_vanished(GDBusConnection *con
 			    g_variant_new("(bs)", FALSE, reason));
 		g_free(reason);
 	}
-	tcmur_unregister_handler(handler);
+	tcmur_unregister_dbus_handler(handler);
 	dbus_unexport_handler(handler);
 }
 
@@ -572,6 +588,8 @@ on_register_handler(TCMUService1HandlerM
 	handler->handle_cmd   = dbus_handler_handle_cmd;
 
 	info = g_new0(struct dbus_info, 1);
+	handler->opaque = info;
+	handler->_is_dbus_handler = 1;
 	info->register_invocation = invocation;
 	info->watcher_id = g_bus_watch_name(G_BUS_TYPE_SYSTEM,
 					    bus_name,
@@ -600,8 +618,16 @@ on_unregister_handler(TCMUService1Handle
 				      "unknown subtype"));
 		return TRUE;
 	}
+	else if (handler->_is_dbus_handler != 1) {
+		g_dbus_method_invocation_return_value(invocation,
+			g_variant_new("(bs)", FALSE,
+				      "cannot unregister internal handler"));
+		return TRUE;
+	}
+
 	dbus_unexport_handler(handler);
-	tcmur_unregister_handler(handler);
+	tcmur_unregister_dbus_handler(handler);
+
 	g_bus_unwatch_name(info->watcher_id);
 	g_free(info);
 	g_free(handler);
--- a/tcmu-runner.h
+++ b/tcmu-runner.h
@@ -72,6 +72,15 @@ struct tcmur_handler {
 	ssize_t (*write)(struct tcmu_device *, struct iovec *, size_t, off_t);
 	ssize_t (*read)(struct tcmu_device *, struct iovec *, size_t, off_t);
 	int (*flush)(struct tcmu_device *);
+
+	/*
+	 * internal field, don't touch this
+	 *
+	 * indicates to tcmu-runner whether this is an internal handler loaded
+	 * via dlopen or an external handler registered via dbus. In the
+	 * latter case opaque will point to a struct dbus_info.
+	 */
+	bool _is_dbus_handler;
 };
 
 /*