linux-kernel - Re: [PATCH net] bpf: cpumap: use GFP_KERNEL instead of GFP_ATOMIC in __cpu_map_entry

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]

Date:   Wed, 14 Feb 2018 19:04:56 +0200
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Michal Hocko <mhocko@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        akpm@...ux-foundation.org, dhowells@...hat.com, hannes@...xchg.org
Subject: Re: [PATCH net] bpf: cpumap: use GFP_KERNEL instead of GFP_ATOMIC in
 __cpu_map_entry_alloc()

On Wed, Feb 14, 2018 at 10:17:34PM +0800, Jason Wang wrote:
> There're several implications after commit 0bf7800f1799 ("ptr_ring:
> try vmalloc() when kmalloc() fails") with the using of vmalloc() since
> can't allow GFP_ATOMIC but mandate GFP_KERNEL. This will lead a WARN
> since cpumap try to call with GFP_ATOMIC. Fortunately, entry
> allocation of cpumap can only be done through syscall path which means
> GFP_ATOMIC is not necessary, so fixing this by replacing GFP_ATOMIC
> with GFP_KERNEL.
> 
> Reported-by: syzbot+1a240cdb1f4cc88819df@...kaller.appspotmail.com
> Fixes: 0bf7800f1799 ("ptr_ring: try vmalloc() when kmalloc() fails")
> Cc: Michal Hocko <mhocko@...nel.org>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: Matthew Wilcox <willy@...radead.org>
> Cc: Jesper Dangaard Brouer <brouer@...hat.com>
> Cc: akpm@...ux-foundation.org
> Cc: dhowells@...hat.com
> Cc: hannes@...xchg.org
> Signed-off-by: Jason Wang <jasowang@...hat.com>

Frankly I'd start with the revert. The original patch was rushed
into net without enough justification IMHO, and we just seem to keep
piling up these things. How about deferring all these ideas
to net-next?

> ---
>  kernel/bpf/cpumap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index fbfdada6..a4bb0b3 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -334,7 +334,7 @@ static int cpu_map_kthread_run(void *data)
>  static struct bpf_cpu_map_entry *__cpu_map_entry_alloc(u32 qsize, u32 cpu,
>  						       int map_id)
>  {
> -	gfp_t gfp = GFP_ATOMIC|__GFP_NOWARN;
> +	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
>  	struct bpf_cpu_map_entry *rcpu;
>  	int numa, err;
>  
> -- 
> 2.7.4