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.
############################################
# 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
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>
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
Source and destination have the same little-endian annotation: this
patch removes forced conversion from host byte order to little-endian.
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
You're CC'ing all the lustre people on this by mistake.
Can we find which patch introduced this bug, and add a Fixes: tag and
CC whoever introduced it?
Please, resend with the correct CC list.
regards,
dan carpenter
On Sat, Jun 06, 2015 at 05:34:00PM -0700, David Decotigny wrote:
> Source and destination have the same little-endian annotation: this
> patch removes forced conversion from host byte order to little-endian.
The patch seems correct but the changelog is a bit wrong. It will
change it from little endian to host endian but we don't want the dest
to be host endian, we want it to be little endain.
regards,
dan carpenter
This 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. Not sure where to find the
parent repos.
PS: sorry for the incorrect To/Cc, going to resend to more appropriate
recipients.
On Sun, Jun 7, 2015 at 4:20 AM, Dan Carpenter <[email protected]> wrote:
> You're CC'ing all the lustre people on this by mistake.
>
> Can we find which patch introduced this bug, and add a Fixes: tag and
> CC whoever introduced it?
>
> Please, resend with the correct CC list.
>
> regards,
> dan carpenter
>