netdev - Re: [patch 1/1] net/netfilter/x_tables.c: make allocation less aggressive

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1517342038.3715.97.camel@gmail.com>
Date:   Tue, 30 Jan 2018 11:53:58 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     akpm@...ux-foundation.org, davem@...emloft.net,
        netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
        mhocko@...nel.org, fw@...len.de, mhocko@...e.com
Subject: Re: [patch 1/1] net/netfilter/x_tables.c: make allocation less
 aggressive

On Tue, 2018-01-30 at 11:30 -0800, akpm@...ux-foundation.org wrote:
> From: Michal Hocko <mhocko@...nel.org>
> Subject: net/netfilter/x_tables.c: make allocation less aggressive
> 
> syzbot has noticed that xt_alloc_table_info can allocate a lot of memory. 
> This is an admin only interface but an admin in a namespace is sufficient
> as well.  eacd86ca3b03 ("net/netfilter/x_tables.c: use kvmalloc() in
> xt_alloc_table_info()") has changed the opencoded kmalloc->vmalloc
> fallback into kvmalloc.  It has dropped __GFP_NORETRY on the way because
> vmalloc has simply never fully supported __GFP_NORETRY semantic.  This is
> still the case because e.g.  page tables backing the vmalloc area are
> hardcoded GFP_KERNEL.
> 
> Revert back to __GFP_NORETRY as a poors man defence against excessively
> large allocation request here.  We will not rule out the OOM killer
> completely but __GFP_NORETRY should at least stop the large request in
> most cases.
> 
> [akpm@...ux-foundation.org: coding-style fixes]
> Fixes: eacd86ca3b03 ("net/netfilter/x_tables.c: use kvmalloc() in xt_alloc_tableLink: http://lkml.kernel.org/r/20180130140104.GE21609@dhcp22.suse.cz
> Signed-off-by: Michal Hocko <mhocko@...e.com>
> Acked-by: Florian Westphal <fw@...len.de>
> Reviewed-by: Andrew Morton <akpm@...ux-foundation.org>
> Cc: David S. Miller <davem@...emloft.net>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
> 
>  net/netfilter/x_tables.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff -puN net/netfilter/x_tables.c~net-netfilter-x_tablesc-make-allocation-less-aggressive net/netfilter/x_tables.c
> --- a/net/netfilter/x_tables.c~net-netfilter-x_tablesc-make-allocation-less-aggressive
> +++ a/net/netfilter/x_tables.c
> @@ -1008,7 +1008,12 @@ struct xt_table_info *xt_alloc_table_inf
>  	if ((size >> PAGE_SHIFT) + 2 > totalram_pages)
>  		return NULL;
>  
> -	info = kvmalloc(sz, GFP_KERNEL);
> +	/* __GFP_NORETRY is not fully supported by kvmalloc but it should
> +	 * work reasonably well if sz is too large and bail out rather
> +	 * than shoot all processes down before realizing there is nothing
> +	 * more to reclaim.
> +	 */
> +	info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
>  	if (!info)
>  		return NULL;


How is __GFP_NORETRY working exactly ?

Surely, if some firewall tools attempt to load a new iptables rules, we
do not want to abort them if the request can be satisfied after few
pages moved on swap or written back to disk.

We want to avoid huge allocations, but leave reasonable ones succeed.

Thanks.