Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Larry Finger @ 2017-02-04 18:41 UTC (permalink / raw)
  To: Dmitry Osipenko, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <f5682c5a-6c2c-8a35-61b6-1d7a30d2c1f6@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On 02/04/2017 10:58 AM, Dmitry Osipenko wrote:
> Seems the problem is caused by rtl92c_dm_*() casting .priv to "struct
> rtl_pci_priv", while it is "struct rtl_usb_priv".

Those routines are shared by rtl8192ce and rtl8192cu, thus we need to make that 
difference in cast to be immaterial. I think we need to move "struct 
bt_coexist_info" to the beginning of both rtlpci_priv and rtl_usb_priv. Then it 
should not matter.

I do not have a gcc version new enough to turn KASAN testing on, thus the 
attached patch is only compile tested. Does it fix the problem?

Larry


[-- Attachment #2: reorder_private_data.patch --]
[-- Type: text/x-patch, Size: 1053 bytes --]

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.h b/drivers/net/wireless/realtek/rtlwifi/pci.h
index 578b1d9..d9039ea 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.h
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.h
@@ -271,10 +271,10 @@ struct mp_adapter {
 };
 
 struct rtl_pci_priv {
+	struct bt_coexist_info bt_coexist;
+	struct rtl_led_ctl ledctl;
 	struct rtl_pci dev;
 	struct mp_adapter ndis_adapter;
-	struct rtl_led_ctl ledctl;
-	struct bt_coexist_info bt_coexist;
 };
 
 #define rtl_pcipriv(hw)		(((struct rtl_pci_priv *)(rtl_priv(hw))->priv))
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.h b/drivers/net/wireless/realtek/rtlwifi/usb.h
index a6d43d2..cdb9e06 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.h
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.h
@@ -146,8 +146,9 @@ struct rtl_usb {
 };
 
 struct rtl_usb_priv {
-	struct rtl_usb dev;
+	struct bt_coexist_info bt_coexist;
 	struct rtl_led_ctl ledctl;
+	struct rtl_usb dev;
 };
 
 #define rtl_usbpriv(hw)	 (((struct rtl_usb_priv *)(rtl_priv(hw))->priv))

^ permalink raw reply related

* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Dmitry Osipenko @ 2017-02-04 19:32 UTC (permalink / raw)
  To: Larry Finger, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <edbcbe3f-9b5d-cead-5885-27bc55ef550b@lwfinger.net>

On 04.02.2017 21:41, Larry Finger wrote:
> On 02/04/2017 10:58 AM, Dmitry Osipenko wrote:
>> Seems the problem is caused by rtl92c_dm_*() casting .priv to "struct
>> rtl_pci_priv", while it is "struct rtl_usb_priv".
> 
> Those routines are shared by rtl8192ce and rtl8192cu, thus we need to make that
> difference in cast to be immaterial. I think we need to move "struct
> bt_coexist_info" to the beginning of both rtlpci_priv and rtl_usb_priv. Then it
> should not matter.
> 
> I do not have a gcc version new enough to turn KASAN testing on, thus the
> attached patch is only compile tested. Does it fix the problem?

Thank you for the patch, it indeed fixes the bug.

I noticed that struct rtl_priv contains .btcoexist, isn't it duplicated in the
struct rtl_pci_priv?

-- 
Dmitry

^ permalink raw reply

* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Larry Finger @ 2017-02-05  1:05 UTC (permalink / raw)
  To: Dmitry Osipenko, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <b100135b-97c4-b2bc-4f19-2574e573878b@gmail.com>

On 02/04/2017 01:32 PM, Dmitry Osipenko wrote:
> On 04.02.2017 21:41, Larry Finger wrote:
>> On 02/04/2017 10:58 AM, Dmitry Osipenko wrote:
>>> Seems the problem is caused by rtl92c_dm_*() casting .priv to "struct
>>> rtl_pci_priv", while it is "struct rtl_usb_priv".
>>
>> Those routines are shared by rtl8192ce and rtl8192cu, thus we need to make that
>> difference in cast to be immaterial. I think we need to move "struct
>> bt_coexist_info" to the beginning of both rtlpci_priv and rtl_usb_priv. Then it
>> should not matter.
>>
>> I do not have a gcc version new enough to turn KASAN testing on, thus the
>> attached patch is only compile tested. Does it fix the problem?
>
> Thank you for the patch, it indeed fixes the bug.
>
> I noticed that struct rtl_priv contains .btcoexist, isn't it duplicated in the
> struct rtl_pci_priv?

Thanks for testing. When I submit the patch, is it OK to cite your reporting and 
testing?

Yes, the bt_coexist_info structure is in two different places. I will change the 
code in rtl8192c-common and rtl8192ce to use only the one in rtlpriv. That 
should satisfy the problem you reported, as well as clean up the code.

Thanks again,

Larry

^ permalink raw reply

* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Dmitry Osipenko @ 2017-02-05 11:34 UTC (permalink / raw)
  To: Larry Finger, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <3f1ad9bc-c065-4f1f-be78-70c0702867b4@lwfinger.net>

On 05.02.2017 04:05, Larry Finger wrote:
> On 02/04/2017 01:32 PM, Dmitry Osipenko wrote:
>> On 04.02.2017 21:41, Larry Finger wrote:
>>> On 02/04/2017 10:58 AM, Dmitry Osipenko wrote:
>>>> Seems the problem is caused by rtl92c_dm_*() casting .priv to "struct
>>>> rtl_pci_priv", while it is "struct rtl_usb_priv".
>>>
>>> Those routines are shared by rtl8192ce and rtl8192cu, thus we need to make that
>>> difference in cast to be immaterial. I think we need to move "struct
>>> bt_coexist_info" to the beginning of both rtlpci_priv and rtl_usb_priv. Then it
>>> should not matter.
>>>
>>> I do not have a gcc version new enough to turn KASAN testing on, thus the
>>> attached patch is only compile tested. Does it fix the problem?
>>
>> Thank you for the patch, it indeed fixes the bug.
>>
>> I noticed that struct rtl_priv contains .btcoexist, isn't it duplicated in the
>> struct rtl_pci_priv?
> 
> Thanks for testing. When I submit the patch, is it OK to cite your reporting and
> testing?

Sure, Tested-by: Dmitry Osipenko <digetx@gmail.com>

> Yes, the bt_coexist_info structure is in two different places. I will change the
> code in rtl8192c-common and rtl8192ce to use only the one in rtlpriv. That
> should satisfy the problem you reported, as well as clean up the code.
> 
> Thanks again,

Good, thank you.

BTW, I have an issue with the 8192cu: WiFi stops to work after a while (3-15
minutes) if I enable WMM QoS on the AP. There is nothing suspicious in KMSG,
connection is up but no packets go in/out. I tried to enable debug messages in
the driver, so when the WiFi stops to work I see that some "temperature/led"
notify still going on in the driver, but nothing happens when I try to initiate
a transfer (say to open a web page) - the log is silent, like the requests are
getting stuck/dropped somewhere before reaching the driver. Is it a known issue?
With the QoS disabled everything works hunky-dory, however I get 2x-4x faster
download speed with QoS enabled (while it works.)

I noticed that rtl92c_init_edca_param() isn't wired in the driver, so I suppose
the QoS isn't implemented yet, right?

If it is an expected behaviour, I think at least printing a warning message in
the KMSG like "QoS unimplemented, you may expect problems" should be good enough
to avoid confusion.

-- 
Dmitry

^ permalink raw reply

* [PATCH v2 0/2] mac80211: use crypto shash for AES cmac
From: Ard Biesheuvel @ 2017-02-05 15:23 UTC (permalink / raw)
  To: johannes, jouni, linux-wireless; +Cc: netdev, davem, Ard Biesheuvel

This is something I spotted while working on AES in various modes for
ARM and arm64.

The mac80211 aes_cmac code reimplements the CMAC algorithm based on the
core AES cipher, which is rather restrictive in how platforms can satisfy
the dependency on this algorithm. For instance, SIMD implementations may
have a considerable setup time, which cannot be amortized over the entire
input when calling into the crypto API one block at a time. Also, it prevents
the use of more secure fixed time implementations, since not all AES drivers
expose the cipher interface.

So switch aes_cmac to use a cmac(aes) shash. Before updating the aes_cmac code
in patch #2, the FILS AEAD code is moved to using a cmac(aes) shash supplied by
the crypto API so that we can remove the open coded version entirely in the
second patch.

NOTE: Jouni has been so kind to test patch #2, and confirmed that it is working.
      I have not tested patch #1 myself, mainly because the test methodology
      requires downloading Ubuntu installer images, and I am currently on a
      metered 3G connection (and will be for another couple of weeks)

Ard Biesheuvel (2):
  mac80211: fils_aead: Use crypto api CMAC shash rather than bare cipher
  mac80211: aes-cmac: switch to shash CMAC driver

 net/mac80211/Kconfig     |   1 +
 net/mac80211/aes_cmac.c  | 130 +++++---------------
 net/mac80211/aes_cmac.h  |  15 +--
 net/mac80211/fils_aead.c |  74 +++++------
 net/mac80211/key.h       |   2 +-
 5 files changed, 70 insertions(+), 152 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH v2 1/2] mac80211: fils_aead: Use crypto api CMAC shash rather than bare cipher
From: Ard Biesheuvel @ 2017-02-05 15:23 UTC (permalink / raw)
  To: johannes, jouni, linux-wireless; +Cc: netdev, davem, Ard Biesheuvel
In-Reply-To: <1486308208-3252-1-git-send-email-ard.biesheuvel@linaro.org>

Switch the FILS AEAD code to use a cmac(aes) shash instantiated by the
crypto API rather than reusing the open coded implementation in
aes_cmac_vector(). This makes the code more understandable, and allows
platforms to implement cmac(aes) in a more secure (*) and efficient way
than is typically possible when using the AES cipher directly.

So replace the crypto_cipher by a crypto_shash, and update the aes_s2v()
routine to call the shash interface directly.

* In particular, the generic table based AES implementation is sensitive
  to known-plaintext timing attacks on the key, to which AES based MAC
  algorithms are especially vulnerable, given that their plaintext is not
  usually secret. Time invariant alternatives are available (e.g., based
  on SIMD algorithms), but may incur a setup cost that is prohibitive when
  operating on a single block at a time, which is why they don't usually
  expose the cipher API.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/mac80211/Kconfig     |  1 +
 net/mac80211/aes_cmac.h  |  4 --
 net/mac80211/fils_aead.c | 74 +++++++++-----------
 3 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index 3891cbd2adea..76e30f4797fb 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -6,6 +6,7 @@ config MAC80211
 	select CRYPTO_AES
 	select CRYPTO_CCM
 	select CRYPTO_GCM
+	select CRYPTO_CMAC
 	select CRC32
 	---help---
 	  This option enables the hardware independent IEEE 802.11
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index c827e1d5de8b..3702041f44fd 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,10 +11,6 @@
 
 #include <linux/crypto.h>
 
-void gf_mulx(u8 *pad);
-void aes_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
-		     const u8 *addr[], const size_t *len, u8 *mac,
-		     size_t mac_len);
 struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
 						   size_t key_len);
 void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index ecfdd97758a3..a294a57e856d 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -9,66 +9,60 @@
 
 #include <crypto/aes.h>
 #include <crypto/algapi.h>
+#include <crypto/hash.h>
 #include <crypto/skcipher.h>
 
 #include "ieee80211_i.h"
 #include "aes_cmac.h"
 #include "fils_aead.h"
 
-static int aes_s2v(struct crypto_cipher *tfm,
+static void gf_mulx(u8 *pad)
+{
+	u64 a = get_unaligned_be64(pad);
+	u64 b = get_unaligned_be64(pad + 8);
+
+	put_unaligned_be64((a << 1) | (b >> 63), pad);
+	put_unaligned_be64((b << 1) ^ ((a >> 63) ? 0x87 : 0), pad + 8);
+}
+
+static int aes_s2v(struct crypto_shash *tfm,
 		   size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
 {
 	u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE];
+	struct shash_desc *desc;
+	u8 buf[sizeof(*desc) + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
 	size_t i;
-	const u8 *data[2];
-	size_t data_len[2], data_elems;
+
+	desc = (struct shash_desc *)buf;
+	desc->tfm = tfm;
 
 	/* D = AES-CMAC(K, <zero>) */
-	memset(tmp, 0, AES_BLOCK_SIZE);
-	data[0] = tmp;
-	data_len[0] = AES_BLOCK_SIZE;
-	aes_cmac_vector(tfm, 1, data, data_len, d, AES_BLOCK_SIZE);
+	crypto_shash_digest(desc, (u8[AES_BLOCK_SIZE]){}, AES_BLOCK_SIZE, d);
 
 	for (i = 0; i < num_elem - 1; i++) {
 		/* D = dbl(D) xor AES_CMAC(K, Si) */
 		gf_mulx(d); /* dbl */
-		aes_cmac_vector(tfm, 1, &addr[i], &len[i], tmp,
-				AES_BLOCK_SIZE);
+		crypto_shash_digest(desc, addr[i], len[i], tmp);
 		crypto_xor(d, tmp, AES_BLOCK_SIZE);
 	}
 
+	crypto_shash_init(desc);
+
 	if (len[i] >= AES_BLOCK_SIZE) {
 		/* len(Sn) >= 128 */
-		size_t j;
-		const u8 *pos;
-
 		/* T = Sn xorend D */
-
-		/* Use a temporary buffer to perform xorend on Sn (addr[i]) to
-		 * avoid modifying the const input argument.
-		 */
-		data[0] = addr[i];
-		data_len[0] = len[i] - AES_BLOCK_SIZE;
-		pos = addr[i] + data_len[0];
-		for (j = 0; j < AES_BLOCK_SIZE; j++)
-			tmp[j] = pos[j] ^ d[j];
-		data[1] = tmp;
-		data_len[1] = AES_BLOCK_SIZE;
-		data_elems = 2;
+		crypto_shash_update(desc, addr[i], len[i] - AES_BLOCK_SIZE);
+		crypto_xor(d, addr[i] + len[i] - AES_BLOCK_SIZE,
+			   AES_BLOCK_SIZE);
 	} else {
 		/* len(Sn) < 128 */
 		/* T = dbl(D) xor pad(Sn) */
 		gf_mulx(d); /* dbl */
-		memset(tmp, 0, AES_BLOCK_SIZE);
-		memcpy(tmp, addr[i], len[i]);
-		tmp[len[i]] = 0x80;
-		crypto_xor(d, tmp, AES_BLOCK_SIZE);
-		data[0] = d;
-		data_len[0] = sizeof(d);
-		data_elems = 1;
+		crypto_xor(d, addr[i], len[i]);
+		d[len[i]] ^= 0x80;
 	}
 	/* V = AES-CMAC(K, T) */
-	aes_cmac_vector(tfm, data_elems, data, data_len, v, AES_BLOCK_SIZE);
+	crypto_shash_finup(desc, d, AES_BLOCK_SIZE, v);
 
 	return 0;
 }
@@ -80,7 +74,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
 			   size_t len[], u8 *out)
 {
 	u8 v[AES_BLOCK_SIZE];
-	struct crypto_cipher *tfm;
+	struct crypto_shash *tfm;
 	struct crypto_skcipher *tfm2;
 	struct skcipher_request *req;
 	int res;
@@ -95,14 +89,14 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
 
 	/* S2V */
 
-	tfm = crypto_alloc_cipher("aes", 0, 0);
+	tfm = crypto_alloc_shash("cmac(aes)", 0, 0);
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
 	/* K1 for S2V */
-	res = crypto_cipher_setkey(tfm, key, key_len);
+	res = crypto_shash_setkey(tfm, key, key_len);
 	if (!res)
 		res = aes_s2v(tfm, num_elem, addr, len, v);
-	crypto_free_cipher(tfm);
+	crypto_free_shash(tfm);
 	if (res)
 		return res;
 
@@ -157,7 +151,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
 			   size_t num_elem, const u8 *addr[], size_t len[],
 			   u8 *out)
 {
-	struct crypto_cipher *tfm;
+	struct crypto_shash *tfm;
 	struct crypto_skcipher *tfm2;
 	struct skcipher_request *req;
 	struct scatterlist src[1], dst[1];
@@ -210,14 +204,14 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
 
 	/* S2V */
 
-	tfm = crypto_alloc_cipher("aes", 0, 0);
+	tfm = crypto_alloc_shash("cmac(aes)", 0, 0);
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
 	/* K1 for S2V */
-	res = crypto_cipher_setkey(tfm, key, key_len);
+	res = crypto_shash_setkey(tfm, key, key_len);
 	if (!res)
 		res = aes_s2v(tfm, num_elem, addr, len, check);
-	crypto_free_cipher(tfm);
+	crypto_free_shash(tfm);
 	if (res)
 		return res;
 	if (memcmp(check, frame_iv, AES_BLOCK_SIZE) != 0)
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 2/2] mac80211: aes-cmac: switch to shash CMAC driver
From: Ard Biesheuvel @ 2017-02-05 15:23 UTC (permalink / raw)
  To: johannes, jouni, linux-wireless; +Cc: netdev, davem, Ard Biesheuvel
In-Reply-To: <1486308208-3252-1-git-send-email-ard.biesheuvel@linaro.org>

Instead of open coding the CMAC algorithm in the mac80211 driver using
byte wide xors and calls into the crypto layer for each block of data,
instantiate a cmac(aes) synchronous hash and pass all the data into it
directly. This does not only simplify the code, it also allows the use
of more efficient and more secure implementations, especially on
platforms where SIMD ciphers have considerable setup time.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/mac80211/aes_cmac.c | 130 +++++---------------
 net/mac80211/aes_cmac.h |  11 +-
 net/mac80211/key.h      |   2 +-
 3 files changed, 35 insertions(+), 108 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index d0bd5fff5f0a..0d4d2af52a56 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -22,126 +22,52 @@
 #define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
 #define AAD_LEN 20
 
-
-void gf_mulx(u8 *pad)
-{
-	int i, carry;
-
-	carry = pad[0] & 0x80;
-	for (i = 0; i < AES_BLOCK_SIZE - 1; i++)
-		pad[i] = (pad[i] << 1) | (pad[i + 1] >> 7);
-	pad[AES_BLOCK_SIZE - 1] <<= 1;
-	if (carry)
-		pad[AES_BLOCK_SIZE - 1] ^= 0x87;
-}
-
-void aes_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
-		     const u8 *addr[], const size_t *len, u8 *mac,
-		     size_t mac_len)
-{
-	u8 cbc[AES_BLOCK_SIZE], pad[AES_BLOCK_SIZE];
-	const u8 *pos, *end;
-	size_t i, e, left, total_len;
-
-	memset(cbc, 0, AES_BLOCK_SIZE);
-
-	total_len = 0;
-	for (e = 0; e < num_elem; e++)
-		total_len += len[e];
-	left = total_len;
-
-	e = 0;
-	pos = addr[0];
-	end = pos + len[0];
-
-	while (left >= AES_BLOCK_SIZE) {
-		for (i = 0; i < AES_BLOCK_SIZE; i++) {
-			cbc[i] ^= *pos++;
-			if (pos >= end) {
-				e++;
-				pos = addr[e];
-				end = pos + len[e];
-			}
-		}
-		if (left > AES_BLOCK_SIZE)
-			crypto_cipher_encrypt_one(tfm, cbc, cbc);
-		left -= AES_BLOCK_SIZE;
-	}
-
-	memset(pad, 0, AES_BLOCK_SIZE);
-	crypto_cipher_encrypt_one(tfm, pad, pad);
-	gf_mulx(pad);
-
-	if (left || total_len == 0) {
-		for (i = 0; i < left; i++) {
-			cbc[i] ^= *pos++;
-			if (pos >= end) {
-				e++;
-				pos = addr[e];
-				end = pos + len[e];
-			}
-		}
-		cbc[left] ^= 0x80;
-		gf_mulx(pad);
-	}
-
-	for (i = 0; i < AES_BLOCK_SIZE; i++)
-		pad[i] ^= cbc[i];
-	crypto_cipher_encrypt_one(tfm, pad, pad);
-	memcpy(mac, pad, mac_len);
-}
-
-
-void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
+void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
 			const u8 *data, size_t data_len, u8 *mic)
 {
-	const u8 *addr[3];
-	size_t len[3];
-	u8 zero[CMAC_TLEN];
+	struct shash_desc *desc;
+	u8 buf[sizeof(*desc) + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
+	u8 out[crypto_shash_digestsize(tfm)];
 
-	memset(zero, 0, CMAC_TLEN);
-	addr[0] = aad;
-	len[0] = AAD_LEN;
-	addr[1] = data;
-	len[1] = data_len - CMAC_TLEN;
-	addr[2] = zero;
-	len[2] = CMAC_TLEN;
+	desc = (struct shash_desc *)buf;
+	desc->tfm = tfm;
 
-	aes_cmac_vector(tfm, 3, addr, len, mic, CMAC_TLEN);
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, aad, AAD_LEN);
+	crypto_shash_update(desc, data, data_len - CMAC_TLEN);
+	crypto_shash_finup(desc, (u8[CMAC_TLEN]){}, CMAC_TLEN, out);
+
+	memcpy(mic, out, CMAC_TLEN);
 }
 
-void ieee80211_aes_cmac_256(struct crypto_cipher *tfm, const u8 *aad,
+void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
 			    const u8 *data, size_t data_len, u8 *mic)
 {
-	const u8 *addr[3];
-	size_t len[3];
-	u8 zero[CMAC_TLEN_256];
+	struct shash_desc *desc;
+	u8 buf[sizeof(*desc) + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR;
 
-	memset(zero, 0, CMAC_TLEN_256);
-	addr[0] = aad;
-	len[0] = AAD_LEN;
-	addr[1] = data;
-	len[1] = data_len - CMAC_TLEN_256;
-	addr[2] = zero;
-	len[2] = CMAC_TLEN_256;
+	desc = (struct shash_desc *)buf;
+	desc->tfm = tfm;
 
-	aes_cmac_vector(tfm, 3, addr, len, mic, CMAC_TLEN_256);
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, aad, AAD_LEN);
+	crypto_shash_update(desc, data, data_len - CMAC_TLEN_256);
+	crypto_shash_finup(desc, (u8[CMAC_TLEN_256]){}, CMAC_TLEN_256, mic);
 }
 
-struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
-						   size_t key_len)
+struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
+						  size_t key_len)
 {
-	struct crypto_cipher *tfm;
+	struct crypto_shash *tfm;
 
-	tfm = crypto_alloc_cipher("aes", 0, CRYPTO_ALG_ASYNC);
+	tfm = crypto_alloc_shash("cmac(aes)", 0, 0);
 	if (!IS_ERR(tfm))
-		crypto_cipher_setkey(tfm, key, key_len);
+		crypto_shash_setkey(tfm, key, key_len);
 
 	return tfm;
 }
 
-
-void ieee80211_aes_cmac_key_free(struct crypto_cipher *tfm)
+void ieee80211_aes_cmac_key_free(struct crypto_shash *tfm)
 {
-	crypto_free_cipher(tfm);
+	crypto_free_shash(tfm);
 }
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 3702041f44fd..fef531f42003 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -10,13 +10,14 @@
 #define AES_CMAC_H
 
 #include <linux/crypto.h>
+#include <crypto/hash.h>
 
-struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
-						   size_t key_len);
-void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
+struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
+						  size_t key_len);
+void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
 			const u8 *data, size_t data_len, u8 *mic);
-void ieee80211_aes_cmac_256(struct crypto_cipher *tfm, const u8 *aad,
+void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
 			    const u8 *data, size_t data_len, u8 *mic);
-void ieee80211_aes_cmac_key_free(struct crypto_cipher *tfm);
+void ieee80211_aes_cmac_key_free(struct crypto_shash *tfm);
 
 #endif /* AES_CMAC_H */
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index 4aa20cef0859..ebdb80b85dc3 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -93,7 +93,7 @@ struct ieee80211_key {
 		} ccmp;
 		struct {
 			u8 rx_pn[IEEE80211_CMAC_PN_LEN];
-			struct crypto_cipher *tfm;
+			struct crypto_shash *tfm;
 			u32 replays; /* dot11RSNAStatsCMACReplays */
 			u32 icverrors; /* dot11RSNAStatsCMACICVErrors */
 		} aes_cmac;
-- 
2.7.4

^ permalink raw reply related

* [PATCH] rtlwifi: rtl8192c-common: Fix "BUG: KASAN:
From: Larry Finger @ 2017-02-05 16:24 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Larry Finger, Dmitry Osipenko

Kernels built with CONFIG_KASAN=y report the following BUG for rtl8192cu
and rtl8192c-common:

==================================================================
BUG: KASAN: slab-out-of-bounds in rtl92c_dm_bt_coexist+0x858/0x1e40
     [rtl8192c_common] at addr ffff8801c90edb08
Read of size 1 by task kworker/0:1/38
page:ffffea0007243800 count:1 mapcount:0 mapping:          (null)
     index:0x0 compound_mapcount: 0
flags: 0x8000000000004000(head)
page dumped because: kasan: bad access detected
CPU: 0 PID: 38 Comm: kworker/0:1 Not tainted 4.9.7-gentoo #3
Hardware name: Gigabyte Technology Co., Ltd. To be filled by
     O.E.M./Z77-DS3H, BIOS F11a 11/13/2013
Workqueue: rtl92c_usb rtl_watchdog_wq_callback [rtlwifi]
  0000000000000000 ffffffff829eea33 ffff8801d7f0fa30 ffff8801c90edb08
  ffffffff824c0f09 ffff8801d4abee80 0000000000000004 0000000000000297
  ffffffffc070b57c ffff8801c7aa7c48 ffff880100000004 ffffffff000003e8
Call Trace:
  [<ffffffff829eea33>] ? dump_stack+0x5c/0x79
  [<ffffffff824c0f09>] ? kasan_report_error+0x4b9/0x4e0
  [<ffffffffc070b57c>] ? _usb_read_sync+0x15c/0x280 [rtl_usb]
  [<ffffffff824c0f75>] ? __asan_report_load1_noabort+0x45/0x50
  [<ffffffffc06d7a88>] ? rtl92c_dm_bt_coexist+0x858/0x1e40 [rtl8192c_common]
  [<ffffffffc06d7a88>] ? rtl92c_dm_bt_coexist+0x858/0x1e40 [rtl8192c_common]
  [<ffffffffc06d0cbe>] ? rtl92c_dm_rf_saving+0x96e/0x1330 [rtl8192c_common]
...

The problem is due to rtl8192ce and rtl8192cu sharing routines, and having
different layouts of struct rtl_pci_priv, which is used by rtl8192ce, and
struct rtl_usb_priv, which is used by rtl8192cu. The problem was resolved
by placing the struct bt_coexist_info at the head of each of those private
areas.

Reported-and-tested-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org> # 4.0+
Cc: Dmitry Osipenko <digetx@gmail.com>
---

Kalle,

This bug has been in the code since kernel 4.0. To my knowledge, it has
never caused a crash, thus I see no particular need to rush the fix to
mainline. Including it in 4.11 should be OK.

I have a better fix in mind that is much more invasive, but that will not
need to be backported to older kernels as this change will fix the bug.
That second fix will be submitted later.

Larry
---
 drivers/net/wireless/realtek/rtlwifi/pci.h | 4 ++--
 drivers/net/wireless/realtek/rtlwifi/usb.h | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.h b/drivers/net/wireless/realtek/rtlwifi/pci.h
index 578b1d9..d9039ea 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.h
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.h
@@ -271,10 +271,10 @@ struct mp_adapter {
 };
 
 struct rtl_pci_priv {
+	struct bt_coexist_info bt_coexist;
+	struct rtl_led_ctl ledctl;
 	struct rtl_pci dev;
 	struct mp_adapter ndis_adapter;
-	struct rtl_led_ctl ledctl;
-	struct bt_coexist_info bt_coexist;
 };
 
 #define rtl_pcipriv(hw)		(((struct rtl_pci_priv *)(rtl_priv(hw))->priv))
diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.h b/drivers/net/wireless/realtek/rtlwifi/usb.h
index a6d43d2..cdb9e06 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.h
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.h
@@ -146,8 +146,9 @@ struct rtl_usb {
 };
 
 struct rtl_usb_priv {
-	struct rtl_usb dev;
+	struct bt_coexist_info bt_coexist;
 	struct rtl_led_ctl ledctl;
+	struct rtl_usb dev;
 };
 
 #define rtl_usbpriv(hw)	 (((struct rtl_usb_priv *)(rtl_priv(hw))->priv))
-- 
2.10.2

^ permalink raw reply related

* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Larry Finger @ 2017-02-05 17:30 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <03153d2a-abfd-78b4-4365-b80ec718e3e1@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On 02/05/2017 05:34 AM, Dmitry Osipenko wrote:
> BTW, I have an issue with the 8192cu: WiFi stops to work after a while (3-15
> minutes) if I enable WMM QoS on the AP. There is nothing suspicious in KMSG,
> connection is up but no packets go in/out. I tried to enable debug messages in
> the driver, so when the WiFi stops to work I see that some "temperature/led"
> notify still going on in the driver, but nothing happens when I try to initiate
> a transfer (say to open a web page) - the log is silent, like the requests are
> getting stuck/dropped somewhere before reaching the driver. Is it a known issue?
> With the QoS disabled everything works hunky-dory, however I get 2x-4x faster
> download speed with QoS enabled (while it works.)
>
> I noticed that rtl92c_init_edca_param() isn't wired in the driver, so I suppose
> the QoS isn't implemented yet, right?
>
> If it is an expected behaviour, I think at least printing a warning message in
> the KMSG like "QoS unimplemented, you may expect problems" should be good enough
> to avoid confusion.

As you have already seen, I decided to defer the more invasive patch. When 
backporting to stable, the smaller the change the better.

I have no knowledge of the internals of the RTL8192CU chip. As a result, the 
kinds of changes I can make are limited. I do know that the chip does implement 
QoS. I also noticed that the set_qos() callback routine was very different in 
rtl8192ce than in rtl8192cu. Attached is an untested patch to make the CU 
routine look like the CE version. Please see if it makes a difference.

Driver rtl8192cu has never been maintained by Realtek, and it will likely be 
removed from the kernel in the next few cycles. As you are running a new kernel, 
I would recommend rtl8xxxu instead. That driver has high reliability, and the 
speed is improving. Your other option would be a driver offered by the vendor of 
your particular device. Realtek used to have these drivers on their web site, 
but they now seem to have been removed. If your vendor does not have a driver, 
http://www.edimax.com/edimax/mw/cufiles/files/download/Driver_Utility/transfer/Wireless/NIC/EW-7811Un/EW-7811Un_Linux_driver_v1.0.0.5.zip 
should work.

Larry




[-- Attachment #2: rtl8192cu_add_qos.patch --]
[-- Type: text/x-patch, Size: 1003 bytes --]

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
index 1b124ea..1fcf570 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
@@ -387,7 +387,23 @@ void rtl92c_set_qos(struct ieee80211_hw *hw, int aci)
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 
 	rtl92c_dm_init_edca_turbo(hw);
-	rtlpriv->cfg->ops->set_hw_reg(hw, HW_VAR_AC_PARAM, (u8 *)&aci);
+	switch (aci) {
+	case AC1_BK:
+		rtl_write_dword(rtlpriv, REG_EDCA_BK_PARAM, 0xa44f);
+		break;
+	case AC0_BE:
+		/* rtl_write_dword(rtlpriv, REG_EDCA_BE_PARAM, u4b_ac_param); */
+		break;
+	case AC2_VI:
+		rtl_write_dword(rtlpriv, REG_EDCA_VI_PARAM, 0x5e4322);
+		break;
+	case AC3_VO:
+		rtl_write_dword(rtlpriv, REG_EDCA_VO_PARAM, 0x2f3222);
+		break;
+	default:
+		WARN_ONCE(true, "rtl8192ce: invalid aci: %d !\n", aci);
+		break;
+	}
 }
 
 void rtl92c_init_driver_info_size(struct ieee80211_hw *hw, u8 size)

^ permalink raw reply related

* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Dmitry Osipenko @ 2017-02-05 18:15 UTC (permalink / raw)
  To: Larry Finger; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <92f6dcb7-bbee-533a-7d49-21670286a3f3@lwfinger.net>

On 05.02.2017 20:30, Larry Finger wrote:
> On 02/05/2017 05:34 AM, Dmitry Osipenko wrote:
>> BTW, I have an issue with the 8192cu: WiFi stops to work after a while (3-15
>> minutes) if I enable WMM QoS on the AP. There is nothing suspicious in KMSG,
>> connection is up but no packets go in/out. I tried to enable debug messages in
>> the driver, so when the WiFi stops to work I see that some "temperature/led"
>> notify still going on in the driver, but nothing happens when I try to initiate
>> a transfer (say to open a web page) - the log is silent, like the requests are
>> getting stuck/dropped somewhere before reaching the driver. Is it a known issue?
>> With the QoS disabled everything works hunky-dory, however I get 2x-4x faster
>> download speed with QoS enabled (while it works.)
>>
>> I noticed that rtl92c_init_edca_param() isn't wired in the driver, so I suppose
>> the QoS isn't implemented yet, right?
>>
>> If it is an expected behaviour, I think at least printing a warning message in
>> the KMSG like "QoS unimplemented, you may expect problems" should be good enough
>> to avoid confusion.
> 
> As you have already seen, I decided to defer the more invasive patch. When
> backporting to stable, the smaller the change the better.

That is a right approach.

> I have no knowledge of the internals of the RTL8192CU chip. As a result, the
> kinds of changes I can make are limited. I do know that the chip does implement
> QoS. I also noticed that the set_qos() callback routine was very different in
> rtl8192ce than in rtl8192cu. Attached is an untested patch to make the CU
> routine look like the CE version. Please see if it makes a difference.

Thank you a lot, unfortunately it doesn't help.

> Driver rtl8192cu has never been maintained by Realtek, and it will likely be
> removed from the kernel in the next few cycles. As you are running a new kernel,
> I would recommend rtl8xxxu instead. That driver has high reliability, and the
> speed is improving. Your other option would be a driver offered by the vendor of
> your particular device. Realtek used to have these drivers on their web site,
> but they now seem to have been removed. If your vendor does not have a driver,
> http://www.edimax.com/edimax/mw/cufiles/files/download/Driver_Utility/transfer/Wireless/NIC/EW-7811Un/EW-7811Un_Linux_driver_v1.0.0.5.zip
> should work.

Oh, I wasn't aware of rtl8xxxu. Thought it is a driver for some other HW. Thanks
a lot for pointing to it, will give it a whirl.

I'm maintaining a personal fork of a downstream driver for a dozen kernel
releases now, need a hostapd sometime. Fortunately, it's not a big burden.

Thank you a lot again, keep up the good work.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH V3 5/9] brcmfmac: pcie: store private pointer to struct brcmf_pub
From: Arend Van Spriel @ 2017-02-05 19:49 UTC (permalink / raw)
  To: Rafał Miłecki, Kalle Valo
  Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
	linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170202213321.11591-5-zajec5@gmail.com>

On 2-2-2017 22:33, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Getting this pointer in PCIe code is not trivial and requires using
> dev_get_drvdata helper which adds extra line of code. Having access to
> this struct is useful for using generic stuff and e.g. improving logging
> messages.

It is actually by design that getting the pointer is not trivial. We
have made an effort to use struct device pointer as handle between
common and bus-specific code. So I have some reservations about the
approach in this patch series. I had a few days off so I want to look at
it in more detail tomorrow and probably give it a spin.

Regards,
Arend

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 6fae4cf3f6ab..8a3c6e2e4b38 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -265,6 +265,7 @@ struct brcmf_pciedev_info {
>  	void (*write_ptr)(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
>  			  u16 value);
>  	struct brcmf_mp_device *settings;
> +	struct brcmf_pub *pub;
>  };
>  
>  struct brcmf_pcie_ringbuf {
> @@ -1564,14 +1565,18 @@ static void brcmf_pcie_release_resource(struct brcmf_pciedev_info *devinfo)
>  
>  static int brcmf_pcie_attach_bus(struct brcmf_pciedev_info *devinfo)
>  {
> +	struct device *dev = &devinfo->pdev->dev;
> +	struct brcmf_bus *bus = dev_get_drvdata(dev);
>  	int ret;
>  
>  	/* Attach to the common driver interface */
> -	ret = brcmf_attach(&devinfo->pdev->dev, devinfo->settings);
> +	ret = brcmf_attach(dev, devinfo->settings);
>  	if (ret) {
>  		brcmf_err("brcmf_attach failed\n");
>  	} else {
> -		ret = brcmf_bus_started(&devinfo->pdev->dev);
> +		devinfo->pub = bus->drvr;
> +
> +		ret = brcmf_bus_started(dev);
>  		if (ret)
>  			brcmf_err("dongle is not responding\n");
>  	}
> 

^ permalink raw reply

* Re: Installing driver for Ubuntu 16.04 Intel(R) Wireless WiFi Link 4965AGN
From: Sedat Dilek @ 2017-02-06  6:01 UTC (permalink / raw)
  To: Leopoldo; +Cc: linux-wireless
In-Reply-To: <ee13dae1-d01f-baff-a4d3-fe3e77fa85ec@leopoldosoria.com>

On Sun, Feb 5, 2017 at 6:31 PM, Leopoldo <info@leopoldosoria.com> wrote:
> Dear Sedat,
>
> Did you have the chance to have a look to this?
>

No.
If it was not clear said: I did not promise to look at it, I just said
sent the logs and people here can look at it.

- Sedat -

> Kind regards.
>
> Leopoldo
>
>
> El 16/01/17 a las 21:33, Leopoldo escribi=C3=B3:
>
> Dear Sedat,
>
> Please, find attached the requested 2 files.
>
> Thanking you invaluable collaboration.
>
> Leopoldo
>
>
> El 02/01/17 a las 20:39, Sedat Dilek escribi=C3=B3:
>
> On Mon, Jan 2, 2017 at 7:11 PM,  <info@leopoldosoria.com> wrote:
>
> Dear Madam Sedat,
>
> Hehe, my family name is indeed a female first name - Sedat is male.
>
> Thank you for your answer.
>
> I already consulted Ubuntu Community but it is not so nice for a beginner
> like me. I tried several recomedations from them, but no way.  So, I
> thought, it is better to ask to the source.
>
> Yes, I have the firmware you mention, but wifi doesn't work. The icon app=
ear
> in the top bar but it doesn't seem to be conected. It doesn't seach
> conections even when wifi is activated. The ligh of wifi simbol in the
> laptop lights some times but others it doesn't.
>
> It's difficult to answer without knowing anything about your hardware,
> your WLAN infrastructure and your WLAN setup on Ubuntu/xenial.
>
> It might be helpful if you attach your linux-config and dmesg output.
>
> $ cp -v /boot/config-$(uname -r) /tmp
> $ dmesg > /tmp/dmesg.txt
>
> Attach those two files from /tmp directory and people can look at this.
>
> - Sedat -
>
> Any help is welcome and I won't forget ladies next time, sorry.
>
> Thank you for your help and have a happy new year 2017 also.
>
> Regards,
> Leopoldo
>
>
>
>
> El 02-01-2017 15:06, Sedat Dilek escribi=C3=B3:
>
> On Mon, Jan 2, 2017 at 2:11 PM,  <info@leopoldosoria.com> wrote:
>
> Dear Sirs,
> Which of the below drivers should I install for my Lenovo 3000 V200?
> I am runing Ubuntu 16.04
>
> Intel=C2=AE Wireless WiFi Link 4965AGN
> 2.6.24+ iwlwifi-4965-ucode-4.44.14.tgz
> 2.6.24+ iwlwifi-4965-ucode-4.44.15.tgz
> 2.6.24+ iwlwifi-4965-ucode-4.44.17.tgz
> 2.6.24+ iwlwifi-4965-ucode-4.44.1.18.tgz
> 2.6.24+ iwlwifi-4965-ucode-4.44.1.20.tgz
> 2.6.24+ iwlwifi-4965-ucode-228.57.1.21.tgz
> 2.6.28+ (?)     iwlwifi-4965-ucode-228.57.2.21.tgz
> 2.6.28+ (?)     iwlwifi-4965-ucode-228.57.2.23.tgz
> 2.6.28+ (?)     iwlwifi-4965-ucode-228.61.2.24.tgz
>
> Ans how shoudl I proceed for intalling the right driver?
>
> Thank you
>
> [ What about the Ladies, Sir :-)? ]
>
> Happy new 2017!
>
> You need the linux-firmware [1] package.
> Normally, Ubuntu/xenial should ship it.
>
> Check if you have...
>
> /lib/firmware/iwlwifi-4965-2.ucode
>
> ...file.
>
> Not sure why you are asking or what is not working.
> AFAICS, Ubuntu should have some nice wikis explaining a WLAN setup.
> A good strategy will always be to consult your distro's help before askin=
g
> here.
>
> [3] as a gift.
>
> Regards,
> - Sedat -
>
> [1] http://packages.ubuntu.com/xenial/linux-firmware
> [2] http://packages.ubuntu.com/xenial/all/linux-firmware/filelist
> [3] http://www.catb.org/~esr/faqs/smart-questions.html
>
>
>

^ permalink raw reply

* Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails
From: Mohammed Shafi Shajakhan @ 2017-02-06  6:02 UTC (permalink / raw)
  To: Valo, Kalle
  Cc: Shajakhan, Mohammed Shafi (Mohammed Shafi),
	ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <871svr8d83.fsf@kamboji.qca.qualcomm.com>

Hi Kalle,

sorry for the delay

On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote:
> Kalle Valo <kvalo@qca.qualcomm.com> writes:
> 
> > Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes:
> >
> >> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >>
> >> This fixes the below crash when ath10k probe firmware fails,
> >> NAPI polling tries to access a rx ring resource which was never
> >> allocated, fix this by disabling NAPI right away once the probe
> >> firmware fails by calling 'ath10k_hif_stop'. Its good to note
> >> that the error is never propogated to 'ath10k_pci_probe' when
> >> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
> >> PCI related things seems to be ok
> >>
> >> BUG: unable to handle kernel NULL pointer dereference at (null)
> >> IP:  __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
> >> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
> >>
> >> Call Trace:
> >>
> >> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
> >> [ath10k_core]
> >> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0
> >> [ath10k_core]
> >> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80
> >> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150
> >> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
> >> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
> >> [<ffffffff817863af>] net_rx_action+0x20f/0x370
> >>
> >> Reported-by: Ben Greear <greearb@candelatech.com>
> >> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
> >> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >
> > Is there an easy way to reproduce this bug? I don't see it on my x86
> > laptop with qca988x and I call rmmod all the time. I would like to test
> > this myself.
> >
> >> --- a/drivers/net/wireless/ath/ath10k/core.c
> >> +++ b/drivers/net/wireless/ath/ath10k/core.c
> >> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
> >>  	ath10k_core_free_firmware_files(ar);
> >>  
> >>  err_power_down:
> >> +	ath10k_hif_stop(ar);
> >>  	ath10k_hif_power_down(ar);
> >>  
> >>  	return ret;
> >
> > This breaks the symmetry, we should not be calling ath10k_hif_stop() if
> > we haven't called ath10k_hif_start() from the same function. This can
> > just create a bigger mess later, for example with other bus support like
> > sdio or usb. In theory it should enough that we call
> > ath10k_hif_power_down() and pci.c does the rest correctly "behind the
> > scenes".
> >
> > I investigated this a bit and I think the real cause is that we call
> > napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from
> > ath10k_pci_hif_stop(). Does anyone remember why?
> >
> > I was expecting that we would call napi_enable()/napi_disable() either
> > in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not
> > mixed like it's currently.
> 
> So below is something I was thinking of, now napi_enable() is called
> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would
> that work?
> 
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
>  
>  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
>  
> +	napi_enable(&ar->napi);
> +
>  	ath10k_pci_irq_enable(ar);
>  	ath10k_pci_rx_post(ar);
>  
> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
>  		ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
>  		goto err_ce;
>  	}
> -	napi_enable(&ar->napi);
>  
>  	return 0;
>

[shafi] I think I tried this change some time back, but it had some regression
during device start up, let me check this once and get back to you.

regards,
shafi

^ permalink raw reply

* Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD
From: Johannes Berg @ 2017-02-06  6:54 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linux-wireless, Ard Biesheuvel, Herbert Xu
In-Reply-To: <1486224522-26392-1-git-send-email-jouni@qca.qualcomm.com>

Hi,

> The skcipher could have been of the async variant which may return
> from skcipher_encrypt() with -EINPROGRESS after having queued the
> request.
> The FILS AEAD implementation here does not have code for dealing with
> that possibility, so allocate a sync cipher explicitly to avoid
> potential issues with hardware accelerators.

> -	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
> +	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0,
> CRYPTO_ALG_ASYNC);

I'll apply this, after having found some code elsewhere that does
something similar, but I'll note that this is super confusing, since
the only documentation mentioning this flag says:

The mask flag restricts the type of cipher. The only allowed flag is
CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
asynchronous ciphers. Usually, a caller provides a 0 for the mask flag.

(I have a vague feeling the first sentence was intended to be
documentation for the algorithm *implementation* specifying the flag,
and the second for a caller doing a lookup, or something strange?)

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: Fix FILS AEAD protection in Association Request frame
From: Johannes Berg @ 2017-02-06  6:55 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linux-wireless, Ard Biesheuvel
In-Reply-To: <1486209562-23415-1-git-send-email-jouni@qca.qualcomm.com>

On Sat, 2017-02-04 at 13:59 +0200, Jouni Malinen wrote:
> Incorrect num_elem parameter value (1 vs. 5) was used in the
> aes_siv_encrypt() call. This resulted in only the first one of the
> five
> AAD vectors to SIV getting included in calculation. This does not
> protect all the contents correctly and would not interoperate with a
> standard compliant implementation.
> 
> Fix this by using the correct number. A matching fix is needed in the
> AP
> side (hostapd) to get FILS authentication working properly.

Applied, thanks.

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: Fix adding of mesh vendor IEs
From: Johannes Berg @ 2017-02-06  6:57 UTC (permalink / raw)
  To: Sven Eckelmann, linux-wireless
  Cc: Thorsten Horstmann, Mathias Kretschmer, Simon Wunderlich
In-Reply-To: <20170203133829.3219-1-sven@narfation.org>

On Fri, 2017-02-03 at 14:38 +0100, Sven Eckelmann wrote:
> From: Thorsten Horstmann <thorsten@defutech.de>
> 
> The function ieee80211_ie_split_vendor doesn't return 0 on errors.
> Instead
> it returns any offset < ielen when WLAN_EID_VENDOR_SPECIFIC is found.
> The
> return value in mesh_add_vendor_ies must therefore be checked against
> ifmsh->ie_len and not 0. Otherwise all ifmsh->ie starting with
> WLAN_EID_VENDOR_SPECIFIC will be rejected.
> 
> Fixes: 082ebb0c258d ("mac80211: fix mesh beacon format")
> Signed-off-by: Thorsten Horstmann <thorsten@defutech.de>
> Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.
> de>
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> [sven@narfation.org: Add commit message]
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

I'm impressed by how many people signed this off without thinking about
a commit message ... :)

Anyway, applied.

johannes

^ permalink raw reply

* Re: [PATCH] nl80211: Fix mesh HT operation check
From: Johannes Berg @ 2017-02-06  7:00 UTC (permalink / raw)
  To: Masashi Honma; +Cc: linux-wireless
In-Reply-To: <1485388573-4264-1-git-send-email-masashi.honma@gmail.com>

On Thu, 2017-01-26 at 08:56 +0900, Masashi Honma wrote:
> commit 9757235f451c27deaa88925399f070ff6fcea832 ('nl80211: correct
> checks for NL80211_MESHCONF_HT_OPMODE value') missed to mask a flag
> when replacing FILL_IN_MESH_PARAM_IF_SET with checking codes. This
> could drop the received HT operation value when called by
> nl80211_update_mesh_config().

Applied, thanks.

johannes

^ permalink raw reply

* pull-request: mac80211 2017-02-06
From: Johannes Berg @ 2017-02-06  8:36 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

I know it's super late, but I was travelling last week and the
whole FILS AEAD thing only played out over the weekend anyway.
Since the FILS code is all new in this cycle, it'd be good to
have the fixes, and the others are a bit older but still would
be good to fix sooner rather than later, I think.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 7892032cfe67f4bde6fc2ee967e45a8fbaf33756:

  ip6_gre: fix ip6gre_err() invalid reads (2017-02-05 17:23:04 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2017-02-06

for you to fetch changes up to fd551bac4795854adaa87bad7e5136083719802b:

  nl80211: Fix mesh HT operation check (2017-02-06 07:59:07 +0100)

----------------------------------------------------------------
A few simple fixes:
 * fix FILS AEAD cipher usage to use the correct AAD vectors
   and to use synchronous algorithms
 * fix using mesh HT operation data from userspace
 * fix adding mesh vendor elements to beacons & plink frames

----------------------------------------------------------------
Jouni Malinen (2):
      mac80211: Fix FILS AEAD protection in Association Request frame
      mac80211: Allocate a sync skcipher explicitly for FILS AEAD

Masashi Honma (1):
      nl80211: Fix mesh HT operation check

Thorsten Horstmann (1):
      mac80211: Fix adding of mesh vendor IEs

 net/mac80211/fils_aead.c | 6 +++---
 net/mac80211/mesh.c      | 2 +-
 net/wireless/nl80211.c   | 1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: [PATCH v2 1/2] mac80211: fils_aead: Use crypto api CMAC shash rather than bare cipher
From: Johannes Berg @ 2017-02-06  8:47 UTC (permalink / raw)
  To: Ard Biesheuvel, jouni, linux-wireless; +Cc: netdev, davem
In-Reply-To: <1486308208-3252-2-git-send-email-ard.biesheuvel@linaro.org>


>  {
>  	u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE];
> +	struct shash_desc *desc;
> +	u8 buf[sizeof(*desc) + crypto_shash_descsize(tfm)]
> CRYPTO_MINALIGN_ATTR;
>  	size_t i;
> -	const u8 *data[2];
> -	size_t data_len[2], data_elems;
> +
> +	desc = (struct shash_desc *)buf;
> +	desc->tfm = tfm;
> 
> +	crypto_shash_digest(desc, (u8[AES_BLOCK_SIZE]){},
> AES_BLOCK_SIZE, d);

That's an interesting expression in there. Can we name it into a real
variable? :)

I'm also slightly worried about stack usage now - do we know none of
this goes into an sg list eventually?

johannes

^ permalink raw reply

* Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD
From: Herbert Xu @ 2017-02-06  9:00 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Jouni Malinen, linux-wireless, Ard Biesheuvel
In-Reply-To: <1486364077.14226.1.camel@sipsolutions.net>

On Mon, Feb 06, 2017 at 07:54:37AM +0100, Johannes Berg wrote:
> Hi,
> 
> > The skcipher could have been of the async variant which may return
> > from skcipher_encrypt() with -EINPROGRESS after having queued the
> > request.
> > The FILS AEAD implementation here does not have code for dealing with
> > that possibility, so allocate a sync cipher explicitly to avoid
> > potential issues with hardware accelerators.
> 
> > -	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
> > +	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0,
> > CRYPTO_ALG_ASYNC);
> 
> I'll apply this, after having found some code elsewhere that does
> something similar, but I'll note that this is super confusing, since
> the only documentation mentioning this flag says:
> 
> The mask flag restricts the type of cipher. The only allowed flag is
> CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
> asynchronous ciphers. Usually, a caller provides a 0 for the mask flag.

The type and mask are used as follows when checking an algorithm:

	alg->type & mask == type & mask

So to request a synchronous algorithm (that is, one with the
CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and
mask to CRYPTO_ALG_ASYNC.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD
From: Johannes Berg @ 2017-02-06  9:04 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Jouni Malinen, linux-wireless, Ard Biesheuvel
In-Reply-To: <20170206090039.GA9467@gondor.apana.org.au>


> The type and mask are used as follows when checking an algorithm:
> 
> 	alg->type & mask == type & mask
> 
> So to request a synchronous algorithm (that is, one with the
> CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and
> mask to CRYPTO_ALG_ASYNC.

Ah. Ok, that makes sense, thanks for the explanation.

johannes

^ permalink raw reply

* Re: [PATCH v2 1/2] mac80211: fils_aead: Use crypto api CMAC shash rather than bare cipher
From: Ard Biesheuvel @ 2017-02-06  9:08 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Jouni Malinen, <linux-wireless@vger.kernel.org>,
	<netdev@vger.kernel.org>, David S. Miller
In-Reply-To: <1486370872.14226.6.camel@sipsolutions.net>

On 6 February 2017 at 08:47, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>>  {
>>       u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE];
>> +     struct shash_desc *desc;
>> +     u8 buf[sizeof(*desc) + crypto_shash_descsize(tfm)]
>> CRYPTO_MINALIGN_ATTR;

I realised we have a more idiomatic SHASH_DESC_ON_STACK for this.

>>       size_t i;
>> -     const u8 *data[2];
>> -     size_t data_len[2], data_elems;
>> +
>> +     desc = (struct shash_desc *)buf;
>> +     desc->tfm = tfm;
>>
>> +     crypto_shash_digest(desc, (u8[AES_BLOCK_SIZE]){},
>> AES_BLOCK_SIZE, d);
>
> That's an interesting expression in there. Can we name it into a real
> variable? :)
>

Sure, if you prefer.

> I'm also slightly worried about stack usage now - do we know none of
> this goes into an sg list eventually?
>

Shashes do not usually use scatterlists: the shash API does not use
them, but uses u8[] arrays and lengths everywhere, and shashes are
explicitly synchronous, which means they are unsuitable for being
exposed on top of a high latency peripheral that uses DMA.

^ permalink raw reply

* Re: [PATCH V2] mtd: bcm47xxsflash: use platform_(set|get)_drvdata
From: Boris Brezillon @ 2017-02-06  9:46 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	Cyrille Pitchen, Kalle Valo, linux-mtd, linux-wireless,
	Hauke Mehrtens, Rafał Miłecki
In-Reply-To: <20170116162818.12094-1-zajec5@gmail.com>

On Mon, 16 Jan 2017 17:28:18 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> We have generic place & helpers for storing platform driver data so
> there is no reason for using custom priv pointer.
> 
> This allows cleaning up struct bcma_sflash from unneeded fields.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
> Kalle: This is mtd focused patch, so I guess it should go through mtd tree. Do
>        you find bcma change important enough to care to Ack it? :)
> ---
>  drivers/mtd/devices/bcm47xxsflash.c         | 6 +++---
>  include/linux/bcma/bcma_driver_chipcommon.h | 3 ---
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
> index 514be04..4decd8c 100644
> --- a/drivers/mtd/devices/bcm47xxsflash.c
> +++ b/drivers/mtd/devices/bcm47xxsflash.c
> @@ -284,7 +284,6 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev)
>  	b47s = devm_kzalloc(dev, sizeof(*b47s), GFP_KERNEL);
>  	if (!b47s)
>  		return -ENOMEM;
> -	sflash->priv = b47s;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
> @@ -334,6 +333,8 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev)
>  	b47s->size = sflash->size;
>  	bcm47xxsflash_fill_mtd(b47s, &pdev->dev);
>  
> +	platform_set_drvdata(pdev, b47s);
> +
>  	err = mtd_device_parse_register(&b47s->mtd, probes, NULL, NULL, 0);
>  	if (err) {
>  		pr_err("Failed to register MTD device: %d\n", err);
> @@ -349,8 +350,7 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev)
>  
>  static int bcm47xxsflash_bcma_remove(struct platform_device *pdev)
>  {
> -	struct bcma_sflash *sflash = dev_get_platdata(&pdev->dev);
> -	struct bcm47xxsflash *b47s = sflash->priv;
> +	struct bcm47xxsflash *b47s = platform_get_drvdata(pdev);
>  
>  	mtd_device_unregister(&b47s->mtd);
>  	iounmap(b47s->window);
> diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
> index b20e3d5..2f1c690 100644
> --- a/include/linux/bcma/bcma_driver_chipcommon.h
> +++ b/include/linux/bcma/bcma_driver_chipcommon.h
> @@ -593,9 +593,6 @@ struct bcma_sflash {
>  	u32 blocksize;
>  	u16 numblocks;
>  	u32 size;
> -
> -	struct mtd_info *mtd;
> -	void *priv;
>  };
>  #endif
>  

^ permalink raw reply

* Re: [PATCH v2 0/2] mac80211: use crypto shash for AES cmac
From: Malinen, Jouni @ 2017-02-06 10:01 UTC (permalink / raw)
  To: ard.biesheuvel@linaro.org
  Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1486308208-3252-1-git-send-email-ard.biesheuvel@linaro.org>

On Sun, Feb 05, 2017 at 03:23:26PM +0000, Ard Biesheuvel wrote:
> NOTE: Jouni has been so kind to test patch #2, and confirmed that it is w=
orking.
>       I have not tested patch #1 myself, mainly because the test methodol=
ogy
>       requires downloading Ubuntu installer images, and I am currently on=
 a
>       metered 3G connection (and will be for another couple of weeks)

These v2 patches pass the test cases as well.

(And you don't really need Ubuntu to run the hwsim test cases; any
reasonably recent distribution that is capable of running kvm should
work.)
=20
--=20
Jouni Malinen                                            PGP id EFC895FA=

^ permalink raw reply

* Re: [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails
From: Mohammed Shafi Shajakhan @ 2017-02-06 10:04 UTC (permalink / raw)
  To: Valo, Kalle
  Cc: Shajakhan, Mohammed Shafi (Mohammed Shafi),
	ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
In-Reply-To: <871svr8d83.fsf@kamboji.qca.qualcomm.com>

Hi Kalle,

the change suggested by you helps, and the device probe, scan
is successful as well. Still good to have this change part of your
basic sanity and regression testing !

regards,
shafi

On Wed, Jan 25, 2017 at 01:46:28PM +0000, Valo, Kalle wrote:
> Kalle Valo <kvalo@qca.qualcomm.com> writes:
> 
> > Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes:
> >
> >> From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >>
> >> This fixes the below crash when ath10k probe firmware fails,
> >> NAPI polling tries to access a rx ring resource which was never
> >> allocated, fix this by disabling NAPI right away once the probe
> >> firmware fails by calling 'ath10k_hif_stop'. Its good to note
> >> that the error is never propogated to 'ath10k_pci_probe' when
> >> ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
> >> PCI related things seems to be ok
> >>
> >> BUG: unable to handle kernel NULL pointer dereference at (null)
> >> IP:  __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
> >> __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
> >>
> >> Call Trace:
> >>
> >> [<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
> >> [ath10k_core]
> >> [<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0
> >> [ath10k_core]
> >> [<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80
> >> [<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150
> >> [<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
> >> [<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
> >> [<ffffffff817863af>] net_rx_action+0x20f/0x370
> >>
> >> Reported-by: Ben Greear <greearb@candelatech.com>
> >> Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
> >> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
> >
> > Is there an easy way to reproduce this bug? I don't see it on my x86
> > laptop with qca988x and I call rmmod all the time. I would like to test
> > this myself.
> >
> >> --- a/drivers/net/wireless/ath/ath10k/core.c
> >> +++ b/drivers/net/wireless/ath/ath10k/core.c
> >> @@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
> >>  	ath10k_core_free_firmware_files(ar);
> >>  
> >>  err_power_down:
> >> +	ath10k_hif_stop(ar);
> >>  	ath10k_hif_power_down(ar);
> >>  
> >>  	return ret;
> >
> > This breaks the symmetry, we should not be calling ath10k_hif_stop() if
> > we haven't called ath10k_hif_start() from the same function. This can
> > just create a bigger mess later, for example with other bus support like
> > sdio or usb. In theory it should enough that we call
> > ath10k_hif_power_down() and pci.c does the rest correctly "behind the
> > scenes".
> >
> > I investigated this a bit and I think the real cause is that we call
> > napi_enable() from ath10k_pci_hif_power_up() and napi_disable() from
> > ath10k_pci_hif_stop(). Does anyone remember why?
> >
> > I was expecting that we would call napi_enable()/napi_disable() either
> > in ath10k_hif_power_up/down() or ath10k_hif_start()/stop(), but not
> > mixed like it's currently.
> 
> So below is something I was thinking of, now napi_enable() is called
> from ath10k_hif_start() and napi_disable() from ath10k_hif_stop(). Would
> that work?
> 
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -1648,6 +1648,8 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
>  
>  	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
>  
> +	napi_enable(&ar->napi);
> +
>  	ath10k_pci_irq_enable(ar);
>  	ath10k_pci_rx_post(ar);
>  
> @@ -2532,7 +2534,6 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
>  		ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
>  		goto err_ce;
>  	}
> -	napi_enable(&ar->napi);
>  
>  	return 0;
> 
> -- 
> Kalle Valo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox