[rh7] keys, user: Fix high order allocation in user_instantiate()

Submitted by Andrey Ryabinin on Sept. 14, 2020, 10:55 a.m.

Details

Message ID 20200914105537.18186-1-aryabinin@virtuozzo.com
State New
Series "keys, user: Fix high order allocation in user_instantiate()"
Headers show

Commit Message

Andrey Ryabinin Sept. 14, 2020, 10:55 a.m. 
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@virtuozzo.com>
---
 security/keys/user_defined.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index bc8d3227dc4b..b13d70b69069 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -9,6 +9,7 @@ 
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -75,7 +76,7 @@  int user_instantiate(struct key *key, struct key_preparsed_payload *prep)
 		goto error;
 
 	ret = -ENOMEM;
-	upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
+	upayload = kvmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
 	if (!upayload)
 		goto error;
 
@@ -96,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);
-	kzfree(payload);
+	memset(payload, 0, sizeof(*payload) + payload->datalen);
+	kvfree(payload);
 }
 
 /*
@@ -115,7 +117,7 @@  int user_update(struct key *key, struct key_preparsed_payload *prep)
 
 	/* construct a replacement payload */
 	ret = -ENOMEM;
-	upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
+	upayload = kvmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
 	if (!upayload)
 		goto error;
 
@@ -182,7 +184,8 @@  void user_destroy(struct key *key)
 {
 	struct user_key_payload *upayload = key->payload.data;
 
-	kzfree(upayload);
+	memset(upayload, 0, sizeof(*upayload) + upayload->datalen);
+	kvfree(upayload);
 }
 
 EXPORT_SYMBOL_GPL(user_destroy);

Comments

Denis V. Lunev Sept. 14, 2020, 11:03 a.m. 
On 9/14/20 1:55 PM, Andrey Ryabinin wrote:
> 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@virtuozzo.com>
> ---
>  security/keys/user_defined.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
> index bc8d3227dc4b..b13d70b69069 100644
> --- a/security/keys/user_defined.c
> +++ b/security/keys/user_defined.c
> @@ -9,6 +9,7 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>  
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -75,7 +76,7 @@ int user_instantiate(struct key *key, struct key_preparsed_payload *prep)
>  		goto error;
>  
>  	ret = -ENOMEM;
> -	upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
> +	upayload = kvmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
>  	if (!upayload)
>  		goto error;
>  
> @@ -96,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);
> -	kzfree(payload);
> +	memset(payload, 0, sizeof(*payload) + payload->datalen);
> +	kvfree(payload);
>  }
>  
>  /*
> @@ -115,7 +117,7 @@ int user_update(struct key *key, struct key_preparsed_payload *prep)
>  
>  	/* construct a replacement payload */
>  	ret = -ENOMEM;
> -	upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
> +	upayload = kvmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
>  	if (!upayload)
>  		goto error;
>  
> @@ -182,7 +184,8 @@ void user_destroy(struct key *key)
>  {
>  	struct user_key_payload *upayload = key->payload.data;
>  
> -	kzfree(upayload);
> +	memset(upayload, 0, sizeof(*upayload) + upayload->datalen);
> +	kvfree(upayload);
>  }
>  
>  EXPORT_SYMBOL_GPL(user_destroy);
can you pls add #PSBM-107794 into subject line. This will keep track of
the issue
in the kernel changelog