linux-kernel - Re: [PATCH] fs, mm: account filp and names caches to kmemcg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171006075900.icqjx5rr7hctn3zd@dhcp22.suse.cz>
Date:   Fri, 6 Oct 2017 09:59:00 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Shakeel Butt <shakeelb@...gle.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Greg Thelen <gthelen@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs, mm: account filp and names caches to kmemcg

On Thu 05-10-17 15:21:44, Shakeel Butt wrote:
> The allocations from filp and names kmem caches can be directly
> triggered by user space applications. A buggy application can
> consume a significant amount of unaccounted system memory. Though
> we have not noticed such buggy applications in our production
> but upon close inspection, we found that a lot of machines spend
> very significant amount of memory on these caches. So, these
> caches should be accounted to kmemcg.
> 
> Signed-off-by: Shakeel Butt <shakeelb@...gle.com>
> ---
>  fs/dcache.c     | 2 +-
>  fs/file_table.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index f90141387f01..fb3449161063 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -3642,7 +3642,7 @@ void __init vfs_caches_init_early(void)
>  void __init vfs_caches_init(void)
>  {
>  	names_cachep = kmem_cache_create("names_cache", PATH_MAX, 0,
> -			SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> +			SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);

I might be wrong but isn't name cache only holding temporary objects
used for path resolution which are not stored anywhere?

>  
>  	dcache_init();
>  	inode_init();
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 61517f57f8ef..567888cdf7d3 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -312,7 +312,7 @@ void put_filp(struct file *file)
>  void __init files_init(void)
>  {
>  	filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
> -			SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
> +			SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT, NULL);
>  	percpu_counter_init(&nr_files, 0, GFP_KERNEL);
>  }

Don't we have a limit for the maximum number of open files?

-- 
Michal Hocko
SUSE Labs