| Message ID | 20191205223721.40034-1-shakeelb@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memcg: account security cred as well to kmemcg | expand |
Shakeel Butt writes: >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 :-) >Signed-off-by: Shakeel Butt <shakeelb@google.com> Acked-by: Chris Down <chris@chrisdown.name>
On Thu, Dec 05, 2019 at 02:37:21PM -0800, Shakeel Butt 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. > > Signed-off-by: Shakeel Butt <shakeelb@google.com> Reviewed-by: Roman Gushchin <guro@fb.com> Thanks!
On Thu 05-12-19 14:37:21, Shakeel Butt 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. I am not familiar with this area much. What is the timelife of these objects? Do they go away with a task allocating them? > Signed-off-by: Shakeel Butt <shakeelb@google.com> > > --- > kernel/cred.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/cred.c b/kernel/cred.c > index c0a4c12d38b2..9ed51b70ed80 100644 > --- a/kernel/cred.c > +++ b/kernel/cred.c > @@ -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); > -- > 2.24.0.393.g34dc348eaf-goog >
On Fri, Dec 6, 2019 at 12:17 AM Michal Hocko <mhocko@kernel.org> wrote: > > On Thu 05-12-19 14:37:21, Shakeel Butt 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. > > 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. > > Signed-off-by: Shakeel Butt <shakeelb@google.com> > > > > --- > > kernel/cred.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/cred.c b/kernel/cred.c > > index c0a4c12d38b2..9ed51b70ed80 100644 > > --- a/kernel/cred.c > > +++ b/kernel/cred.c > > @@ -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); > > -- > > 2.24.0.393.g34dc348eaf-goog > > > > -- > Michal Hocko > SUSE Labs
On Thu, 5 Dec 2019 14:37:21 -0800 Shakeel Butt <shakeelb@google.com> 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?
On Fri, Dec 6, 2019 at 4:13 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Thu, 5 Dec 2019 14:37:21 -0800 Shakeel Butt <shakeelb@google.com> 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? Yes, I think it is simple and safe enough for stable.
On Fri 06-12-19 08:51:21, Shakeel Butt wrote: > On Fri, Dec 6, 2019 at 12:17 AM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Thu 05-12-19 14:37:21, Shakeel Butt 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. > > > > 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. > > > Signed-off-by: Shakeel Butt <shakeelb@google.com> With that clarification Acked-by: Michal Hocko <mhocko@suse.com> > > > > > > --- > > > kernel/cred.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/kernel/cred.c b/kernel/cred.c > > > index c0a4c12d38b2..9ed51b70ed80 100644 > > > --- a/kernel/cred.c > > > +++ b/kernel/cred.c > > > @@ -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); > > > -- > > > 2.24.0.393.g34dc348eaf-goog > > > > > > > -- > > Michal Hocko > > SUSE Labs
diff --git a/kernel/cred.c b/kernel/cred.c index c0a4c12d38b2..9ed51b70ed80 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -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);
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 <shakeelb@google.com> --- kernel/cred.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)