From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.
Until now, the BlockAck management frames were completely unprotected
whatever the status of the MFP negotiation. So, some devices dropped
these frames.
The device has two knobs to control the MFP. One global and one per
station. Normally, the driver should always enable global MFP. Then it
should enable MFP on every station with which MFP was successfully
negotiated. Unfortunately, the older firmwares only provide the
global control.
So, this patch enable global MFP as it is exposed in the beacon. Then it
marks every station with which the MFP is effective.
Thus, the support for the old firmwares is not so bad. It may only
encounter some difficulties to negotiate BA sessions when the local
device (the AP) is MFP capable (ieee80211w=1) but the station is not.
The only solution for this case is to upgrade the firmware.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/sta.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
@@ -674,15 +674,16 @@ int wfx_ampdu_action(struct ieee80211_hw *hw,structieee80211_vif*vif,structieee80211_ampdu_params*params){-/* Aggregation is implemented fully in firmware,-*includingblockacknegotiation.Donotallow-*mac80211stacktodoanything:itinterfereswith-*thefirmware.-*/--/* Note that we still need this function stubbed. */--return-ENOTSUPP;+// Aggregation is implemented fully in firmware+switch(params->action){+caseIEEE80211_AMPDU_RX_START:+caseIEEE80211_AMPDU_RX_STOP:+// Just acknowledge it to enable frame re-ordering+return0;+default:+// Leave the firmware doing its business for tx aggregation+return-ENOTSUPP;+}}intwfx_add_chanctx(structieee80211_hw*hw,
@@ -753,17 +753,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)return-EOPNOTSUPP;}-for(i=0;i<ARRAY_SIZE(wdev->vif);i++){-if(!wdev->vif[i]){-wdev->vif[i]=vif;-wvif->id=i;-break;-}-}-if(i==ARRAY_SIZE(wdev->vif)){-mutex_unlock(&wdev->conf_mutex);-return-EOPNOTSUPP;-}// FIXME: prefer use of container_of() to get vifwvif->vif=vif;wvif->wdev=wdev;
@@ -780,12 +769,22 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)init_completion(&wvif->scan_complete);INIT_WORK(&wvif->scan_work,wfx_hw_scan_work);-mutex_unlock(&wdev->conf_mutex);--hif_set_macaddr(wvif,vif->addr);-wfx_tx_queues_init(wvif);wfx_tx_policy_init(wvif);++for(i=0;i<ARRAY_SIZE(wdev->vif);i++){+if(!wdev->vif[i]){+wdev->vif[i]=vif;+wvif->id=i;+break;+}+}+WARN(i==ARRAY_SIZE(wdev->vif),"try to instantiate more vif than supported");++hif_set_macaddr(wvif,vif->addr);++mutex_unlock(&wdev->conf_mutex);+wvif=NULL;while((wvif=wvif_iterate(wdev,wvif))!=NULL){// Combo mode does not support Block Acks. We can re-enable them
@@ -817,6 +816,7 @@ void wfx_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)wvif->vif=NULL;mutex_unlock(&wdev->conf_mutex);+wvif=NULL;while((wvif=wvif_iterate(wdev,wvif))!=NULL){// Combo mode does not support Block Acks. We can re-enable them
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The initial developer has feared msecs_to_jiffies() could round down the
result. However, the documentation of msecs_to_jiffies() says that the
result is rounded upward. So the increment of the result of
msecs_to_jiffies() is not necessary.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/bh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -33,7 +33,7 @@ static void device_wakeup(struct wfx_dev *wdev)// wait_for_completion_done_timeout()). So we have to emulate// it.if(wait_for_completion_timeout(&wdev->hif.ctrl_ready,-msecs_to_jiffies(2)+1))+msecs_to_jiffies(2)))complete(&wdev->hif.ctrl_ready);elsedev_err(wdev->dev,"timeout while wake up chip\n");
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The host and the device can be connected with a called Wake-Up GPIO.
When the host fall down this GPIO, it allows the device to enter in deep
sleep and no communication with the device is no more possible (the
device wakes up automatically on DTIM and fetch data if necessary).
So, before to communicate with the device, the driver have to raise the
Wake-up GPIO and then wait for an IRQ from the device.
Unfortunately, old firmwares have a race in sleep/wake-up process and
the device may never wake up. In this case, the IRQ is not sent and
driver complains with "timeout while wake up chip". Then, the driver
tries anyway to access the bus and an other error is raised by the bus.
Fortunately, when the bug occurs, it is possible to fall down the IRQ
and the device will eventually finish the sleep process. Then the driver
can wake it up normally.
The patch implements that workaround and add a retry limit in case
something goes very wrong.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/bh.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
@@ -18,25 +18,40 @@staticvoiddevice_wakeup(structwfx_dev*wdev){+intmax_retry=3;+if(!wdev->pdata.gpio_wakeup)return;if(gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup))return;-gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup,1);if(wfx_api_older_than(wdev,1,4)){+gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup,1);if(!completion_done(&wdev->hif.ctrl_ready))usleep_range(2000,2500);-}else{+return;+}+for(;;){+gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup,1);// completion.h does not provide any function to wait// completion without consume it (a kind of// wait_for_completion_done_timeout()). So we have to emulate// it.if(wait_for_completion_timeout(&wdev->hif.ctrl_ready,-msecs_to_jiffies(2)))+msecs_to_jiffies(2))){complete(&wdev->hif.ctrl_ready);-else+return;+}elseif(max_retry-->0){+// Older firmwares have a race in sleep/wake-up process.+// Redo the process is sufficient to unfreeze the+// chip.dev_err(wdev->dev,"timeout while wake up chip\n");+gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup,0);+usleep_range(2000,2500);+}else{+dev_err(wdev->dev,"max wake-up retries reached\n");+return;+}}}
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
In the old days, ieee80211 powersave has some impact on the Rx speed.
These problems are solved for a long time now. There is no more reason
to not enabling it.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/main.c | 1 -
1 file changed, 1 deletion(-)
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
When MFP is enabled, the multicast management frames are not protected,
in fact. Instead, but they should include an IE containing the MMIC of
the frames (i.e. a cryptographic signature).
Until now, the driver didn't correctly detect this kind of frames (they
are not marked protected but they are associated to a key) and didn't
ask to the device to encrypt them.
In add, the device is not able to generate the IE itself. Mac80211 has
to generate the IE and let the device compute the MMIC.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/data_tx.c | 5 +++--
drivers/staging/wfx/key.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -325,6 +325,8 @@ static int wfx_tx_get_icv_len(struct ieee80211_key_conf *hw_key)if(!hw_key)return0;+if(hw_key->cipher==WLAN_CIPHER_SUITE_AES_CMAC)+return0;mic_space=(hw_key->cipher==WLAN_CIPHER_SUITE_TKIP)?8:0;returnhw_key->icv_len+mic_space;}
@@ -350,8 +352,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,memset(tx_info->rate_driver_data,0,sizeof(structwfx_tx_priv));// Fill tx_privtx_priv=(structwfx_tx_priv*)tx_info->rate_driver_data;-if(ieee80211_has_protected(hdr->frame_control))-tx_priv->hw_key=hw_key;+tx_priv->hw_key=hw_key;// Fill hif_msgWARN(skb_headroom(skb)<wmsg_len,"not enough space in skb");
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The device is able to scan while running an Access Point. Just declare
it.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/main.c | 1 +
drivers/staging/wfx/scan.c | 4 ----
2 files changed, 1 insertion(+), 4 deletions(-)
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
A binary operator should be followed by exactly one space.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/key.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Until now, hif_map_link() get as argument the raw value for
map_link_flags when map_link_flags is defined as a bitfield. It was
error prone.
Now hif_map_link() takes explicit value for every flags of the
struct map_link_flags.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/hif_tx.c | 5 +++--
drivers/staging/wfx/hif_tx.h | 3 ++-
drivers/staging/wfx/sta.c | 4 ++--
3 files changed, 7 insertions(+), 5 deletions(-)
@@ -499,7 +499,7 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable)returnret;}-inthif_map_link(structwfx_vif*wvif,u8*mac_addr,intflags,intsta_id)+inthif_map_link(structwfx_vif*wvif,boolunmap,u8*mac_addr,intsta_id,boolmfp){intret;structhif_msg*hif;
@@ -509,7 +509,8 @@ int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)return-ENOMEM;if(mac_addr)ether_addr_copy(body->mac_addr,mac_addr);-body->map_link_flags=*(structhif_map_link_flags*)&flags;+body->map_link_flags.mfpc=mfp?1:0;+body->map_link_flags.map_direction=unmap?1:0;body->peer_sta_id=sta_id;wfx_fill_header(hif,wvif->id,HIF_REQ_ID_MAP_LINK,sizeof(*body));ret=wfx_cmd_send(wvif->wdev,hif,NULL,0,false);
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The device need to receive a skb with necessary space for the ICV. So,
the driver adds this space before to send the frame.
Currently, once the frame is sent, the driver restore the original
content of the skb. However, this step is useless. Mac80211 don't do it
when software encryption is enabled.
Once we have removed this step, it appears that it is no more necessary
to keep hw_key in tx_priv. Then, it is possible to simplify a bunch of
code in the Tx path.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/data_tx.c | 16 ++++------------
drivers/staging/wfx/data_tx.h | 3 +--
2 files changed, 5 insertions(+), 14 deletions(-)
@@ -350,14 +349,11 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,// From now tx_info->control is unusablememset(tx_info->rate_driver_data,0,sizeof(structwfx_tx_priv));-// Fill tx_priv-tx_priv=(structwfx_tx_priv*)tx_info->rate_driver_data;-tx_priv->hw_key=hw_key;// Fill hif_msgWARN(skb_headroom(skb)<wmsg_len,"not enough space in skb");WARN(offset&1,"attempt to transmit an unaligned frame");-skb_put(skb,wfx_tx_get_icv_len(tx_priv->hw_key));+skb_put(skb,wfx_tx_get_icv_len(hw_key));skb_push(skb,wmsg_len);memset(skb->data,0,wmsg_len);hif_msg=(structhif_msg*)skb->data;
@@ -499,18 +494,15 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)arg->packet_id);return;}+tx_info=IEEE80211_SKB_CB(skb);wvif=wdev_to_wvif(wdev,((structhif_msg*)skb->data)->interface);WARN_ON(!wvif);if(!wvif)return;-tx_info=IEEE80211_SKB_CB(skb);-tx_priv=wfx_skb_tx_priv(skb);++// Note that wfx_pending_get_pkt_us_delay() get data from tx_info_trace_tx_stats(arg,skb,wfx_pending_get_pkt_us_delay(wdev,skb));--// You can touch to tx_priv, but don't touch to tx_info->status.wfx_tx_fill_rates(wdev,tx_info,arg);-skb_trim(skb,skb->len-wfx_tx_get_icv_len(tx_priv->hw_key));-// From now, you can touch to tx_info->status, but do not touch to// tx_priv anymore// FIXME: use ieee80211_tx_info_clear_status()
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.
Until now, the driver directly read the RSN IE of the BSS. However, it
didn't work when the BSS was MFP capable (ieee80211w=1) and the local
device has disabled MFP (ieee80211w=0).
This patch read the MFP information directly from the struct
ieee80211_sta. This information take into account the MFP negotiated
during the association. In addition, the code is far simpler.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/sta.c | 34 +++-------------------------------
1 file changed, 3 insertions(+), 31 deletions(-)
@@ -427,6 +396,9 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,sta_priv->vif_id=wvif->id;+if(vif->type==NL80211_IFTYPE_STATION)+hif_set_mfp(wvif,sta->mfp,sta->mfp);+// In station mode, the firmware interprets new link-id as a TDLS peer.if(vif->type==NL80211_IFTYPE_STATION&&!sta->tdls)return0;
From: Dan Carpenter <hidden> Date: 2020-08-24 09:53:15
On Thu, Aug 20, 2020 at 05:58:47PM +0200, Jerome Pouiller wrote:
quoted hunk
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.
Until now, the BlockAck management frames were completely unprotected
whatever the status of the MFP negotiation. So, some devices dropped
these frames.
The device has two knobs to control the MFP. One global and one per
station. Normally, the driver should always enable global MFP. Then it
should enable MFP on every station with which MFP was successfully
negotiated. Unfortunately, the older firmwares only provide the
global control.
So, this patch enable global MFP as it is exposed in the beacon. Then it
marks every station with which the MFP is effective.
Thus, the support for the old firmwares is not so bad. It may only
encounter some difficulties to negotiate BA sessions when the local
device (the AP) is MFP capable (ieee80211w=1) but the station is not.
The only solution for this case is to upgrade the firmware.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/sta.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
On Monday 24 August 2020 11:50:42 CEST Dan Carpenter wrote:
On Thu, Aug 20, 2020 at 05:58:47PM +0200, Jerome Pouiller wrote:
quoted
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.
Until now, the BlockAck management frames were completely unprotected
whatever the status of the MFP negotiation. So, some devices dropped
these frames.
The device has two knobs to control the MFP. One global and one per
station. Normally, the driver should always enable global MFP. Then it
should enable MFP on every station with which MFP was successfully
negotiated. Unfortunately, the older firmwares only provide the
global control.
So, this patch enable global MFP as it is exposed in the beacon. Then it
marks every station with which the MFP is effective.
Thus, the support for the old firmwares is not so bad. It may only
encounter some difficulties to negotiate BA sessions when the local
device (the AP) is MFP capable (ieee80211w=1) but the station is not.
The only solution for this case is to upgrade the firmware.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/sta.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
@@ -474,6 +474,25 @@ static int wfx_upload_ap_templates(struct wfx_vif *wvif)return0;}+staticvoidwfx_set_mfp_ap(structwfx_vif*wvif)+{+structsk_buff*skb=ieee80211_beacon_get(wvif->wdev->hw,wvif->vif);+constintieoffset=offsetof(structieee80211_mgmt,u.beacon.variable);+constu16*ptr=(u16*)cfg80211_find_ie(WLAN_EID_RSN,+skb->data+ieoffset,+skb->len-ieoffset);+constintpairwise_cipher_suite_count_offset=8/sizeof(u16);+constintpairwise_cipher_suite_size=4/sizeof(u16);+constintakm_suite_size=4/sizeof(u16);++if(ptr){+ptr+=pairwise_cipher_suite_count_offset;+ptr+=1+pairwise_cipher_suite_size**ptr;
The value of "*ptr" comes from skb->data. How do we know that it
doesn't point to something beyond the end of the skb->data buffer?
I think the beacon come from hostapd (or any userspace application with
the necessary permissions). Indeed, it could be corrupted.
I have noticed that WLAN_EID_RSN is parsed at multiple places in the
kernel and I haven't seen any particular check :( (and WLAN_EID_RSN is
probably not the only dangerous IE).
Anyway, I am going to add a few checks on values of ptr.