Message ID | 20200914105537.18186-1-aryabinin@virtuozzo.com |
---|---|
State | New |
Series | "keys, user: Fix high order allocation in user_instantiate()" |
Headers | show |
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);
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
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(-)