[Devel] [PATCH RH9 04/30] keys, user: Fix high order allocation in user_instantiate() #PSBM-107794

[Andrey Zhadchenko]

From: Andrey Ryabinin <aryabinin at virtuozzo.com>

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 749e2a4..b06aace 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 @@ int user_preparse(struct key_preparsed_payload *prep)
  */
 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);
-- 
1.8.3.1