From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johannes Berg Subject: Re: [PATCH] mac80211: aes-cmac: remove VLA usage Date: Wed, 21 Mar 2018 14:48:14 +0100 Message-ID: <1521640094.2645.29.camel@sipsolutions.net> References: <20180321134247.GA1275@embeddedgus> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org To: "Gustavo A. R. Silva" , "David S. Miller" Return-path: In-Reply-To: <20180321134247.GA1275@embeddedgus> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Wed, 2018-03-21 at 08:42 -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wvla, remove VLAs and replace them > with dynamic memory allocation instead. > > The use of stack Variable Length Arrays needs to be avoided, as they > can be a vector for stack exhaustion, which can be both a runtime bug > or a security flaw. Also, in general, as code evolves it is easy to > lose track of how big a VLA can get. Thus, we can end up having runtime > failures that are hard to debug. > > Also, fixed as part of the directive to remove all VLAs from > the kernel: https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Gustavo A. R. Silva > --- > net/mac80211/aes_cmac.c | 36 ++++++++++++++++++++++++------------ > 1 file changed, 24 insertions(+), 12 deletions(-) > > diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c > index 2fb6558..c9444bf 100644 > --- a/net/mac80211/aes_cmac.c > +++ b/net/mac80211/aes_cmac.c > @@ -27,30 +27,42 @@ static const u8 zero[CMAC_TLEN_256]; > void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad, > const u8 *data, size_t data_len, u8 *mic) > { > - SHASH_DESC_ON_STACK(desc, tfm); > + struct shash_desc *shash; > u8 out[AES_BLOCK_SIZE]; > > - desc->tfm = tfm; > + shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm), > + GFP_KERNEL); > + if (!shash) > + return; Honestly, this seems like a really bad idea - you're now hitting kmalloc for every TX/RX frame here. SHA_DESC_ON_STACK() should just be fixed to not need a VLA, but take some sort of maximum, I guess? johannes