linux-kernel - [PATCH 3/4] kdbus: never return <0 from ioctls if we changed state

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1438958196-23036-4-git-send-email-daniel@zonque.org>
Date:	Fri,  7 Aug 2015 16:36:35 +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 3/4] kdbus: never return <0 from ioctls if we changed state

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

If an ioctl() returns <0, user-space should be safe to assume it had no
effect on the state of any object. This might not be always possible, but
in kdbus we adhered to this rule. But there's one exception, namely
KDBUS_CMD_NAME_ACQUIRE. This call used to fail with -EALREADY if we owned
a name and tried to acquire it again. However, regardless whether the
name was already owned, the name-flags are updated according to the newly
provided flags. Hence, we change the state of name-ownership, but might
still return an error.

This patch changes behavior and now returns 0 in those cases. User-space
still gets the same information (via return_flags), but will no longer be
told that the call failed. The tests reflect that and simply check for
KDBUS_NAME_ACQUIRED in 'return_flags'.

Reviewed-by: Daniel Mack <daniel@...que.org>
Signed-off-by: David Herrmann <dh.herrmann@...il.com>
---
 ipc/kdbus/names.c                          | 3 ---
 tools/testing/selftests/kdbus/test-chat.c  | 6 ++++--
 tools/testing/selftests/kdbus/test-names.c | 4 ----
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index a47ee54..bf44ca3 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -291,11 +291,9 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags,
 		/*
 		 * Already the primary owner of the name, flags were already
 		 * updated. Nothing to do.
-		 * For compatibility, we have to return -EALREADY.
 		 */
 
 		owner->flags |= KDBUS_NAME_PRIMARY;
-		ret = -EALREADY;
 
 	} else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) &&
 		   (flags & KDBUS_NAME_REPLACE_EXISTING)) {
@@ -344,7 +342,6 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags,
 		 */
 
 		list_del_init(&owner->name_entry);
-		ret = -EEXIST;
 	} else {
 		/*
 		 * Name is already claimed and queueing is not requested.
diff --git a/tools/testing/selftests/kdbus/test-chat.c b/tools/testing/selftests/kdbus/test-chat.c
index 71a92d8..41e5b53 100644
--- a/tools/testing/selftests/kdbus/test-chat.c
+++ b/tools/testing/selftests/kdbus/test-chat.c
@@ -41,8 +41,10 @@ int kdbus_test_chat(struct kdbus_test_env *env)
 	ret = kdbus_name_acquire(conn_a, "foo.bar.double", NULL);
 	ASSERT_RETURN(ret == 0);
 
-	ret = kdbus_name_acquire(conn_a, "foo.bar.double", NULL);
-	ASSERT_RETURN(ret == -EALREADY);
+	flags = 0;
+	ret = kdbus_name_acquire(conn_a, "foo.bar.double", &flags);
+	ASSERT_RETURN(ret == 0);
+	ASSERT_RETURN(!(flags & KDBUS_NAME_ACQUIRED));
 
 	ret = kdbus_name_release(conn_a, "foo.bar.double");
 	ASSERT_RETURN(ret == 0);
diff --git a/tools/testing/selftests/kdbus/test-names.c b/tools/testing/selftests/kdbus/test-names.c
index fd4ac5a..9217465 100644
--- a/tools/testing/selftests/kdbus/test-names.c
+++ b/tools/testing/selftests/kdbus/test-names.c
@@ -143,10 +143,6 @@ int kdbus_test_name_conflict(struct kdbus_test_env *env)
 	ret = conn_is_name_owner(env->conn, name);
 	ASSERT_RETURN(ret == 0);
 
-	/* check that we can't acquire it again from the 1st connection */
-	ret = kdbus_name_acquire(env->conn, name, NULL);
-	ASSERT_RETURN(ret == -EALREADY);
-
 	/* check that we also can't acquire it again from the 2nd connection */
 	ret = kdbus_name_acquire(conn, name, NULL);
 	ASSERT_RETURN(ret == -EEXIST);
-- 
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/