lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri,  7 Aug 2015 16:36:34 +0200
From:	Daniel Mack <daniel@...que.org>
To:	gregkh@...uxfoundation.org
Cc:	dh.herrmann@...il.com, tixxdz@...ndz.org, teg@...m.no,
	linux-kernel@...r.kernel.org
Subject: [PATCH 2/4] kdbus: inform caller about exact updates on NAME_ACQUIRE

From: David Herrmann <dh.herrmann@...il.com>

This adds two new return flags for KDBUS_CMD_NAME_ACQUIRE:

 * The KDBUS_NAME_PRIMARY flag is set for a name if, and only if, the
   connection is currently the primary owner of a name. It is thus the
   negation of KDBUS_NAME_IN_QUEUE, but is required to distinguish the
   case from the situation where the connection is neither queued nor the
   primary owner of a name.
   Additionally, this flag is included in name listings and events.

 * The KDBUS_NAME_ACQUIRED flag is exclusively used as return flag for
   KDBUS_CMD_NAME_ACQUIRE to let the caller know whether _this_ exact
   call actually queued the connection on the name. If the flag is not
   set, the connection was either already queued and only the flags were
   updated, or the connection is not queued at all.

This information was previously available to the caller via error-codes
from KDBUS_CMD_NAME_ACQUIRE. However, in some situations we actually
modify kernel state but return an error. This is considered bad style and
we really need to avoid this. Hence, these two new flags allow us to
avoid returning errors, but still inform the caller about the exact
conditions of the execution.

Reviewed-by: Daniel Mack <daniel@...que.org>
Signed-off-by: David Herrmann <dh.herrmann@...il.com>
---
 include/uapi/linux/kdbus.h |  4 ++++
 ipc/kdbus/names.c          | 38 ++++++++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/kdbus.h b/include/uapi/linux/kdbus.h
index ecffc6b..4fc44cb 100644
--- a/include/uapi/linux/kdbus.h
+++ b/include/uapi/linux/kdbus.h
@@ -854,6 +854,8 @@ enum kdbus_make_flags {
  * @KDBUS_NAME_QUEUE:			Name should be queued if busy
  * @KDBUS_NAME_IN_QUEUE:		Name is queued
  * @KDBUS_NAME_ACTIVATOR:		Name is owned by a activator connection
+ * @KDBUS_NAME_PRIMARY:			Primary owner of the name
+ * @KDBUS_NAME_ACQUIRED:		Name was acquired/queued _now_
  */
 enum kdbus_name_flags {
 	KDBUS_NAME_REPLACE_EXISTING	= 1ULL <<  0,
@@ -861,6 +863,8 @@ enum kdbus_name_flags {
 	KDBUS_NAME_QUEUE		= 1ULL <<  2,
 	KDBUS_NAME_IN_QUEUE		= 1ULL <<  3,
 	KDBUS_NAME_ACTIVATOR		= 1ULL <<  4,
+	KDBUS_NAME_PRIMARY		= 1ULL <<  5,
+	KDBUS_NAME_ACQUIRED		= 1ULL <<  6,
 };
 
 /**
diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index 7a6e61c..a47ee54 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -211,7 +211,8 @@ kdbus_name_lookup_unlocked(struct kdbus_name_registry *reg, const char *name)
 	return kdbus_name_entry_find(reg, kdbus_strhash(name), name);
 }
 
-static int kdbus_name_become_activator(struct kdbus_name_owner *owner)
+static int kdbus_name_become_activator(struct kdbus_name_owner *owner,
+				       u64 *return_flags)
 {
 	if (kdbus_name_owner_is_used(owner))
 		return -EALREADY;
@@ -221,23 +222,30 @@ static int kdbus_name_become_activator(struct kdbus_name_owner *owner)
 	owner->name->activator = owner;
 	owner->flags |= KDBUS_NAME_ACTIVATOR;
 
-	if (kdbus_name_entry_first(owner->name))
+	if (kdbus_name_entry_first(owner->name)) {
 		owner->flags |= KDBUS_NAME_IN_QUEUE;
-	else
+	} else {
+		owner->flags |= KDBUS_NAME_PRIMARY;
 		kdbus_notify_name_change(owner->conn->ep->bus,
 					 KDBUS_ITEM_NAME_ADD,
 					 0, owner->conn->id,
 					 0, owner->flags,
 					 owner->name->name);
+	}
+
+	if (return_flags)
+		*return_flags = owner->flags | KDBUS_NAME_ACQUIRED;
 
 	return 0;
 }
 
-static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
+static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags,
+			     u64 *return_flags)
 {
 	struct kdbus_name_owner *primary, *activator;
 	struct kdbus_name_entry *name;
 	struct kdbus_bus *bus;
+	u64 nflags = 0;
 	int ret = 0;
 
 	name = owner->name;
@@ -259,6 +267,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 		 */
 
 		list_add(&owner->name_entry, &name->queue);
+		owner->flags |= KDBUS_NAME_PRIMARY;
+		nflags |= KDBUS_NAME_ACQUIRED;
 
 		/* move messages to new owner on activation */
 		if (activator) {
@@ -268,6 +278,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 					activator->conn->id, owner->conn->id,
 					activator->flags, owner->flags,
 					name->name);
+			activator->flags &= ~KDBUS_NAME_PRIMARY;
 			activator->flags |= KDBUS_NAME_IN_QUEUE;
 		} else {
 			kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_ADD,
@@ -283,6 +294,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 		 * For compatibility, we have to return -EALREADY.
 		 */
 
+		owner->flags |= KDBUS_NAME_PRIMARY;
 		ret = -EALREADY;
 
 	} else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) &&
@@ -295,6 +307,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 
 		list_del_init(&owner->name_entry);
 		list_add(&owner->name_entry, &name->queue);
+		owner->flags |= KDBUS_NAME_PRIMARY;
+		nflags |= KDBUS_NAME_ACQUIRED;
 
 		kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_CHANGE,
 					 primary->conn->id, owner->conn->id,
@@ -303,6 +317,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 
 		/* requeue old primary, or drop if queueing not wanted */
 		if (primary->flags & KDBUS_NAME_QUEUE) {
+			primary->flags &= ~KDBUS_NAME_PRIMARY;
 			primary->flags |= KDBUS_NAME_IN_QUEUE;
 		} else {
 			list_del_init(&primary->name_entry);
@@ -317,8 +332,10 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 		 */
 
 		owner->flags |= KDBUS_NAME_IN_QUEUE;
-		if (!kdbus_name_owner_is_used(owner))
+		if (!kdbus_name_owner_is_used(owner)) {
 			list_add_tail(&owner->name_entry, &name->queue);
+			nflags |= KDBUS_NAME_ACQUIRED;
+		}
 	} else if (kdbus_name_owner_is_used(owner)) {
 		/*
 		 * Already queued on name, but re-queueing was not requested.
@@ -337,6 +354,9 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
 		ret = -EEXIST;
 	}
 
+	if (return_flags)
+		*return_flags = owner->flags | nflags;
+
 	return ret;
 }
 
@@ -392,15 +412,12 @@ int kdbus_name_acquire(struct kdbus_name_registry *reg,
 	}
 
 	if (flags & KDBUS_NAME_ACTIVATOR)
-		ret = kdbus_name_become_activator(owner);
+		ret = kdbus_name_become_activator(owner, return_flags);
 	else
-		ret = kdbus_name_update(owner, flags);
+		ret = kdbus_name_update(owner, flags, return_flags);
 	if (ret < 0)
 		goto exit;
 
-	if (return_flags)
-		*return_flags = owner->flags;
-
 exit:
 	if (owner && !kdbus_name_owner_is_used(owner))
 		kdbus_name_owner_free(owner);
@@ -431,6 +448,7 @@ static void kdbus_name_release_unlocked(struct kdbus_name_owner *owner)
 		if (next) {
 			/* hand to next in queue */
 			next->flags &= ~KDBUS_NAME_IN_QUEUE;
+			next->flags |= KDBUS_NAME_PRIMARY;
 			if (next == name->activator)
 				kdbus_conn_move_messages(next->conn,
 							 owner->conn,
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ