ACK: [PATCH] keys: Do not cache key in task struct if key is requested from kernel thread

[Philip Cox]

On Wed, Apr 26, 2023 at 1:19 PM Tim Gardner <tim.gardner at canonical.com>
wrote:

> From: David Howells <dhowells at redhat.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2017801
>
> [ Upstream commit 47f9e4c924025c5be87959d3335e66fcbb7f6b5c ]
>
> The key which gets cached in task structure from a kernel thread does not
> get invalidated even after expiry.  Due to which, a new key request from
> kernel thread will be served with the cached key if it's present in task
> struct irrespective of the key validity.  The change is to not cache key in
> task_struct when key requested from kernel thread so that kernel thread
> gets a valid key on every key request.
>
> The problem has been seen with the cifs module doing DNS lookups from a
> kernel thread and the results getting pinned by being attached to that
> kernel thread's cache - and thus not something that can be easily got rid
> of.  The cache would ordinarily be cleared by notify-resume, but kernel
> threads don't do that.
>
> This isn't seen with AFS because AFS is doing request_key() within the
> kernel half of a user thread - which will do notify-resume.
>
> Fixes: 7743c48e54ee ("keys: Cache result of request_key*() temporarily in
> task_struct")
> Signed-off-by: Bharath SM <bharathsm at microsoft.com>
> Signed-off-by: David Howells <dhowells at redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko at kernel.org>
> cc: Shyam Prasad N <nspmangalore at gmail.com>
> cc: Steve French <smfrench at gmail.com>
> cc: keyrings at vger.kernel.org
> cc: linux-cifs at vger.kernel.org
> cc: linux-fsdevel at vger.kernel.org
> Link:
> https://lore.kernel.org/r/CAGypqWw951d=zYRbdgNR4snUDvJhWL=q3=WOyh7HhSJupjz2vA@mail.gmail.com/
> Signed-off-by
> <https://lore.kernel.org/r/CAGypqWw951d=zYRbdgNR4snUDvJhWL=q3=WOyh7HhSJupjz2vA@mail.gmail.com/Signed-off-by>:
> Sasha Levin <sashal at kernel.org>
> (cherry picked from commit 97674f4cd05ef35963a5f73ba785582c82801982
> linux-5.4.y)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  security/keys/request_key.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/security/keys/request_key.c b/security/keys/request_key.c
> index 957b9e3e1492..17c9c0cfb6f5 100644
> --- a/security/keys/request_key.c
> +++ b/security/keys/request_key.c
> @@ -38,9 +38,12 @@ static void cache_requested_key(struct key *key)
>  #ifdef CONFIG_KEYS_REQUEST_CACHE
>         struct task_struct *t = current;
>
> -       key_put(t->cached_requested_key);
> -       t->cached_requested_key = key_get(key);
> -       set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
> +       /* Do not cache key if it is a kernel thread */
> +       if (!(t->flags & PF_KTHREAD)) {
> +               key_put(t->cached_requested_key);
> +               t->cached_requested_key = key_get(key);
> +               set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
> +       }
>  #endif
>  }
>
> --
> 2.34.1
>
>
> --
>
> Acked-by: Philip Cox <philip.cox at canonical.com>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230426/d1f1c919/attachment.html>