Skip to contentSkip to conversation

nfc: fix get_unaligned_...() misuses

PATCH In review Al Viro 3 participants 5 messages Apr 6, 2017 – Apr 16, 2017

Subsystems (2)
NFC NXP-nci NFC driver
lore
Share & tools
lore

Al Viro

author
#0 lore raw

PATCH 1/1 nfc: fix get_unaligned_...() misuses

* use unaligned.h, not unaligned/access_ok.h
* if a local variable of type uint16_t is unaligned, your compiler is FUBAR
* the whole point of get_unaligned_... is to avoid memcpy + ..._to_cpu(). Using it *after* memcpy() (into aligned object, no less) is pointless.

Signed-off-by Al Viro <[email protected]>
drivers/nfc/nfcmrvl/fw_dnld.c +3 -4
index f8dcdf4b24f6..d3b04da0f16f 100644
@@ -17,7 +17,7 @@
  */
 
 #include <linux/module.h>
-#include <linux/unaligned/access_ok.h>
+#include <linux/unaligned.h>
 #include <linux/firmware.h>
 #include <linux/nfc.h>
 #include <net/nfc/nci.h>

@@ -281,12 +281,11 @@ static int process_state_fw_dnld(struct nfcmrvl_private *priv,
 			return -EINVAL;
 		}
 		skb_pull(skb, 1);
-		memcpy(&len, skb->data, 2);
+		len = get_unaligned_le16(skb->data);
 		skb_pull(skb, 2);
+		comp_len = get_unaligned_le16(skb->data);
 		memcpy(&comp_len, skb->data, 2);
 		skb_pull(skb, 2);
-		len = get_unaligned_le16(&len);
-		comp_len = get_unaligned_le16(&comp_len);
 		if (((~len) & 0xFFFF) != comp_len) {
 			nfc_err(priv->dev, "bad len complement: %x %x %x",
 				len, comp_len, (~len & 0xFFFF));
drivers/nfc/nxp-nci/firmware.c +1 -1
index 5291797324ba..f27b03e3b5ae 100644
@@ -24,7 +24,7 @@
 #include <linux/completion.h>
 #include <linux/firmware.h>
 #include <linux/nfc.h>
-#include <linux/unaligned/access_ok.h>
+#include <linux/unaligned.h>
 
 #include "nxp-nci.h"
drivers/nfc/nxp-nci/i2c.c +1 -2
index 36099e557730..9da35d2898fc 100644
@@ -36,7 +36,6 @@
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/platform_data/nxp-nci.h>
-#include <linux/unaligned/access_ok.h>
 
 #include <net/nfc/nfc.h>
 

@@ -127,7 +126,7 @@ static int nxp_nci_i2c_fw_read(struct nxp_nci_i2c_phy *phy,
 		goto fw_read_exit;
 	}
 
-	frame_len = (get_unaligned_be16(&header) & NXP_NCI_FW_FRAME_LEN_MASK) +
+	frame_len = (be16_to_cpu(header) & NXP_NCI_FW_FRAME_LEN_MASK) +
 		    NXP_NCI_FW_CRC_LEN;
 
 	*skb = alloc_skb(NXP_NCI_FW_HDR_LEN + frame_len, GFP_KERNEL);
author
#1 lore raw
Quoted 1 line · Al Viro

* use unaligned.h, not unaligned/access_ok.h

... which got misspelled in that patch, sorry... Fixed variant follows:

commit b3e79ba1708c9b74781079c9f8617448fce36b51

Author: Al Viro <[email protected]>
Date:   Thu Apr 6 12:42:14 2017 -0400

nfc: fix get_unaligned_...() misuses

* use unaligned.h, not unaligned/access_ok.h
* if a local variable of type uint16_t is unaligned, your compiler is FUBAR
* the whole point of get_unaligned_... is to avoid memcpy + ..._to_cpu(). Using it *after* memcpy() (into aligned object, no less) is pointless.

Signed-off-by Al Viro <[email protected]>
drivers/nfc/nfcmrvl/fw_dnld.c +3 -4
index f8dcdf4b24f6..ad1f0624ceee 100644
@@ -17,11 +17,11 @@
  */
 
 #include <linux/module.h>
-#include <linux/unaligned/access_ok.h>
 #include <linux/firmware.h>
 #include <linux/nfc.h>
 #include <net/nfc/nci.h>
 #include <net/nfc/nci_core.h>
+#include <asm/unaligned.h>
 #include "nfcmrvl.h"
 
 #define FW_DNLD_TIMEOUT			15000

@@ -281,12 +281,11 @@ static int process_state_fw_dnld(struct nfcmrvl_private *priv,
 			return -EINVAL;
 		}
 		skb_pull(skb, 1);
-		memcpy(&len, skb->data, 2);
+		len = get_unaligned_le16(skb->data);
 		skb_pull(skb, 2);
+		comp_len = get_unaligned_le16(skb->data);
 		memcpy(&comp_len, skb->data, 2);
 		skb_pull(skb, 2);
-		len = get_unaligned_le16(&len);
-		comp_len = get_unaligned_le16(&comp_len);
 		if (((~len) & 0xFFFF) != comp_len) {
 			nfc_err(priv->dev, "bad len complement: %x %x %x",
 				len, comp_len, (~len & 0xFFFF));
drivers/nfc/nxp-nci/firmware.c +1 -1
index 5291797324ba..553011f58339 100644
@@ -24,7 +24,7 @@
 #include <linux/completion.h>
 #include <linux/firmware.h>
 #include <linux/nfc.h>
-#include <linux/unaligned/access_ok.h>
+#include <asm/unaligned.h>
 
 #include "nxp-nci.h"
drivers/nfc/nxp-nci/i2c.c +1 -2
index 36099e557730..9da35d2898fc 100644
@@ -36,7 +36,6 @@
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/platform_data/nxp-nci.h>
-#include <linux/unaligned/access_ok.h>
 
 #include <net/nfc/nfc.h>
 

@@ -127,7 +126,7 @@ static int nxp_nci_i2c_fw_read(struct nxp_nci_i2c_phy *phy,
 		goto fw_read_exit;
 	}
 
-	frame_len = (get_unaligned_be16(&header) & NXP_NCI_FW_FRAME_LEN_MASK) +
+	frame_len = (be16_to_cpu(header) & NXP_NCI_FW_FRAME_LEN_MASK) +
 		    NXP_NCI_FW_CRC_LEN;
 
 	*skb = alloc_skb(NXP_NCI_FW_HDR_LEN + frame_len, GFP_KERNEL);
#2 lore raw
Quoted 9 lines · Al Viro
Quoted 1 line · Al Viro

* use unaligned.h, not unaligned/access_ok.h

... which got misspelled in that patch, sorry... Fixed variant follows:

commit b3e79ba1708c9b74781079c9f8617448fce36b51

Author: Al Viro <[email protected]>
Date:   Thu Apr 6 12:42:14 2017 -0400
Quoted 8 lines

nfc: fix get_unaligned_...() misuses

* use unaligned.h, not unaligned/access_ok.h
* if a local variable of type uint16_t is unaligned, your compiler is FUBAR
* the whole point of get_unaligned_... is to avoid memcpy + ..._to_cpu(). Using it *after* memcpy() (into aligned object, no less) is pointless.

Signed-off-by Al Viro <[email protected]>

Partly applied to nfc-next. The unaligned/access_ok.h replacements were already applied.

Cheers,
Samuel.

build bot
#3 lore raw
In reply to Al Viro
url: https://github.com/0day-ci/linux/commits/Al-Viro/nfc-fix-get_unaligned_-misuses/20170407-123722 config: i386-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901…

Hi Al,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc5 next-20170406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Al-Viro/nfc-fix-get_unaligned_-misuses/20170407-123722
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386

All errors (new ones prefixed by >>):

>> drivers/nfc/nxp-nci/firmware.c:27:29: fatal error: linux/unaligned.h: No such file or directory
    #include <linux/unaligned.h>
                                ^

compilation terminated.

vim +27 drivers/nfc/nxp-nci/firmware.c

    21	 * along with this program; if not, see <http://www.gnu.org/licenses/>.
    22	 */
    23	
    24	#include <linux/completion.h>
    25	#include <linux/firmware.h>
    26	#include <linux/nfc.h>
  > 27	#include <linux/unaligned.h>
    28	
    29	#include "nxp-nci.h"
    30

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#4 lore raw
In reply to Al Viro
url: https://github.com/0day-ci/linux/commits/Al-Viro/nfc-fix-get_unaligned_-misuses/20170407-123722 config: x86_64-randconfig-a0-04071419 (attached as .config) compiler: gcc-4.4 (Debian 4.4.7-8)…

Hi Al,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc5 next-20170406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Al-Viro/nfc-fix-get_unaligned_-misuses/20170407-123722
config: x86_64-randconfig-a0-04071419 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

>> drivers/nfc//nfcmrvl/fw_dnld.c:20:29: error: linux/unaligned.h: No such file or directory
   drivers/nfc//nfcmrvl/fw_dnld.c: In function 'process_state_fw_dnld':
>> drivers/nfc//nfcmrvl/fw_dnld.c:284: error: implicit declaration of function 'get_unaligned_le16'

vim +/get_unaligned_le16 +284 drivers/nfc//nfcmrvl/fw_dnld.c

278 skb_pull(skb, 3);
279 if (skb->data[0] != HELPER_CMD_PACKET_FORMAT || skb->len != 5) {
280 nfc_err(priv->dev, "bad command");
281 return -EINVAL;
282 }
283 skb_pull(skb, 1);

 > 284			len = get_unaligned_le16(skb->data);

285 skb_pull(skb, 2);
286 comp_len = get_unaligned_le16(skb->data);
287 memcpy(&comp_len, skb->data, 2);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation