[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200811101746.GN1793@kadam>
Date: Tue, 11 Aug 2020 13:17:47 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: gregkh@...uxfoundation.org, sfr@...b.auug.org.au,
longman@...hat.com, akpm@...ux-foundation.org, mhocko@...e.com,
hannes@...xchg.org, devel@...verdev.osuosl.org,
kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: ks7010: Do not use GFP_KERNEL in atomic context
On Sun, Aug 09, 2020 at 01:18:46PM +0200, Christophe JAILLET wrote:
> A possible call chain is as follow:
> ks_wlan_start_xmit (ks_wlan_net.c)
> --> hostif_data_request (ks_hostif.c)
> --> michael_mic (ks_hostif.c)
>
> 'ks_wlan_start_xmit()' is a '.ndo_start_xmit()' function (see
> net_device_ops structure). Such calls are guarded by the __netif_tx_lock
> spinlock. So memory allocation must be atomic.
>
> So, use GFP_ATOMIC instead of GFP_KERNEL 'in michael_mic()'
>
> Fixes: ???
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> This is completely speculative. I don't know if the call chain given above
> if possible in RL application.
> So review carefully :)
>
> If the fix is correct, it is also more the starting point of a bigger
> change, because in 'michael_mic()' there is a call to
> 'crypto_alloc_shash()' and this function uses GFP_KERNEL internally (in
> 'crypto_create_tfm()')
> Should this need to be changed, I don't know how 'ks_hostif.c' should be
> fixed. Changing allocation in 'crypto/api.c' looks like an overkill.
>
> In other word, I think that my patch is wrong, but don't know what else to
> propose :).
Your patch is correct but you're also right that it's incomplete.
If you look at drivers/staging/rtl8192e/rtllib_crypt_tkip.c then they
declare the shash on stack instead of using crypto_alloc_shash().
SHASH_DESC_ON_STACK(desc, tfm_michael);
That's probably what we should do here as well. Although I don't know
this code very well at all... This is probably the sort of change where
it would be good to have someone test it.
regards,
dan carpenter
Powered by blists - more mailing lists