Re: [PATCH 4.17 REGRESSION fix] Revert "staging:r8188eu: Use lib80211 to support TKIP"
On 07/14/2018 09:31 PM, Hans de Goede wrote:
Commit b83b8b1881c4 ("staging:r8188eu: Use lib80211 to support TKIP")
is causing 2 problems for me:
1) One boot the wifi on a laptop with a r8188eu wifi device would not
connect and dmesg contained an oops about scheduling while atomic
pointing to the tkip code. This went away after reverting the commit.
2) I reverted the revert to try and get the oops from 1. again to be able
to add it to this commit message. But now the system did connect to the
wifi only to print a whole bunch of oopses, followed by a hardfreeze a
few seconds later. Subsequent reboots also all lead to scenario 2. Until
I reverted the commit again.
Revert the commit fixes both issues making the laptop usable again.
It is really bad idea to load module in tasklet.
Acked-by: Ivan Safonov
Cc: sta...@vger.kernel.org
Cc: Ivan Safonov
Signed-off-by: Hans de Goede
---
drivers/staging/rtl8188eu/Kconfig | 1 -
drivers/staging/rtl8188eu/core/rtw_recv.c | 161 +-
drivers/staging/rtl8188eu/core/rtw_security.c | 92 +-
3 files changed, 160 insertions(+), 94 deletions(-)
diff --git a/drivers/staging/rtl8188eu/Kconfig
b/drivers/staging/rtl8188eu/Kconfig
index 673fdce25530..ff7832798a77 100644
--- a/drivers/staging/rtl8188eu/Kconfig
+++ b/drivers/staging/rtl8188eu/Kconfig
@@ -7,7 +7,6 @@ config R8188EU
select LIB80211
select LIB80211_CRYPT_WEP
select LIB80211_CRYPT_CCMP
- select LIB80211_CRYPT_TKIP
---help---
This option adds the Realtek RTL8188EU USB device such as TP-Link
TL-WN725N.
If built as a module, it will be called r8188eu.
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 05936a45eb93..c6857a5be12a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -23,7 +23,6 @@
#include
#include
#include
-#include
#define ETHERNET_HEADER_SIZE 14 /* Ethernet Header Length */
#define LLC_HEADER_SIZE 6 /* LLC Header Length */
@@ -221,20 +220,31 @@ u32 rtw_free_uc_swdec_pending_queue(struct adapter
*adapter)
static int recvframe_chkmic(struct adapter *adapter,
struct recv_frame *precvframe)
{
- int res = _SUCCESS;
- struct rx_pkt_attrib *prxattrib = >attrib;
- struct sta_info *stainfo = rtw_get_stainfo(>stapriv,
prxattrib->ta);
+ int i, res = _SUCCESS;
+ u32 datalen;
+ u8 miccode[8];
+ u8 bmic_err = false, brpt_micerror = true;
+ u8 *pframe, *payload, *pframemic;
+ u8 *mickey;
+ struct sta_info*stainfo;
+ struct rx_pkt_attrib *prxattrib = >attrib;
+ struct security_priv *psecuritypriv = >securitypriv;
+
+ struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+
+ stainfo = rtw_get_stainfo(>stapriv, >ta[0]);
if (prxattrib->encrypt == _TKIP_) {
+ RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
+("\n %s: prxattrib->encrypt==_TKIP_\n", __func__));
+ RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
+("\n %s:
da=0x%02x:0x%02x:0x%02x:0x%02x:0x%02x:0x%02x\n",
+ __func__, prxattrib->ra[0], prxattrib->ra[1],
prxattrib->ra[2],
+ prxattrib->ra[3], prxattrib->ra[4],
prxattrib->ra[5]));
+
+ /* calculate mic code */
if (stainfo) {
- int key_idx;
- const int iv_len = 8, icv_len = 4, key_length = 32;
- struct sk_buff *skb = precvframe->pkt;
- u8 key[32], iv[8], icv[4], *pframe = skb->data;
- void *crypto_private = NULL;
- struct lib80211_crypto_ops *crypto_ops =
try_then_request_module(lib80211_get_crypto_ops("TKIP"), "lib80211_crypt_tkip");
- struct security_priv *psecuritypriv =
>securitypriv;
-
if (IS_MCAST(prxattrib->ra)) {
if (!psecuritypriv) {
res = _FAIL;
@@ -243,58 +253,115 @@ static int recvframe_chkmic(struct adapter *adapter,
DBG_88E("\n %s: didn't install group
key!!\n", __func__);
goto exit;
}
- key_idx = prxattrib->key_index;
- memcpy(key,
psecuritypriv->dot118021XGrpKey[key_idx].skey, 16);
- memcpy(key + 16,
psecuritypriv->dot118021XGrprxmickey[key_idx].skey, 16);
+ mickey =
>dot118021XGrprxmickey[prxattrib->key_index].skey[0];
+
+
[PATCH] wireless/lib80211: Convert from ahash to shash
In preparing to remove all stack VLA usage from the kernel[1], this
removes the discouraged use of AHASH_REQUEST_ON_STACK in favor of
the smaller SHASH_DESC_ON_STACK by converting from ahash-wrapped-shash
to direct shash. The stack allocation will be made a fixed size in a
later patch to the crypto subsystem.
[1]
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com
Signed-off-by: Kees Cook
---
net/wireless/lib80211_crypt_tkip.c | 55 --
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/net/wireless/lib80211_crypt_tkip.c
b/net/wireless/lib80211_crypt_tkip.c
index ba0a1f398ce5..e6bce1f130c9 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -65,9 +65,9 @@ struct lib80211_tkip_data {
int key_idx;
struct crypto_skcipher *rx_tfm_arc4;
- struct crypto_ahash *rx_tfm_michael;
+ struct crypto_shash *rx_tfm_michael;
struct crypto_skcipher *tx_tfm_arc4;
- struct crypto_ahash *tx_tfm_michael;
+ struct crypto_shash *tx_tfm_michael;
/* scratch buffers for virt_to_page() (crypto API) */
u8 rx_hdr[16], tx_hdr[16];
@@ -106,8 +106,7 @@ static void *lib80211_tkip_init(int key_idx)
goto fail;
}
- priv->tx_tfm_michael = crypto_alloc_ahash("michael_mic", 0,
- CRYPTO_ALG_ASYNC);
+ priv->tx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->tx_tfm_michael)) {
priv->tx_tfm_michael = NULL;
goto fail;
@@ -120,8 +119,7 @@ static void *lib80211_tkip_init(int key_idx)
goto fail;
}
- priv->rx_tfm_michael = crypto_alloc_ahash("michael_mic", 0,
- CRYPTO_ALG_ASYNC);
+ priv->rx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->rx_tfm_michael)) {
priv->rx_tfm_michael = NULL;
goto fail;
@@ -131,9 +129,9 @@ static void *lib80211_tkip_init(int key_idx)
fail:
if (priv) {
- crypto_free_ahash(priv->tx_tfm_michael);
+ crypto_free_shash(priv->tx_tfm_michael);
crypto_free_skcipher(priv->tx_tfm_arc4);
- crypto_free_ahash(priv->rx_tfm_michael);
+ crypto_free_shash(priv->rx_tfm_michael);
crypto_free_skcipher(priv->rx_tfm_arc4);
kfree(priv);
}
@@ -145,9 +143,9 @@ static void lib80211_tkip_deinit(void *priv)
{
struct lib80211_tkip_data *_priv = priv;
if (_priv) {
- crypto_free_ahash(_priv->tx_tfm_michael);
+ crypto_free_shash(_priv->tx_tfm_michael);
crypto_free_skcipher(_priv->tx_tfm_arc4);
- crypto_free_ahash(_priv->rx_tfm_michael);
+ crypto_free_shash(_priv->rx_tfm_michael);
crypto_free_skcipher(_priv->rx_tfm_arc4);
}
kfree(priv);
@@ -510,29 +508,36 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int
hdr_len, void *priv)
return keyidx;
}
-static int michael_mic(struct crypto_ahash *tfm_michael, u8 * key, u8 * hdr,
- u8 * data, size_t data_len, u8 * mic)
+static int michael_mic(struct crypto_shash *tfm_michael, u8 *key, u8 *hdr,
+ u8 *data, size_t data_len, u8 *mic)
{
- AHASH_REQUEST_ON_STACK(req, tfm_michael);
- struct scatterlist sg[2];
+ SHASH_DESC_ON_STACK(desc, tfm_michael);
int err;
if (tfm_michael == NULL) {
pr_warn("%s(): tfm_michael == NULL\n", __func__);
return -1;
}
- sg_init_table(sg, 2);
- sg_set_buf([0], hdr, 16);
- sg_set_buf([1], data, data_len);
- if (crypto_ahash_setkey(tfm_michael, key, 8))
+ desc->tfm = tfm_michael;
+ desc->flags = 0;
+
+ if (crypto_shash_setkey(tfm_michael, key, 8))
return -1;
- ahash_request_set_tfm(req, tfm_michael);
- ahash_request_set_callback(req, 0, NULL, NULL);
- ahash_request_set_crypt(req, sg, mic, data_len + 16);
- err = crypto_ahash_digest(req);
- ahash_request_zero(req);
+ err = crypto_shash_init(desc);
+ if (err)
+ goto out;
+ err = crypto_shash_update(desc, hdr, 16);
+ if (err)
+ goto out;
+ err = crypto_shash_update(desc, data, data_len);
+ if (err)
+ goto out;
+ err = crypto_shash_final(desc, mic);
+
+out:
+ shash_desc_zero(desc);
return err;
}
@@ -654,9 +659,9 @@ static int lib80211_tkip_set_key(void *key, int len, u8 *
seq, void *priv)
{
struct lib80211_tkip_data *tkey = priv;
int keyidx;
- struct crypto_ahash *tfm = tkey->tx_tfm_michael;
+ struct crypto_shash *tfm = tkey->tx_tfm_michael;
struct
Re: [PATCH v2] mac80211: Fix wlan freezes/clear text packet leaks at rekey
Hi, >>> So I think we're probably better off accepting the set_key but not >>> actually using it, and instead disconnecting... even if that's awkward >>> and should come with a big comment :-) >> >> Instead of returning an error I'll change the code to accept the rekey >> but do nothing with it. (Basically delete the new key and keep the old >> active). >> And of course calling ieee80211_set_disassoc() prior to return "success". > > Right. Did you handle/consider modes other than BSS/P2P client btw? I've tested that on a client only and it's already not working. Problem is, that with ieee80211_set_disassoc() we just dump the association in the kernel and are not informing the userspace, so the state machine is stuck in "connected" but the STA is unable to communicate. I also tried ieee80211_mgd_deauth(): While better this is basically the same behaviour as returning an error on key replace. By setting the error code to inactivity at least wpa_supplicant was actually starting to reconnect, unfortunatelly set_key then failes since we purged the assosication in the kernel. (Or that's my best interpretation from the logs). Networkmanager then again prompted for the password... I started experimenting with just signals to the userspace, but then paused... Trying to control the state machine with spoofed errors trying to trigger an "desireable" action can't be the way forward, can it? Even if we get that working changes or different implementations in userspace may well break it. I now think we only have two reasonable ways forward: 1) The user friendly one: If the userspace does not know about the new API just print a error message and do the insecure key replace. With potential clear text packet leaks and connection freezes. 2) The secure way: Report an error on key install and accept that users will encounter new problems when they are using rekeys with the old API with "old" userspace software. Since we have this issue in the kernel at least as long as we have mac80211 I would vote for 1) here. Fix mac80211 and add a new way for the userspace to to securely replace the keys and detect when this is not possible. Then get the userspace software updated to act accordingly, finally preventing the clear text packet leaks. After some time we can then decide to reject insecure rekeys, burning only those who use kernels much newer than the userspace. Does this sound like an reasonable approch or should I go back figuring out how to trick the userspace to reconnect without user notification/intervention? Alexander pEpkey.asc Description: application/pgp-keys
