memcg: account security cred as well to kmemcg

PATCH Applied Shakeel Butt 5 participants 8 messages Dec 5, 2019 – Dec 9, 2019

Subsystems (2)

Memory management Credentials

lore

Share & tools

lore

Shakeel Butt

author

2019-12-05 22:39 UTC #0 lore raw

PATCH 1/1 memcg: account security cred as well to kmemcg

The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.

Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

Signed-off-by Shakeel Butt <[email protected]>

---
 kernel/cred.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/cred.c +3 -3

index c0a4c12d38b2..9ed51b70ed80 100644
@@ -223,7 +223,7 @@ struct cred *cred_alloc_blank(void)
 	new->magic = CRED_MAGIC;
 #endif
 
-	if (security_cred_alloc_blank(new, GFP_KERNEL) < 0)
+	if (security_cred_alloc_blank(new, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 
 	return new;

@@ -282,7 +282,7 @@ struct cred *prepare_creds(void)
 	new->security = NULL;
 #endif
 
-	if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+	if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 	validate_creds(new);
 	return new;

@@ -715,7 +715,7 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
 #ifdef CONFIG_SECURITY
 	new->security = NULL;
 #endif
-	if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+	if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 
 	put_cred(old);

Signature · 1 line

2.24.0.393.g34dc348eaf-goog

Chris Down

Acked-by on v1 1/1

2019-12-05 23:16 UTC #1 lore raw

Quoted 9 lines · Shakeel Butt

The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

Makes sense. I took a look through other cred-related allocations to see if any others stood out and this looks like it covers all the relevant cases. __alloc_file is the only other one that caught my eye, but SLAB_ACCOUNT is on the filp cache already.

Thanks :-)

Quoted 1 line

Signed-off-by Shakeel Butt <[email protected]>

Acked-by Chris Down <[email protected]>

Roman Gushchin

Reviewed-by on v1 1/1

2019-12-05 23:23 UTC #2 lore raw

In reply to Shakeel Butt

Quoted 11 lines · Shakeel Butt

The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.
Signed-off-by Shakeel Butt <[email protected]>

Reviewed-by Roman Gushchin <[email protected]>

Thanks!

Michal Hocko

2019-12-06 08:17 UTC #3 lore raw

In reply to Shakeel Butt

Quoted 9 lines · Shakeel Butt

The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

I am not familiar with this area much. What is the timelife of these objects? Do they go away with a task allocating them?

Quoted 40 lines

Signed-off-by Shakeel Butt <[email protected]>

---
 kernel/cred.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/cred.c +3 -3

index c0a4c12d38b2..9ed51b70ed80 100644
@@ -223,7 +223,7 @@ struct cred *cred_alloc_blank(void)
 	new->magic = CRED_MAGIC;
 #endif
 
-	if (security_cred_alloc_blank(new, GFP_KERNEL) < 0)
+	if (security_cred_alloc_blank(new, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 
 	return new;

@@ -282,7 +282,7 @@ struct cred *prepare_creds(void)
 	new->security = NULL;
 #endif
 
-	if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+	if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 	validate_creds(new);
 	return new;

@@ -715,7 +715,7 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
 #ifdef CONFIG_SECURITY
 	new->security = NULL;
 #endif
-	if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+	if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
 		goto error;
 
 	put_cred(old);

Signature · 1 line

2.24.0.393.g34dc348eaf-goog

Signature · 2 lines

Michal Hocko
SUSE Labs

Shakeel Butt

author

2019-12-06 16:51 UTC #4 lore raw

Quoted 12 lines · Michal Hocko

Quoted 9 lines · Shakeel Butt
The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

Quoted 3 lines

I am not familiar with this area much. What is the timelife of these objects? Do they go away with a task allocating them?

Lifetime is at least the life of the process allocating them.

Quoted 44 lines

Quoted 38 lines

Signed-off-by Shakeel Butt <[email protected]>

---
 kernel/cred.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/cred.c +3 -4

index c0a4c12d38b2..9ed51b70ed80 100644
@@ -223,7 +223,7 @@ struct cred *cred_alloc_blank(void)
      new->magic = CRED_MAGIC;
 #endif
 
-     if (security_cred_alloc_blank(new, GFP_KERNEL) < 0)
+     if (security_cred_alloc_blank(new, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
 
      return new;

@@ -282,7 +282,7 @@ struct cred *prepare_creds(void)
      new->security = NULL;
 #endif
 
-     if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+     if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
      validate_creds(new);
      return new;

@@ -715,7 +715,7 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
 #ifdef CONFIG_SECURITY
      new->security = NULL;
 #endif
-     if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+     if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
 
      put_cred(old);
--

Quoted 2 lines

2.24.0.393.g34dc348eaf-goog

--
Michal Hocko
SUSE Labs

Michal Hocko

Acked-by

2019-12-09 14:43 UTC #5 lore raw

Quoted 13 lines · Shakeel Butt

Quoted 12 lines · Michal Hocko
Quoted 9 lines · Shakeel Butt
The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

Quoted 5 lines

Quoted 3 lines
I am not familiar with this area much. What is the timelife of these objects? Do they go away with a task allocating them?
Lifetime is at least the life of the process allocating them.

Thanks for the clarification! It is better to be explicit about this in the changelog I believe because it would make a review much easier. Accounting for objects which are not bound to a user process context is more complex and requires much more considerations.

Quoted 1 line

Quoted 1 line
Quoted 1 line
Signed-off-by Shakeel Butt <[email protected]>

With that clarification

Acked-by Michal Hocko <[email protected]>

Quoted 43 lines

Quoted 37 lines

---
 kernel/cred.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/cred.c +3 -4

index c0a4c12d38b2..9ed51b70ed80 100644
@@ -223,7 +223,7 @@ struct cred *cred_alloc_blank(void)
      new->magic = CRED_MAGIC;
 #endif
 
-     if (security_cred_alloc_blank(new, GFP_KERNEL) < 0)
+     if (security_cred_alloc_blank(new, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
 
      return new;

@@ -282,7 +282,7 @@ struct cred *prepare_creds(void)
      new->security = NULL;
 #endif
 
-     if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+     if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
      validate_creds(new);
      return new;

@@ -715,7 +715,7 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
 #ifdef CONFIG_SECURITY
      new->security = NULL;
 #endif
-     if (security_prepare_creds(new, old, GFP_KERNEL) < 0)
+     if (security_prepare_creds(new, old, GFP_KERNEL_ACCOUNT) < 0)
              goto error;
 
      put_cred(old);
--

Quoted 2 lines

2.24.0.393.g34dc348eaf-goog

--
Michal Hocko
SUSE Labs

Signature · 2 lines

Michal Hocko
SUSE Labs

Andrew Morton

maintainer

2019-12-07 00:13 UTC #6 lore raw

In reply to Shakeel Butt

Quoted 9 lines · On Thu, 5 Dec 2019 14:37:21 -0800 Shakeel Butt <[email protected]> wrote:

The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.

A bit of an oversight and the fix is simple. Is it worth a cc:stable?

Shakeel Butt

author

2019-12-07 05:07 UTC #7 lore raw

Quoted 6 lines · Andrew Morton

Quoted 3 lines · On Thu, 5 Dec 2019 14:37:21 -0800 Shakeel Butt <[email protected]> wrote:
The cred_jar kmem_cache is already memcg accounted in the current kernel but cred->security is not. Account cred->security to kmemcg.

Quoted 8 lines

Quoted 6 lines
Recently we saw high root slab usage on our production and on further inspection, we found a buggy application leaking processes. Though that buggy application was contained within its memcg but we observe much more system memory overhead, couple of GiBs, during that period. This overhead can adversely impact the isolation on the system. One of source of high overhead, we found was cred->secuity objects.
A bit of an oversight and the fix is simple. Is it worth a cc:stable?

Yes, I think it is simple and safe enough for stable.