linux-kernel - Re: Question on kzalloc and GFP

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: <alpine.DEB.2.10.1609281407430.137474@chino.kir.corp.google.com>
Date:   Wed, 28 Sep 2016 14:11:33 -0700 (PDT)
From:   David Rientjes <rientjes@...gle.com>
To:     Ben Greear <greearb@...delatech.com>
cc:     linux-kernel@...r.kernel.org
Subject: Re: Question on kzalloc and GFP_DMA32

On Tue, 27 Sep 2016, Ben Greear wrote:

> 
> I have been running this patch for a while:
> 
>     ath10k:  Use GPF_DMA32 for firmware swap memory.
> 
>     This fixes OS crash when using QCA 9984 NIC on x86-64 system
>     without vt-d enabled.
> 
>     Also tested on ea8500 with 9980, and x86-64 with 9980 and 9880.
> 
>     All tests were with CT firmware.
> 
>     Signed-off-by: Ben Greear <greearb@...delatech.com>
> 
> -------------------- drivers/net/wireless/ath/ath10k/wmi.c
> --------------------
> index e20aa39..727b3aa 100644
> @@ -4491,7 +4491,7 @@ static int ath10k_wmi_alloc_chunk(struct ath10k *ar, u32
> req_id,
>          if (!pool_size)
>              return -EINVAL;
> 
> -        vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN);
> +        vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN | GFP_DMA32);
>          if (!vaddr)
>              num_units /= 2;
>      }
> 
> 
> It sometimes works, on some systems, but then it also often fails with the
> splat
> below, especially with several NICs in the system.
> 

I suppose it's failing sometimes because the BUG() will trigger when 
trying to allocate new slab or CONFIG_ZONE_DMA32 isn't configured.  That 
shouldn't panic the kernel anymore since commit 72baeef0c271 ("slab: do 
not panic on invalid gfp_mask") in 4.8, but you shouldn't be doing 
kzalloc(..., ... | GFP_DMA32) anyway.

> pool_size is relatively large (maybe 256k or so).
> 

If it's 256k, why allocate through the slab allocator?  Why not 
alloc_pages(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO | __GFP_NOWARN, 
get_order(pool_size))?