The code shows a couple inconsistencies (described in commit
descriptions) which would not be an issue on little-endian cpus, but
could cause breakage on non-LE cpus. Note: I could not test on real
hardware, these patches created based on sparse reports.
Hostory:
- resending the same patches to correct recipients, only changed
commit descriptions (credits to Dan Carpenter)
############################################
# Patch Set Summary:
David Decotigny (2):
staging: rtl8723au: core: avoid bitwise arithmetic with forced
endianness
staging: rtl8723au: core: remove redundant endianness conversion
drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
--
2.2.0.rc0.207.ga3a616c
David Decotigny <[email protected]> writes:
> The code shows a couple inconsistencies (described in commit
> descriptions) which would not be an issue on little-endian cpus, but
> could cause breakage on non-LE cpus. Note: I could not test on real
> hardware, these patches created based on sparse reports.
>
> Hostory:
> - resending the same patches to correct recipients, only changed
> commit descriptions (credits to Dan Carpenter)
>
> ############################################
> # Patch Set Summary:
>
> David Decotigny (2):
> staging: rtl8723au: core: avoid bitwise arithmetic with forced
> endianness
> staging: rtl8723au: core: remove redundant endianness conversion
>
> drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Looks fine to me, however if you fiddle with this same value twice,
wouldn't it be better to do it in one patch?
Cheers,
Jes
Thanks. These look nice.
regards,
dan carpenter
Source and destination have the same little-endian annotation: this
patch removes incorrect byte-swap on non-LE cpus.
This addresses the following sparse warning:
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3911:56: warning: incorrect type in argument 1 (different base types)
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3911:56: expected unsigned short [unsigned] [usertype] val
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3911:56: got restricted __le16 [usertype] BA_timeout_value
Signed-off-by: David Decotigny <[email protected]>
---
drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
index 7c3b5dd..142f214 100644
--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
@@ -3906,8 +3906,8 @@ void issue_action_BA23a(struct rtw_adapter *padapter,
put_unaligned_le16(BA_para_set,
&mgmt->u.action.u.addba_resp.capab);
- put_unaligned_le16(pmlmeinfo->ADDBA_req.BA_timeout_value,
- &mgmt->u.action.u.addba_resp.timeout);
+ mgmt->u.action.u.addba_resp.timeout
+ = pmlmeinfo->ADDBA_req.BA_timeout_value;
pattrib->pktlen += 8;
break;
--
2.2.0.rc0.207.ga3a616c
This fixes bitwise arithmetic performed on the host on a variable
previously converted to little-endian, and subsequently converted
again to little-endian:
- issue_action_BA23a() called with "status" crafted in host byte order
- "status" converted to LE
- bitwise arithmetic on the (LE) "status", performed with masks and
shifts in host byte order
- result converted to LE (again) and stored in device structure
Sparse warning addressed by this patch:
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3806:16: warning: incorrect type in assignment (different base types)
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3806:16: expected unsigned short [unsigned] status
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:3806:16: got restricted __le16 [usertype] <noident>
Additional notes: initial cpu_to_le16 was introduced by kernel bulk
commit 5e93f3520 "staging: r8723au: Add source files for new driver
- part 1", initially from github according to commit description. On
github, this traces back to another bulk commit: 2896bda04353 "Add
new files in core directory", which is the 1st version of the
driver.
Signed-off-by: David Decotigny <[email protected]>
---
drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
index 196beaf..7c3b5dd 100644
--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
@@ -3803,8 +3803,6 @@ void issue_action_BA23a(struct rtw_adapter *padapter,
pattrib->pktlen = sizeof(struct ieee80211_hdr_3addr) + 1;
- status = cpu_to_le16(status);
-
switch (action) {
case WLAN_ACTION_ADDBA_REQ:
pattrib->pktlen += sizeof(mgmt->u.action.u.addba_req);
--
2.2.0.rc0.207.ga3a616c