* [PATCH v2 1/2] keys: Enforce keep guard when moving keys
2026-05-31 20:51 [PATCH v2 0/2] keys: Enforce keep guard when moving keys Liu Mingyu
@ 2026-05-31 20:51 ` Liu Mingyu
2026-06-08 3:45 ` Jarkko Sakkinen
2026-05-31 20:51 ` [PATCH v2 2/2] keys: Add KUnit coverage for KEYCTL_MOVE keep guard Liu Mingyu
1 sibling, 1 reply; 4+ messages in thread
From: Liu Mingyu @ 2026-05-31 20:51 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: keyrings, linux-kernel, Eric Biggers, Kees Cook, Mimi Zohar,
Randy Dunlap
KEYCTL_MOVE removes the source link as part of moving a key
between keyrings. key_unlink() rejects removal of a
KEY_FLAG_KEEP-protected key from a KEY_FLAG_KEEP-protected keyring,
but key_move() did not enforce the same rule.
Reject such moves with -EPERM when both the source keyring and key
are protected. Leave same-keyring moves as a no-op so callers can
continue to use KEYCTL_MOVE idempotently. Document the errno so
callers can distinguish the protected removal case.
Fixes: ed0ac5c7ec37 ("keys: Add a keyctl to move a key between keyrings")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Liu <edwardliu0214@outlook.com>
---
Documentation/security/keys/core.rst | 1 +
security/keys/keyctl.c | 2 ++
security/keys/keyring.c | 7 ++++++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index 326b8a973828..6096ce6c63da 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -600,6 +600,7 @@ The keyctl syscall functions are:
A process must have link permission on the key for this function to be
successful and write permission on both keyrings. Any errors that can
occur from KEYCTL_LINK also apply on the destination keyring here.
+ If the key and source keyring are protected, EPERM will be returned.
* Unlink a key or keyring from another keyring::
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index ef855d69c97a..b37bf1505ec5 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -590,6 +590,8 @@ long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid)
* the caller Write permission. There must also be a link in the from keyring
* to the key. If both keyrings are the same, nothing is done.
*
+ * If the key and source keyring are protected, -EPERM will be returned.
+ *
* If successful, 0 will be returned.
*/
long keyctl_keyring_move(key_serial_t id, key_serial_t from_ringid,
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 5a9887d6b7be..60c184bd9a8d 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1582,7 +1582,8 @@ EXPORT_SYMBOL(key_unlink);
*
* Returns 0 if successful, -ENOTDIR if either keyring isn't a keyring,
* -EKEYREVOKED if either keyring has been revoked, -ENFILE if the second
- * keyring is full, -EDQUOT if there is insufficient key data quota remaining
+ * keyring is full, -EPERM if this would remove a protected key from a
+ * protected keyring, -EDQUOT if there is insufficient key data quota remaining
* to add another link or -ENOMEM if there's insufficient memory. If
* KEYCTL_MOVE_EXCL is set, then -EEXIST will be returned if there's already a
* matching key in @to_keyring.
@@ -1608,6 +1609,10 @@ int key_move(struct key *key,
key_check(from_keyring);
key_check(to_keyring);
+ if (test_bit(KEY_FLAG_KEEP, &from_keyring->flags) &&
+ test_bit(KEY_FLAG_KEEP, &key->flags))
+ return -EPERM;
+
ret = __key_move_lock(from_keyring, to_keyring, &key->index_key);
if (ret < 0)
goto out;
--
2.51.2.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2 2/2] keys: Add KUnit coverage for KEYCTL_MOVE keep guard
2026-05-31 20:51 [PATCH v2 0/2] keys: Enforce keep guard when moving keys Liu Mingyu
2026-05-31 20:51 ` [PATCH v2 1/2] " Liu Mingyu
@ 2026-05-31 20:51 ` Liu Mingyu
1 sibling, 0 replies; 4+ messages in thread
From: Liu Mingyu @ 2026-05-31 20:51 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: keyrings, linux-kernel, Eric Biggers, Kees Cook, Mimi Zohar,
Randy Dunlap
Add keyring KUnit tests that exercise successful key_move(), rejection
of moving a protected key out of a protected source keyring, and the
same-keyring no-op path.
Keep the test-only infrastructure separate from the KEYCTL_MOVE fix so
the fix can be backported without pulling in the KUnit module.
Signed-off-by: Mingyu Liu <edwardliu0214@outlook.com>
---
security/keys/Kconfig | 13 ++++
security/keys/Makefile | 1 +
security/keys/keyring_test.c | 121 +++++++++++++++++++++++++++++++++++
3 files changed, 135 insertions(+)
create mode 100644 security/keys/keyring_test.c
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 84f39e50ca36..acffb5f7385c 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -129,4 +129,17 @@ config KEY_NOTIFICATIONS
This makes use of pipes to handle the notification buffer and
provides KEYCTL_WATCH_KEY to enable/disable watches.
+config KEYS_KUNIT_TEST
+ tristate "KUnit tests for keyrings" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for keyring operations.
+ These tests exercise keyring link and move behavior, including
+ protection of KEY_FLAG_KEEP entries.
+ They are intended for KUnit runs on developer kernels and are not
+ needed for normal systems.
+
+ If you are unsure how to answer this question, answer N.
+
endif # KEYS
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 5f40807f05b3..fa583a4ea945 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SYSCTL) += sysctl.o
obj-$(CONFIG_PERSISTENT_KEYRINGS) += persistent.o
obj-$(CONFIG_KEY_DH_OPERATIONS) += dh.o
obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += keyctl_pkey.o
+obj-$(CONFIG_KEYS_KUNIT_TEST) += keyring_test.o
#
# Key types
diff --git a/security/keys/keyring_test.c b/security/keys/keyring_test.c
new file mode 100644
index 000000000000..0055b50224e9
--- /dev/null
+++ b/security/keys/keyring_test.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for keyring operations.
+ */
+
+#include <keys/user-type.h>
+#include <kunit/test.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/key.h>
+#include <linux/key-type.h>
+#include <linux/keyctl.h>
+#include <linux/module.h>
+#include <linux/uidgid.h>
+
+static void keyring_test_key_put(void *data)
+{
+ key_put(data);
+}
+
+static struct key *test_keyring_alloc(struct kunit *test, const char *desc,
+ unsigned long flags)
+{
+ struct key *keyring;
+
+ keyring = keyring_alloc(desc, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ current_cred(), KEY_POS_ALL | KEY_USR_ALL,
+ KEY_ALLOC_NOT_IN_QUOTA | flags, NULL, NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, keyring);
+ KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test,
+ keyring_test_key_put,
+ keyring), 0);
+
+ return keyring;
+}
+
+static struct key *test_user_key_alloc(struct kunit *test, const char *desc,
+ struct key *keyring,
+ unsigned long flags)
+{
+ static const char payload[] = "payload";
+ struct key *key;
+ int ret;
+
+ key = key_alloc(&key_type_user, desc, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ current_cred(), KEY_POS_ALL | KEY_USR_ALL,
+ KEY_ALLOC_NOT_IN_QUOTA | flags, NULL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, key);
+ KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test,
+ keyring_test_key_put,
+ key), 0);
+
+ ret = key_instantiate_and_link(key, payload, sizeof(payload),
+ keyring, NULL);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ return key;
+}
+
+static void keyring_move_user_key(struct kunit *test)
+{
+ struct key *from, *to, *key;
+ int ret;
+
+ from = test_keyring_alloc(test, "move-from", 0);
+ to = test_keyring_alloc(test, "move-to", 0);
+ key = test_user_key_alloc(test, "move-key", from, 0);
+
+ ret = key_move(key, from, to, 0);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ ret = key_move(key, to, from, 0);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+}
+
+static void keyring_move_keep_key_fails(struct kunit *test)
+{
+ struct key *from, *to, *key;
+ int ret;
+
+ from = test_keyring_alloc(test, "keep-from", KEY_ALLOC_SET_KEEP);
+ to = test_keyring_alloc(test, "keep-to", 0);
+ key = test_user_key_alloc(test, "keep-key", from, 0);
+
+ KUNIT_ASSERT_TRUE(test, test_bit(KEY_FLAG_KEEP, &from->flags));
+ KUNIT_ASSERT_TRUE(test, test_bit(KEY_FLAG_KEEP, &key->flags));
+
+ ret = key_move(key, from, to, 0);
+ KUNIT_EXPECT_EQ(test, ret, -EPERM);
+
+ ret = key_move(key, to, from, 0);
+ KUNIT_EXPECT_EQ(test, ret, -ENOENT);
+}
+
+static void keyring_move_keep_same_keyring(struct kunit *test)
+{
+ struct key *keyring, *key;
+ int ret;
+
+ keyring = test_keyring_alloc(test, "keep-same", KEY_ALLOC_SET_KEEP);
+ key = test_user_key_alloc(test, "keep-same-key", keyring, 0);
+
+ ret = key_move(key, keyring, keyring, 0);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+}
+
+static struct kunit_case keyring_test_cases[] = {
+ KUNIT_CASE(keyring_move_user_key),
+ KUNIT_CASE(keyring_move_keep_key_fails),
+ KUNIT_CASE(keyring_move_keep_same_keyring),
+ {}
+};
+
+static struct kunit_suite keyring_test_suite = {
+ .name = "keyring",
+ .test_cases = keyring_test_cases,
+};
+
+kunit_test_suite(keyring_test_suite);
+
+MODULE_LICENSE("GPL");
--
2.51.2.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread