linux-kernel - Re: [RESEND PATCH v2 2/2] memcg, fsnotify: no oom-kill for remote memcg charging

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] [day] [month] [year] [list]

Message-ID: <20190513085114.GD24036@dhcp22.suse.cz>
Date:   Mon, 13 May 2019 10:51:14 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Shakeel Butt <shakeelb@...gle.com>
Cc:     Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Roman Gushchin <guro@...com>, Jan Kara <jack@...e.cz>,
        Amir Goldstein <amir73il@...il.com>, linux-mm@...ck.org,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [RESEND PATCH v2 2/2] memcg, fsnotify: no oom-kill for remote
 memcg charging

On Sun 12-05-19 09:09:27, Shakeel Butt wrote:
[...]
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 6b9c27548997..f78fd4c8f12d 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -288,10 +288,13 @@ struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
>  	/*
>  	 * For queues with unlimited length lost events are not expected and
>  	 * can possibly have security implications. Avoid losing events when
> -	 * memory is short.
> +	 * memory is short. Also make sure to not trigger OOM killer in the
> +	 * target memcg for the limited size queues.
>  	 */
>  	if (group->max_events == UINT_MAX)
>  		gfp |= __GFP_NOFAIL;
> +	else
> +		gfp |= __GFP_RETRY_MAYFAIL;
>  
>  	/* Whoever is interested in the event, pays for the allocation. */
>  	memalloc_use_memcg(group->memcg);
> diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
> index ff30abd6a49b..17c08daa1ba7 100644
> --- a/fs/notify/inotify/inotify_fsnotify.c
> +++ b/fs/notify/inotify/inotify_fsnotify.c
> @@ -99,9 +99,12 @@ int inotify_handle_event(struct fsnotify_group *group,
>  	i_mark = container_of(inode_mark, struct inotify_inode_mark,
>  			      fsn_mark);
>  
> -	/* Whoever is interested in the event, pays for the allocation. */
> +	/*
> +	 * Whoever is interested in the event, pays for the allocation. However
> +	 * do not trigger the OOM killer in the target memcg.

Both comments would be much more helpful if they mentioned _why_ we do
not want to trigger the OOM iller.

> +	 */
>  	memalloc_use_memcg(group->memcg);
> -	event = kmalloc(alloc_len, GFP_KERNEL_ACCOUNT);
> +	event = kmalloc(alloc_len, GFP_KERNEL_ACCOUNT | __GFP_RETRY_MAYFAIL);
>  	memalloc_unuse_memcg();
>  
>  	if (unlikely(!event)) {
-- 
Michal Hocko
SUSE Labs