Re: [PATCH v2 1/2] keys: Enforce keep guard when moving keys

[Jarkko Sakkinen <jarkko@kernel.org>]

On Sun, May 31, 2026 at 08:51:49PM +0000, Liu Mingyu wrote:
> 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

"Enforce the same resriction in key_move() with -EPERM ..."?

> 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
> 

That would makeobvious that something is missing here.

BR, Jarkko