* Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac
From: Ard Biesheuvel @ 2017-02-03 21:55 UTC (permalink / raw)
To: Malinen, Jouni
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20170203214707.GA14147@jouni.qca.qualcomm.com>
On 3 February 2017 at 21:47, Malinen, Jouni <jouni@qca.qualcomm.com> wrote:
> On Fri, Feb 03, 2017 at 07:25:53PM +0000, Ard Biesheuvel wrote:
>> 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. This requires a preparatory
>> patch so that we can remove the open coded implementation, which it shares
>> with the fils aead driver. That driver could receive the same treatment, in
>> which case we could replace patch #1 with one that carries it over first.
>>
>> Note that this is an RFC. I have no idea how I would go about testing this
>> code, but I am on a mission to remove as many dependencies on the generic
>> AES cipher as I can.
>
> Neither the BIP nor FILS cases have any real speed requirements taken
> into account how rarely they end up being used in practice (there is
> really no use case for BIP today and FILS is used only once per
> association). That said, there should be no issues with moving these to
> a more generic mechanism assuming one is available now (I don't think
> that was the case when I was working on BIP and I was too lazy to figure
> out how to convert it or the newer FILS implementation)..
>
> mac80211_hwsim show allow some of the testing to be done with wlantest
> confirming the results in user space (*). I think that would cover all
> of BIP (net/mac80211/aes_cmac.c), but not FILS.
OK, that looks like something I could figure out how to use. But are
you saying the CMAC code is never called in practice?
> For FILS, we do not
> currently have a convenient mechanism for running two different
> instances of kernel or even just mac80211 in the setup, so that would
> likely need testing with real WLAN hardware. I don't currently have a
> good setup for testing this (was using Backports-based solution in the
> past instead of full kernel build and Backports is a bit behind the
> current state..), but I guess I'll need to build something functional
> for this eventually.. Once that's in working condition on two devices,
> it would be straightforward to run a test (snapshot of hostap.git build
> to enable FILS functionality and go through one FILS authentication
> round)..
>
> Another alternative would be to extend wlantest to decrypt/validate FIPS
> AEAD use case based on keys exposed from hostapd or wpa_supplicant.
> There has not been sufficient use case for that so far and I have not
> bothered working on it yet.
>
>
> By the way, FILS AEAD uses SIV mode and I'm not sure it is supported in
> the current crypto code, so that would be one additional piece to take
> care of when considering net/mac80211/fils_aead.c conversion.
>
I did spot something peculiar when looking at the code: if I am
reading the following sequence correctly (from
fils_encrypt_assoc_req())
addr[0] = mgmt->sa;
len[0] = ETH_ALEN;
/* The AP's BSSID */
addr[1] = mgmt->da;
len[1] = ETH_ALEN;
/* The STA's nonce */
addr[2] = assoc_data->fils_nonces;
len[2] = FILS_NONCE_LEN;
/* The AP's nonce */
addr[3] = &assoc_data->fils_nonces[FILS_NONCE_LEN];
len[3] = FILS_NONCE_LEN;
/* The (Re)Association Request frame from the Capability Information
* field to the FILS Session element (both inclusive).
*/
addr[4] = capab;
len[4] = encr - capab;
crypt_len = skb->data + skb->len - encr;
skb_put(skb, AES_BLOCK_SIZE);
return aes_siv_encrypt(assoc_data->fils_kek, assoc_data->fils_kek_len,
encr, crypt_len, 1, addr, len, encr);
the addr[]/len[] arrays are populated with 5 (addr, len) pairs, but
only one is actually passed into aes_siv_encrypt()? This is actually
the main reason I stopped looking into whether I could convert it to
CMAC, because I couldn't figure it out.
^ permalink raw reply
* [PATCH resend 1/4] nfc: Add support RC-S380P to port100
From: OGAWA Hirofumi @ 2017-02-04 1:15 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, Linus Torvalds, linux-kernel, linux-nfc
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff -puN drivers/nfc/port100.c~nfc-add-rcs380p drivers/nfc/port100.c
--- linux/drivers/nfc/port100.c~nfc-add-rcs380p 2016-12-18 22:16:53.503673411 +0900
+++ linux-hirofumi/drivers/nfc/port100.c 2016-12-18 22:16:53.504673416 +0900
@@ -21,8 +21,9 @@
#define VERSION "0.1"
-#define SONY_VENDOR_ID 0x054c
-#define RCS380_PRODUCT_ID 0x06c1
+#define SONY_VENDOR_ID 0x054c
+#define RCS380S_PRODUCT_ID 0x06c1
+#define RCS380P_PRODUCT_ID 0x06c3
#define PORT100_PROTOCOLS (NFC_PROTO_JEWEL_MASK | \
NFC_PROTO_MIFARE_MASK | \
@@ -1477,7 +1478,8 @@ static struct nfc_digital_ops port100_di
};
static const struct usb_device_id port100_table[] = {
- { USB_DEVICE(SONY_VENDOR_ID, RCS380_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380S_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380P_PRODUCT_ID), },
{ }
};
MODULE_DEVICE_TABLE(usb, port100_table);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH resend 2/4] nfc: Send same info for both of NFC_CMD_GET_DEVICE and NFC_EVENT_DEVICE_ADDED
From: OGAWA Hirofumi @ 2017-02-04 1:15 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, Linus Torvalds, linux-kernel, linux-nfc
In-Reply-To: <871sveu579.fsf@mail.parknet.co.jp>
Now, NFC_EVENT_DEVICE_ADDED doesn't send NFC_ATTR_RF_MODE. But
NFC_CMD_GET_DEVICE send.
To get NFC_ATTR_RF_MODE, we have to call NFC_CMD_GET_DEVICE just for
NFC_ATTR_RF_MODE when get NFC_EVENT_DEVICE_ADDED.
This fixes those inconsistent.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
net/nfc/netlink.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff -puN net/nfc/netlink.c~nfc-send-same-info net/nfc/netlink.c
--- linux/net/nfc/netlink.c~nfc-send-same-info 2016-12-18 22:16:55.805686290 +0900
+++ linux-hirofumi/net/nfc/netlink.c 2016-12-18 22:16:55.806686296 +0900
@@ -311,6 +311,17 @@ free_msg:
return -EMSGSIZE;
}
+static int nfc_genl_setup_device_added(struct nfc_dev *dev, struct sk_buff *msg)
+{
+ if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
+ nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
+ nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
+ nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
+ nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ return -1;
+ return 0;
+}
+
int nfc_genl_device_added(struct nfc_dev *dev)
{
struct sk_buff *msg;
@@ -325,10 +336,7 @@ int nfc_genl_device_added(struct nfc_dev
if (!hdr)
goto free_msg;
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
@@ -604,11 +612,7 @@ static int nfc_genl_send_device(struct s
if (cb)
genl_dump_check_consistent(cb, hdr, &nfc_genl_family);
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
- nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH resend 3/4] nfc: Fix RC-S380* needs zero-length packet
From: OGAWA Hirofumi @ 2017-02-04 1:16 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, Linus Torvalds, linux-kernel, linux-nfc
In-Reply-To: <87wpd6sqlw.fsf@mail.parknet.co.jp>
If sent packet size is wMaxPacketSize boundary, this device doesn't
answer. To fix this, we have to send zero-length packet in usb spec.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 1 +
1 file changed, 1 insertion(+)
diff -puN drivers/nfc/port100.c~nfc-need-zero-packet drivers/nfc/port100.c
--- linux-ibmpc/drivers/nfc/port100.c~nfc-need-zero-packet 2016-12-31 18:27:14.814753508 +0900
+++ linux-ibmpc-hirofumi/drivers/nfc/port100.c 2016-12-31 18:32:12.928334607 +0900
@@ -1540,6 +1540,7 @@ static int port100_probe(struct usb_inte
usb_fill_bulk_urb(dev->out_urb, dev->udev,
usb_sndbulkpipe(dev->udev, out_endpoint),
NULL, 0, port100_send_complete, dev);
+ dev->out_urb->transfer_flags = URB_ZERO_PACKET;
dev->skb_headroom = PORT100_FRAME_HEADER_LEN +
PORT100_COMM_RF_HEAD_MAX_LEN;
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH resend 4/4] nfc: Fix hangup of RC-S380* in port100_send_ack()
From: OGAWA Hirofumi @ 2017-02-04 1:16 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton, Linus Torvalds, linux-kernel, linux-nfc
In-Reply-To: <87shnusqkz.fsf_-_@mail.parknet.co.jp>
If port100_send_ack() was called twice or more, it has race to hangup.
port100_send_ack() port100_send_ack()
init_completion()
[...]
dev->cmd_cancel = true
/* this removes previous from completion */
init_completion()
[...]
dev->cmd_cancel = true
wait_for_completion()
/* never be waked up */
wait_for_completion()
Like above race, this code is not assuming port100_send_ack() is
called twice or more.
To fix, this checks dev->cmd_cancel to know if prior cancel is
in-flight or not. And never be remove prior task from completion by
using reinit_completion(), so this guarantees to be waked up properly
soon or later.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff -puN drivers/nfc/port100.c~nfc-port100-hang-fix drivers/nfc/port100.c
--- linux-ibmpc/drivers/nfc/port100.c~nfc-port100-hang-fix 2017-01-03 12:32:13.261785257 +0900
+++ linux-ibmpc-hirofumi/drivers/nfc/port100.c 2017-01-03 12:33:51.965364300 +0900
@@ -726,23 +726,33 @@ static int port100_submit_urb_for_ack(st
static int port100_send_ack(struct port100 *dev)
{
- int rc;
+ int rc = 0;
mutex_lock(&dev->out_urb_lock);
- init_completion(&dev->cmd_cancel_done);
-
- usb_kill_urb(dev->out_urb);
+ /*
+ * If prior cancel is in-flight (dev->cmd_cancel == true), we
+ * can skip to send cancel. Then this will wait the prior
+ * cancel, or merged into the next cancel rarely if next
+ * cancel was started before waiting done. In any case, this
+ * will be waked up soon or later.
+ */
+ if (!dev->cmd_cancel) {
+ reinit_completion(&dev->cmd_cancel_done);
- dev->out_urb->transfer_buffer = ack_frame;
- dev->out_urb->transfer_buffer_length = sizeof(ack_frame);
- rc = usb_submit_urb(dev->out_urb, GFP_KERNEL);
+ usb_kill_urb(dev->out_urb);
- /* Set the cmd_cancel flag only if the URB has been successfully
- * submitted. It will be reset by the out URB completion callback
- * port100_send_complete().
- */
- dev->cmd_cancel = !rc;
+ dev->out_urb->transfer_buffer = ack_frame;
+ dev->out_urb->transfer_buffer_length = sizeof(ack_frame);
+ rc = usb_submit_urb(dev->out_urb, GFP_KERNEL);
+
+ /*
+ * Set the cmd_cancel flag only if the URB has been
+ * successfully submitted. It will be reset by the out
+ * URB completion callback port100_send_complete().
+ */
+ dev->cmd_cancel = !rc;
+ }
mutex_unlock(&dev->out_urb_lock);
@@ -929,8 +939,8 @@ static void port100_send_complete(struct
struct port100 *dev = urb->context;
if (dev->cmd_cancel) {
+ complete_all(&dev->cmd_cancel_done);
dev->cmd_cancel = false;
- complete(&dev->cmd_cancel_done);
}
switch (urb->status) {
@@ -1546,6 +1556,7 @@ static int port100_probe(struct usb_inte
PORT100_COMM_RF_HEAD_MAX_LEN;
dev->skb_tailroom = PORT100_FRAME_TAIL_LEN;
+ init_completion(&dev->cmd_cancel_done);
INIT_WORK(&dev->cmd_complete_work, port100_wq_cmd_complete);
/* The first thing to do with the Port-100 is to set the command type
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* Re: [PATCH] wcn36xx: Implement cancel_hw_scan
From: Mart Raudsepp @ 2017-02-04 9:00 UTC (permalink / raw)
To: Bjorn Andersson, Eugene Krasnikov, Kalle Valo
Cc: wcn36xx, linux-wireless, netdev, linux-kernel, linux-arm-msm
In-Reply-To: <20170201152747.838-1-bjorn.andersson@linaro.org>
> In the even that the wcn36xx interface is brought down while a
> hw_scan
> is active we must abort and wait for the ongoing scan to signal
> completion to mac80211.
>
> Reported-by: Mart Raudsepp <leio@gentoo.org>
> Fixes: 886039036c20 ("wcn36xx: Implement firmware assisted scan")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Mart Raudsepp <leio@gentoo.org>
Please s/even/event/ in commit log
> ---
> drivers/net/wireless/ath/wcn36xx/main.c | 25
> ++++++++++++++++++++++++-
> drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
> b/drivers/net/wireless/ath/wcn36xx/main.c
> index a24edf33be93..bb7110f7fc86 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -574,6 +574,7 @@ static void wcn36xx_hw_scan_worker(struct
> work_struct *work)
> struct cfg80211_scan_request *req = wcn->scan_req;
> u8 channels[WCN36XX_HAL_PNO_MAX_NETW_CHANNELS_EX];
> struct cfg80211_scan_info scan_info = {};
> + bool aborted = false;
> int i;
>
> wcn36xx_dbg(WCN36XX_DBG_MAC, "mac80211 scan %d channels
> worker\n", req->n_channels);
> @@ -585,6 +586,13 @@ static void wcn36xx_hw_scan_worker(struct
> work_struct *work)
>
> wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN);
> for (i = 0; i < req->n_channels; i++) {
> + mutex_lock(&wcn->scan_lock);
> + aborted = wcn->scan_aborted;
> + mutex_unlock(&wcn->scan_lock);
> +
> + if (aborted)
> + break;
> +
> wcn->scan_freq = req->channels[i]->center_freq;
> wcn->scan_band = req->channels[i]->band;
>
> @@ -596,7 +604,7 @@ static void wcn36xx_hw_scan_worker(struct
> work_struct *work)
> }
> wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN);
>
> - scan_info.aborted = false;
> + scan_info.aborted = aborted;
> ieee80211_scan_completed(wcn->hw, &scan_info);
>
> mutex_lock(&wcn->scan_lock);
> @@ -615,6 +623,8 @@ static int wcn36xx_hw_scan(struct ieee80211_hw
> *hw,
> mutex_unlock(&wcn->scan_lock);
> return -EBUSY;
> }
> +
> + wcn->scan_aborted = false;
> wcn->scan_req = &hw_req->req;
> mutex_unlock(&wcn->scan_lock);
>
> @@ -623,6 +633,18 @@ static int wcn36xx_hw_scan(struct ieee80211_hw
> *hw,
> return 0;
> }
>
> +static void wcn36xx_cancel_hw_scan(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif)
> +{
> + struct wcn36xx *wcn = hw->priv;
> +
> + mutex_lock(&wcn->scan_lock);
> + wcn->scan_aborted = true;
> + mutex_unlock(&wcn->scan_lock);
> +
> + cancel_work_sync(&wcn->scan_work);
> +}
> +
> static void wcn36xx_update_allowed_rates(struct ieee80211_sta *sta,
> enum nl80211_band band)
> {
> @@ -1034,6 +1056,7 @@ static const struct ieee80211_ops wcn36xx_ops =
> {
> .tx = wcn36xx_tx,
> .set_key = wcn36xx_set_key,
> .hw_scan = wcn36xx_hw_scan,
> + .cancel_hw_scan = wcn36xx_cancel_hw_scan,
> .bss_info_changed = wcn36xx_bss_info_changed,
> .set_rts_threshold = wcn36xx_set_rts_threshold,
> .sta_add = wcn36xx_sta_add,
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index 571e9f76da7e..b52b4da9a967 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -220,6 +220,7 @@ struct wcn36xx {
> int scan_freq;
> int scan_band;
> struct mutex scan_lock;
> + bool scan_aborted;
>
> /* DXE channels */
> struct wcn36xx_dxe_ch dxe_tx_l_ch; /* TX low
> */
^ permalink raw reply
* Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac
From: Malinen, Jouni @ 2017-02-04 11:35 UTC (permalink / raw)
To: ard.biesheuvel@linaro.org
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <CAKv+Gu9zp+vptF2mHP5MW=x_K5m7WQgMxNNSD5hn93aT_-99xA@mail.gmail.com>
On Fri, Feb 03, 2017 at 09:55:36PM +0000, Ard Biesheuvel wrote:
> OK, that looks like something I could figure out how to use. But are
> you saying the CMAC code is never called in practice?
It will get called if there is a frame that were to need to use BIP.
There are some APs that do send broadcast addressed Deauthentication and
Disassociation frames when terminating the network and those would get
here. In theory, someone could come up with a new Action frame use case
as well with a group addressed Robust Action frame, but no such thing is
defined today. Anyway, this will be needed for certification purposes
and to block DoS with broadcast addressed Deauthentication and
Disassociation frames even if there is not really a common use case
using BIP frames today.
> I did spot something peculiar when looking at the code: if I am
> reading the following sequence correctly (from
> fils_encrypt_assoc_req())
> addr[0] =3D mgmt->sa;
...
> addr[4] =3D capab;
> len[4] =3D encr - capab;
>=20
> crypt_len =3D skb->data + skb->len - encr;
> skb_put(skb, AES_BLOCK_SIZE);
> return aes_siv_encrypt(assoc_data->fils_kek, assoc_data->fils_kek_len,
> encr, crypt_len, 1, addr, len, encr);
>=20
> the addr[]/len[] arrays are populated with 5 (addr, len) pairs, but
> only one is actually passed into aes_siv_encrypt()? This is actually
> the main reason I stopped looking into whether I could convert it to
> CMAC, because I couldn't figure it out.
Argh.. Thanks for finding this!
That is indeed supposed to have num_elems =3D=3D 5. This "works" since the
other end of the exchange is actually in user space (hostapd) and a
similar error is there..
This comes from the development history of FILS support.. The draft
standard changed this AES-SIV AAD design from P802.11ai/D7.0 to D8.0
from a single AAD vector with concatenated fields to separate vectors.
Clearly I added the vectors here in addr[] and len[] arrays, but forgot
to update the num_elems parameter in this direction (STA->AP); the other
direction for Association Response frame is actually using correct
number of AAD vectors.
I'll fix this in hostapd and mac80211.
I did not actually remember that the AP side was still in hostapd for
FILS AEAD in case of mac80211, but with that in mind, the answer to your
earlier question about testing these becomes much simpler.. All you need
to do is this with hostap.git hwsim tests:
hostap/tests/hwsim/vm$ ./vm-run.sh ap_cipher_bip fils_sk_erp
Starting test run in a virtual machine
./run-all.sh: passing the following args to run-tests.py: ap_cipher_bip fil=
s_sk_erp=20
START ap_cipher_bip 1/2
PASS ap_cipher_bip 0.703048 2017-02-04 11:21:35.341174
START fils_sk_erp 2/2
PASS fils_sk_erp 0.667927 2017-02-04 11:21:36.029205
passed all 2 test case(s)
ALL-PASSED
ap_cipher_bip uses wlantest to verify the BIP (AES-128-CMAC) protection
in the frames and fils_sk_erp goes through FILS AEAD exchange with one
side of the operation in mac80211 and the other one in hostapd. This
should be able to discover if something is broken in the AES related
changes in crypto.
And this particular example run here was with your two patches applied,
to the proposed net/mac80211/aes_cmac.c change seems to work fine.
--=20
Jouni Malinen PGP id EFC895FA=
^ permalink raw reply
* [PATCH] mac80211: Fix FILS AEAD protection in Association Request frame
From: Jouni Malinen @ 2017-02-04 11:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Ard Biesheuvel, Jouni Malinen
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.
Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
net/mac80211/fils_aead.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
If there is still time, it would be nice to get this included in Linux
4.10 since fils_aead.c is being added there. If not, Cc: stable could be
added.
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index ecfdd97..e795aaa 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -272,7 +272,7 @@ int fils_encrypt_assoc_req(struct sk_buff *skb,
crypt_len = skb->data + skb->len - encr;
skb_put(skb, AES_BLOCK_SIZE);
return aes_siv_encrypt(assoc_data->fils_kek, assoc_data->fils_kek_len,
- encr, crypt_len, 1, addr, len, encr);
+ encr, crypt_len, 5, addr, len, encr);
}
int fils_decrypt_assoc_resp(struct ieee80211_sub_if_data *sdata,
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac
From: Ard Biesheuvel @ 2017-02-04 14:24 UTC (permalink / raw)
To: Malinen, Jouni
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20170204113514.GA4350@jouni.qca.qualcomm.com>
On 4 February 2017 at 11:35, Malinen, Jouni <jouni@qca.qualcomm.com> wrote:
> On Fri, Feb 03, 2017 at 09:55:36PM +0000, Ard Biesheuvel wrote:
>> OK, that looks like something I could figure out how to use. But are
>> you saying the CMAC code is never called in practice?
>
> It will get called if there is a frame that were to need to use BIP.
> There are some APs that do send broadcast addressed Deauthentication and
> Disassociation frames when terminating the network and those would get
> here. In theory, someone could come up with a new Action frame use case
> as well with a group addressed Robust Action frame, but no such thing is
> defined today. Anyway, this will be needed for certification purposes
> and to block DoS with broadcast addressed Deauthentication and
> Disassociation frames even if there is not really a common use case
> using BIP frames today.
>
>> I did spot something peculiar when looking at the code: if I am
>> reading the following sequence correctly (from
>> fils_encrypt_assoc_req())
>
>> addr[0] = mgmt->sa;
> ...
>> addr[4] = capab;
>> len[4] = encr - capab;
>>
>> crypt_len = skb->data + skb->len - encr;
>> skb_put(skb, AES_BLOCK_SIZE);
>> return aes_siv_encrypt(assoc_data->fils_kek, assoc_data->fils_kek_len,
>> encr, crypt_len, 1, addr, len, encr);
>>
>> the addr[]/len[] arrays are populated with 5 (addr, len) pairs, but
>> only one is actually passed into aes_siv_encrypt()? This is actually
>> the main reason I stopped looking into whether I could convert it to
>> CMAC, because I couldn't figure it out.
>
> Argh.. Thanks for finding this!
>
> That is indeed supposed to have num_elems == 5. This "works" since the
> other end of the exchange is actually in user space (hostapd) and a
> similar error is there..
>
Interesting.
> This comes from the development history of FILS support.. The draft
> standard changed this AES-SIV AAD design from P802.11ai/D7.0 to D8.0
> from a single AAD vector with concatenated fields to separate vectors.
> Clearly I added the vectors here in addr[] and len[] arrays, but forgot
> to update the num_elems parameter in this direction (STA->AP); the other
> direction for Association Response frame is actually using correct
> number of AAD vectors.
>
> I'll fix this in hostapd and mac80211.
>
There is another issue I spotted: the skcipher you allocate may be of
the async variant, which may return from skcipher_encrypt() with
-EINPROGRESS as soon as it has queued the request. Since you don't
deal with that result, you should allocate a sync cipher explicitly:
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index ecfdd97758a3..a31be5262283 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -124,7 +124,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
/* CTR */
- tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+ tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(tfm2)) {
kfree(tmp);
return PTR_ERR(tfm2);
@@ -183,7 +183,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
/* CTR */
- tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+ tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(tfm2))
return PTR_ERR(tfm2);
/* K2 for CTR */
> I did not actually remember that the AP side was still in hostapd for
> FILS AEAD in case of mac80211, but with that in mind, the answer to your
> earlier question about testing these becomes much simpler.. All you need
> to do is this with hostap.git hwsim tests:
>
> hostap/tests/hwsim/vm$ ./vm-run.sh ap_cipher_bip fils_sk_erp
> Starting test run in a virtual machine
> ./run-all.sh: passing the following args to run-tests.py: ap_cipher_bip fils_sk_erp
> START ap_cipher_bip 1/2
> PASS ap_cipher_bip 0.703048 2017-02-04 11:21:35.341174
> START fils_sk_erp 2/2
> PASS fils_sk_erp 0.667927 2017-02-04 11:21:36.029205
> passed all 2 test case(s)
> ALL-PASSED
>
>
> ap_cipher_bip uses wlantest to verify the BIP (AES-128-CMAC) protection
> in the frames and fils_sk_erp goes through FILS AEAD exchange with one
> side of the operation in mac80211 and the other one in hostapd. This
> should be able to discover if something is broken in the AES related
> changes in crypto.
>
> And this particular example run here was with your two patches applied,
> to the proposed net/mac80211/aes_cmac.c change seems to work fine.
Thanks for the instructions and thanks for testing. If I manage to run
this locally, I will follow up with an alternative patch #1 that
switches FILS to use cmac(aes) as well (which looks reasonably
feasible now that I understand the code)
Regards,
Ard.
^ permalink raw reply related
* Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac
From: Malinen, Jouni @ 2017-02-04 14:39 UTC (permalink / raw)
To: ard.biesheuvel@linaro.org
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <CAKv+Gu9p_yng=_=sri8VtRpBCpvL-r1iZ=YgEqNhuQf55p1xBA@mail.gmail.com>
On Sat, Feb 04, 2017 at 02:24:36PM +0000, Ard Biesheuvel wrote:
> There is another issue I spotted: the skcipher you allocate may be of
> the async variant, which may return from skcipher_encrypt() with
> -EINPROGRESS as soon as it has queued the request. Since you don't
> deal with that result, you should allocate a sync cipher explicitly:
> diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
> - tfm2 =3D crypto_alloc_skcipher("ctr(aes)", 0, 0);
> + tfm2 =3D crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
> - tfm2 =3D crypto_alloc_skcipher("ctr(aes)", 0, 0);
> + tfm2 =3D crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
Thanks! Can you send this as a full contribution or do you want me to
do that? I did run this through all the FILS test cases with
mac80211_hwsim.
> Thanks for the instructions and thanks for testing. If I manage to run
> this locally, I will follow up with an alternative patch #1 that
> switches FILS to use cmac(aes) as well (which looks reasonably
> feasible now that I understand the code)
If you have any issues in getting the hwsim test setup working, please
let me know. I'm trying to make it easy for anyone to run those tests in
hopes of improving quality of Linux WLAN contributions and what gets
applied into cfg80211 or mac80211 (or hostap.git for that matter). In
particular the latest step-by-step guide I added for the VM version (*)
was hoping to show how that can be done in 15 minutes from scratch..
(*) http://w1.fi/cgit/hostap/plain/tests/hwsim/vm/example-vm-setup.txt
--=20
Jouni Malinen PGP id EFC895FA=
^ permalink raw reply
* Re: [RFC PATCH 0/2] mac80211: use crypto shash for AES cmac
From: Ard Biesheuvel @ 2017-02-04 14:44 UTC (permalink / raw)
To: Malinen, Jouni
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20170204143913.GA24691@jouni.qca.qualcomm.com>
On 4 February 2017 at 14:39, Malinen, Jouni <jouni@qca.qualcomm.com> wrote:
> On Sat, Feb 04, 2017 at 02:24:36PM +0000, Ard Biesheuvel wrote:
>> There is another issue I spotted: the skcipher you allocate may be of
>> the async variant, which may return from skcipher_encrypt() with
>> -EINPROGRESS as soon as it has queued the request. Since you don't
>> deal with that result, you should allocate a sync cipher explicitly:
>
>> diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
>> - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
>> + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
>
>> - tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
>> + tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
>
> Thanks! Can you send this as a full contribution or do you want me to
> do that?
Please go ahead if you don't mind doing it
> I did run this through all the FILS test cases with
> mac80211_hwsim.
>
Well, even async ciphers can act synchronously: the SIMD based async
ciphers will only enqueue the request for deferred processing when
called in interrupt context (on most architectures) but if you happen
to run on a platform that has a h/w accelerator for ctr(aes), you are
quite likely to see failures here.
>> Thanks for the instructions and thanks for testing. If I manage to run
>> this locally, I will follow up with an alternative patch #1 tha here
>> switches FILS to use cmac(aes) as well (which looks reasonably
>> feasible now that I understand the code)
>
> If you have any issues in getting the hwsim test setup working, please
> let me know. I'm trying to make it easy for anyone to run those tests in
> hopes of improving quality of Linux WLAN contributions and what gets
> applied into cfg80211 or mac80211 (or hostap.git for that matter). In
> particular the latest step-by-step guide I added for the VM version (*)
> was hoping to show how that can be done in 15 minutes from scratch..
>
>
> (*) http://w1.fi/cgit/hostap/plain/tests/hwsim/vm/example-vm-setup.txt
>
I will take a look on Monday
^ permalink raw reply
* rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Dmitry Osipenko @ 2017-02-04 15:29 UTC (permalink / raw)
To: Larry Finger, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
Hello,
I ran a 4.9.7 kernel with CONFIG_KASAN=y on my machine and it detected the
following problem:
[ 23.012238] rtl8192cu: MAC auto ON okay!
[ 23.045656] rtl8192cu: Tx queue select: 0x05
[ 23.541152] ==================================================================
[ 23.541160] BUG: KASAN: slab-out-of-bounds in
rtl92c_dm_bt_coexist+0x858/0x1e40 [rtl8192c_common] at addr ffff8801c90edb08
[ 23.541161] Read of size 1 by task kworker/0:1/38
[ 23.541163] page:ffffea0007243800 count:1 mapcount:0 mapping: (null)
index:0x0 compound_mapcount: 0
[ 23.543194] flags: 0x8000000000004000(head)
[ 23.545178] page dumped because: kasan: bad access detected
[ 23.545181] CPU: 0 PID: 38 Comm: kworker/0:1 Not tainted 4.9.7-gentoo #3
[ 23.545182] Hardware name: Gigabyte Technology Co., Ltd. To be filled by
O.E.M./Z77-DS3H, BIOS F11a 11/13/2013
[ 23.545186] Workqueue: rtl92c_usb rtl_watchdog_wq_callback [rtlwifi]
[ 23.545187] 0000000000000000 ffffffff829eea33 ffff8801d7f0fa30 ffff8801c90edb08
[ 23.545189] ffffffff824c0f09 ffff8801d4abee80 0000000000000004 0000000000000297
[ 23.545191] ffffffffc070b57c ffff8801c7aa7c48 ffff880100000004 ffffffff000003e8
[ 23.545192] Call Trace:
[ 23.545197] [<ffffffff829eea33>] ? dump_stack+0x5c/0x79
[ 23.545200] [<ffffffff824c0f09>] ? kasan_report_error+0x4b9/0x4e0
[ 23.545202] [<ffffffffc070b57c>] ? _usb_read_sync+0x15c/0x280 [rtl_usb]
[ 23.545204] [<ffffffff824c0f75>] ? __asan_report_load1_noabort+0x45/0x50
[ 23.545206] [<ffffffffc06d7a88>] ? rtl92c_dm_bt_coexist+0x858/0x1e40
[rtl8192c_common]
[ 23.545208] [<ffffffffc06d7a88>] ? rtl92c_dm_bt_coexist+0x858/0x1e40
[rtl8192c_common]
[ 23.545210] [<ffffffffc06d0cbe>] ? rtl92c_dm_rf_saving+0x96e/0x1330
[rtl8192c_common]
[ 23.545212] [<ffffffffc06dab80>] ? rtl92c_dm_watchdog+0x1130/0x4590
[rtl8192c_common]
[ 23.545214] [<ffffffffc06d9a50>] ? rtl92c_dm_dynamic_txpower+0x9e0/0x9e0
[rtl8192c_common]
[ 23.545216] [<ffffffff8222038d>] ? pick_next_entity+0x18d/0x400
[ 23.545218] [<ffffffff8223fdde>] ? pick_next_task_fair+0xa6e/0xf60
[ 23.545220] [<ffffffff8207548a>] ? __switch_to+0x7ba/0x1160
[ 23.545223] [<ffffffffc0689a2b>] ? rtl_watchdog_wq_callback+0xb7b/0x1190
[rtlwifi]
[ 23.545225] [<ffffffffc0688eb0>] ? rtl_tx_mgmt_proc+0x2c0/0x2c0 [rtlwifi]
[ 23.545228] [<ffffffff82d1ecfe>] ? drm_fb_helper_dirty_work+0x25e/0x2f0
[ 23.545230] [<ffffffff83c65d20>] ? io_schedule_timeout+0x390/0x390
[ 23.545233] [<ffffffff82b8cef0>] ? update_attr.isra.2+0x170/0x170
[ 23.545234] [<ffffffff82b80a01>] ? fb_flashcursor+0x331/0x3e0
[ 23.545237] [<ffffffff821d7af9>] ? process_one_work+0x539/0x12b0
[ 23.545239] [<ffffffff821d894f>] ? worker_thread+0xdf/0x13e0
[ 23.545240] [<ffffffff8225343c>] ? __wake_up_common+0xbc/0x160
[ 23.545242] [<ffffffff821d8870>] ? process_one_work+0x12b0/0x12b0
[ 23.545244] [<ffffffff821e7f89>] ? kthread+0x1b9/0x210
[ 23.545245] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.545247] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.545248] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.545250] [<ffffffff83c724c5>] ? ret_from_fork+0x25/0x30
[ 23.545251] Memory state around the buggy address:
[ 23.545253] ffff8801c90eda00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.545254] ffff8801c90eda80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.545255] >ffff8801c90edb00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.545256] ^
[ 23.545257] ffff8801c90edb80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.545257] ffff8801c90edc00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.545258] ==================================================================
[ 23.545258] Disabling lock debugging due to kernel taint
[ 23.545300] ==================================================================
[ 23.545303] BUG: KASAN: slab-out-of-bounds in
rtl92c_dm_watchdog+0x3dfb/0x4590 [rtl8192c_common] at addr ffff8801c90edb1c
[ 23.545304] Read of size 4 by task kworker/0:1/38
[ 23.545305] page:ffffea0007243800 count:1 mapcount:0 mapping: (null)
index:0x0 compound_mapcount: 0
[ 23.547353] flags: 0x8000000000004000(head)
[ 23.547354] page dumped because: kasan: bad access detected
[ 23.547355] CPU: 0 PID: 38 Comm: kworker/0:1 Tainted: G B
4.9.7-gentoo #3
[ 23.547356] Hardware name: Gigabyte Technology Co., Ltd. To be filled by
O.E.M./Z77-DS3H, BIOS F11a 11/13/2013
[ 23.547358] Workqueue: rtl92c_usb rtl_watchdog_wq_callback [rtlwifi]
[ 23.547360] 0000000000000000 ffffffff829eea33 ffff8801d7f0fa80 ffff8801c90edb1c
[ 23.547361] ffffffff824c0f09 ffff8801c90e0748 ffff8801c90e1420 0000000000000297
[ 23.547363] ffff8801c90e1420 ffffffff824c0f75 ffff8801c90edb08 ffff8801c90edb08
[ 23.547363] Call Trace:
[ 23.547367] [<ffffffff829eea33>] ? dump_stack+0x5c/0x79
[ 23.547368] [<ffffffff824c0f09>] ? kasan_report_error+0x4b9/0x4e0
[ 23.547370] [<ffffffff824c0f75>] ? __asan_report_load1_noabort+0x45/0x50
[ 23.547371] [<ffffffff824c1015>] ? __asan_report_load4_noabort+0x45/0x50
[ 23.547373] [<ffffffffc06dd84b>] ? rtl92c_dm_watchdog+0x3dfb/0x4590
[rtl8192c_common]
[ 23.547375] [<ffffffffc06dd84b>] ? rtl92c_dm_watchdog+0x3dfb/0x4590
[rtl8192c_common]
[ 23.547377] [<ffffffffc06d9a50>] ? rtl92c_dm_dynamic_txpower+0x9e0/0x9e0
[rtl8192c_common]
[ 23.547378] [<ffffffff8222038d>] ? pick_next_entity+0x18d/0x400
[ 23.547380] [<ffffffff8223fdde>] ? pick_next_task_fair+0xa6e/0xf60
[ 23.547381] [<ffffffff8207548a>] ? __switch_to+0x7ba/0x1160
[ 23.547383] [<ffffffffc0689a2b>] ? rtl_watchdog_wq_callback+0xb7b/0x1190
[rtlwifi]
[ 23.547385] [<ffffffffc0688eb0>] ? rtl_tx_mgmt_proc+0x2c0/0x2c0 [rtlwifi]
[ 23.547387] [<ffffffff82d1ecfe>] ? drm_fb_helper_dirty_work+0x25e/0x2f0
[ 23.547388] [<ffffffff83c65d20>] ? io_schedule_timeout+0x390/0x390
[ 23.547390] [<ffffffff82b8cef0>] ? update_attr.isra.2+0x170/0x170
[ 23.547391] [<ffffffff82b80a01>] ? fb_flashcursor+0x331/0x3e0
[ 23.547393] [<ffffffff821d7af9>] ? process_one_work+0x539/0x12b0
[ 23.547394] [<ffffffff821d894f>] ? worker_thread+0xdf/0x13e0
[ 23.547396] [<ffffffff8225343c>] ? __wake_up_common+0xbc/0x160
[ 23.547398] [<ffffffff821d8870>] ? process_one_work+0x12b0/0x12b0
[ 23.547399] [<ffffffff821e7f89>] ? kthread+0x1b9/0x210
[ 23.547400] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.547401] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.547403] [<ffffffff821e7dd0>] ? kthread_park+0x80/0x80
[ 23.547404] [<ffffffff83c724c5>] ? ret_from_fork+0x25/0x30
[ 23.547405] Memory state around the buggy address:
[ 23.547406] ffff8801c90eda00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.547407] ffff8801c90eda80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.547407] >ffff8801c90edb00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.547408] ^
[ 23.547409] ffff8801c90edb80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.547410] ffff8801c90edc00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
[ 23.547410] ==================================================================
--
Dmitry
^ permalink raw reply
* [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD
From: Jouni Malinen @ 2017-02-04 16:08 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Ard Biesheuvel, Jouni Malinen
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.
This is based on the patch sent out by Ard.
Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
net/mac80211/fils_aead.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index e3bbe24..912f3e2c 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -192,7 +192,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
/* CTR */
- tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+ tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(tfm2)) {
kfree(tmp);
return PTR_ERR(tfm2);
@@ -251,7 +251,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
/* CTR */
- tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+ tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(tfm2))
return PTR_ERR(tfm2);
/* K2 for CTR */
--
2.7.4
^ permalink raw reply related
* Re: rtlwifi: rtl8192c_common: "BUG: KASAN: slab-out-of-bounds"
From: Dmitry Osipenko @ 2017-02-04 16:58 UTC (permalink / raw)
To: Larry Finger, Chaoming Li; +Cc: linux-wireless, Linux Kernel Mailing List
In-Reply-To: <beff9033-0051-fd0f-87fe-8269a0d9c8d0@gmail.com>
Seems the problem is caused by rtl92c_dm_*() casting .priv to "struct
rtl_pci_priv", while it is "struct rtl_usb_priv".
--
Dmitry
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox