[Devel] [PATCH RHEL COMMIT] keys, user: Fix high order allocation in user_instantiate() #PSBM-107794

[Konstantin Khorenko]

The commit is pushed to "branch-rh9-5.14.vz9.1.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after ark-5.14
------>
commit 4e3fa1553337e8fe4c76253af43b9198436ea342
Author: Andrey Ryabinin <ryabinin.a.a at gmail.com>
Date:   Thu Sep 30 16:03:50 2021 +0300

    keys, user: Fix high order allocation in user_instantiate() #PSBM-107794
    
    Adding user key might trigger 4-order allocation which is unreliable
    in case of fragmented memory:
    
     ------------[ cut here ]------------
     WARNING: CPU: 3 PID: 134927 at mm/page_alloc.c:3533 __alloc_pages_nodemask+0x1b1/0x600
     order 4 >= 3, gfp 0x40d0
     Kernel panic - not syncing: panic_on_warn set ...
     CPU: 3 PID: 134927 Comm: add_key01 ve: 0 Kdump: loaded Tainted: G           OE  ------------   3.10.0-1127.18.2.vz7.163.15 #1 163.15
     Hardware name: Virtuozzo KVM, BIOS 1.11.0-2.vz7.2 04/01/2014
     Call Trace:
      dump_stack+0x19/0x1b
      panic+0xe8/0x21f
      __warn+0xfa/0x100
      warn_slowpath_fmt+0x5f/0x80
      __alloc_pages_nodemask+0x1b1/0x600
      alloc_pages_current+0x98/0x110
      kmalloc_order+0x18/0x40
      kmalloc_order_trace+0x26/0xa0
      __kmalloc+0x281/0x2a0
      user_instantiate+0x47/0x90
      __key_instantiate_and_link+0x54/0x100
      key_create_or_update+0x398/0x490
      SyS_add_key+0x12c/0x220
      system_call_fastpath+0x25/0x2a
    
    Use kvmalloc() to avoid potential -ENOMEM due to fragmentation.
    
    https://jira.sw.ru/browse/PSBM-107794
    Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
    
    (cherry picked from commit 499126f3b029a72e2883b386c94fe4fc94f32c91)
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
    
    +++
    keys,user: fix NULL-ptr dereference in user_destroy() #PSBM-108198
    
    key->payload.data could be NULL
    
    BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
    IP: user_destroy+0x13/0x30
    
    Call Trace:
      key_gc_unused_keys.constprop.1+0xfd/0x110
      key_garbage_collector+0x1d7/0x390
      process_one_work+0x185/0x440
      worker_thread+0x126/0x3c0
      kthread+0xd1/0xe0
      ret_from_fork_nospec_begin+0x7/0x21
    
    Add the necessary check to fix this.
    
    https://jira.sw.ru/browse/PSBM-108198
    Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
    
    mFixes: d77ff0bac744 ("keys, user: Fix high order allocation in user_instantiate()")
    (cherry picked from commit a0e271fd8929312b1c5dab72fbc8bc336a296b45)
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
    
    +++
    keys,user: Fix NULL-ptr dereference in user_free_preparse() #PSBM-108291
    
    user_free_preparse() can validly receive "prep" arg with NULL payload
    (prep->payload.data[0]) => add a check for that.
    
    key_create_or_update()
    {
            ...
            if (index_key.type->preparse) {
                    ret = index_key.type->preparse(&prep);
                    // user_preparse(), kvmalloc(), prep->payload.data[0] filled
                    ...
            }
            ...
            ret = __key_instantiate_and_link(key, &prep, keyring, NULL, &edit);
            // it sets prep->payload.data[0] to NULL
            ...
    error_free_prep:
            if (index_key.type->preparse)
                    index_key.type->free_preparse(&prep);
                    // user_free_preparse(), memset(prep->payload.data[0], ...)
                    // crash here
            ...
    }
    
    key_create_or_update()
     __key_instantiate_and_link()
      key->type->instantiate() == generic_key_instantiate()
       prep->payload.data[0] = NULL;
    
    mFixes: d77ff0bac744 ("keys, user: Fix high order allocation in user_instantiate()")
    https://jira.sw.ru/browse/PSBM-108291
    
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
    
    (cherry picked from vz8 commit 164392d23bad844e05064d22f6a9509ec4ceff40)
    Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
---
 security/keys/user_defined.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 749e2a4dcb13..b06aace58428 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -5,6 +5,7 @@
  * Written by David Howells (dhowells at redhat.com)
  */
 
+#include <linux/mm.h>
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -64,7 +65,7 @@ int user_preparse(struct key_preparsed_payload *prep)
 	if (datalen <= 0 || datalen > 32767 || !prep->data)
 		return -EINVAL;
 
-	upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
+	upayload = kvmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
 	if (!upayload)
 		return -ENOMEM;
 
@@ -82,7 +83,12 @@ EXPORT_SYMBOL_GPL(user_preparse);
  */
 void user_free_preparse(struct key_preparsed_payload *prep)
 {
-	kfree_sensitive(prep->payload.data[0]);
+	struct user_key_payload *upayload = prep->payload.data[0];
+
+	if (upayload) {
+		memset(upayload, 0, sizeof(*upayload) + upayload->datalen);
+		kvfree(upayload);
+	}
 }
 EXPORT_SYMBOL_GPL(user_free_preparse);
 
@@ -91,7 +97,8 @@ static void user_free_payload_rcu(struct rcu_head *head)
 	struct user_key_payload *payload;
 
 	payload = container_of(head, struct user_key_payload, rcu);
-	kfree_sensitive(payload);
+	memset(payload, 0, sizeof(*payload) + payload->datalen);
+	kvfree(payload);
 }
 
 /*
@@ -147,7 +154,10 @@ void user_destroy(struct key *key)
 {
 	struct user_key_payload *upayload = key->payload.data[0];
 
-	kfree_sensitive(upayload);
+	if (upayload) {
+		memset(upayload, 0, sizeof(*upayload) + upayload->datalen);
+		kvfree(upayload);
+	}
 }
 
 EXPORT_SYMBOL_GPL(user_destroy);