Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

Adding the network and NFC developers as this really is a NFC driver
bug, not a USB core issue...

On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
> On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> > Hello.
> > I'm having troubles with this NFC card reader. It seems kernel driver
> > pn533 is not working properly.
> > At this moment I have my work stalled. I need to add NFC support to a
> > software product and I can't get the device to work. NFC tools won't
> > work, the device is not detected.
> > 
> > This is what I get from "dmesg":
> > 
> > [    4.182300] nfc: nfc_init: NFC Core ver 0.1
> > [    4.182318] NET: Registered protocol family 39
> > [    4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> > [    4.193366] ------------[ cut here ]------------
> > [    4.193366] transfer buffer not dma capable
> > [    4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> > usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > [    4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> > snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> > snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> > snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> > snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> > intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> > snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> > mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> > ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> > videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> > snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> > intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> > intel_rapl_perf tpm_crb psmouse
> > [    4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> > wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> > sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> > intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> > rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> > ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> > crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> > serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> > libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> > mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> > sysimgblt fb_sys_fops ttm drm agpgart
> > [    4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH #1
> > [    4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> > PRO A50-C, BIOS Version 7.50   09/26/2016
> > [    4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > [    4.193468] RSP: 0018:ffffa3b44282f9f8 EFLAGS: 00010282
> > [    4.193469] RAX: 0000000000000000 RBX: ffff981fc9e320c0 RCX: 0000000000000001
> > [    4.193470] RDX: 0000000080000001 RSI: 0000000000000002 RDI: 00000000ffffffff
> > [    4.193471] RBP: ffff981fd42f0000 R08: 0000000713ed01d2 R09: 000000000000001f
> > [    4.193472] R10: 0000000000000344 R11: 000000000000f300 R12: 00000000014000c0
> > [    4.193473] R13: 00000000fffffff5 R14: ffff981fd2592b98 R15: 00000000c0410280
> > [    4.193475] FS:  00007f4fb98d0d40(0000) GS:ffff981fe6d00000(0000)
> > knlGS:0000000000000000
> > [    4.193476] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [    4.193477] CR2: 0000562b4a68f6e8 CR3: 00000004532d6004 CR4: 00000000003606e0
> > [    4.193478] Call Trace:
> > [    4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> > [    4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> > [    4.193495]  ? __kmalloc+0x19e/0x220
> > [    4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> > [    4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> > [    4.193511]  driver_probe_device+0x2b9/0x460
> > [    4.193514]  ? __driver_attach+0xb6/0xe0
> > [    4.193516]  ? driver_probe_device+0x460/0x460
> > [    4.193518]  ? bus_for_each_dev+0x76/0xc0
> > [    4.193520]  ? bus_add_driver+0x152/0x230
> > [    4.193522]  ? driver_register+0x6b/0xb0
> > [    4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> > [    4.193531]  ? 0xffffffffc13b6000
> > [    4.193534]  ? do_one_initcall+0x48/0x13b
> > [    4.193537]  ? free_unref_page_commit+0x6a/0x100
> > [    4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> > [    4.193542]  ? do_init_module+0x5a/0x210
> > [    4.193544]  ? load_module+0x247a/0x29f0
> > [    4.193549]  ? SyS_init_module+0x139/0x180
> > [    4.193550]  ? SyS_init_module+0x139/0x180
> > [    4.193554]  ? do_syscall_64+0x74/0x190
> > [    4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > [    4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
> > ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
> > 97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
> > 48 8b
> > [    4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
> > [    4.193612] usb 1-1: NFC: Reader power on cmd error -11
> > [    4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
> > [    4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
> > [    4.193637] usbcore: registered new interface driver pn533_usb
> > [    4.198216] usbcore: registered new interface driver usbhid
> > 
> > 
> > Please find the full dmesg in the link below:
> > https://pastebin.com/ck4sZuUY
> 
> Odd that this driver has ever worked.  Has it worked for you on older
> kernels?
> 
> It looks like it is trying to send data off of the stack.  At first
> glance, I see at least two places it is doing this, which is what is
> causing the errors you are seeing.   I'll go audit the whole thing in a
> few hours when I get a chance.
> 
> Are you able to build and test a kernel patch if I make one for you?

Here's a totally untested, and not even built, patch that I knocked up
that should fix this type of issue.

I'll try to at least build it later tonight...

thanks,

greg k-h

--------------

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH] NFC: pn533: don't send USB data off of the stack

It's amazing that this driver ever worked, but now that x86 doesn't
allow USB data to be sent off of the stack, it really does not work at
all.  Fix this up by properly allocating the data for the small
"commands" that get sent to the device.

The USB stack will free the buffer when the data has been transmitted,
that is why there is no kfree() to mirror the call to kmalloc().

Reported-by: Carlos Manuel Santos <cmmpsantos@gmail.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index e153e8b64bb8..15291cc4f4b5 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -150,10 +150,17 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
 	struct pn533_usb_phy *phy = dev->phy;
 	static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
 	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
+	char *buffer;
 	int rc;
 
+	buffer = kmalloc(sizeof(ack), GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+	memcpy(buffer, ack, sizeof(ack));
+
 	phy->out_urb->transfer_buffer = (u8 *)ack;
 	phy->out_urb->transfer_buffer_length = sizeof(ack);
+	phy->out_urb |= URB_FREE_BUFFER;
 	rc = usb_submit_urb(phy->out_urb, flags);
 
 	return rc;
@@ -170,6 +177,7 @@ static int pn533_usb_send_frame(struct pn533 *dev,
 
 	phy->out_urb->transfer_buffer = out->data;
 	phy->out_urb->transfer_buffer_length = out->len;
+	phy->out_urb &= ~URB_FREE_BUFFER;
 
 	print_hex_dump_debug("PN533 TX: ", DUMP_PREFIX_NONE, 16, 1,
 			     out->data, out->len, false);
@@ -375,12 +383,18 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
 	/* Power on th reader (CCID cmd) */
 	u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
 		      0, 0, 0, 0, 0, 0, 3, 0, 0};
+	char *buffer;
 	int rc;
 	void *cntx;
 	struct pn533_acr122_poweron_rdr_arg arg;
 
 	dev_dbg(&phy->udev->dev, "%s\n", __func__);
 
+	buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+	memcpy(buffer, cmd, sizeof(cmd));
+
 	init_completion(&arg.done);
 	cntx = phy->in_urb->context;  /* backup context */
 
@@ -389,6 +403,7 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
 
 	phy->out_urb->transfer_buffer = cmd;
 	phy->out_urb->transfer_buffer_length = sizeof(cmd);
+	phy->out_urb |= URB_FREE_BUFFER;
 
 	print_hex_dump_debug("ACR122 TX: ", DUMP_PREFIX_NONE, 16, 1,
 		       cmd, sizeof(cmd), false);

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

On Thu, May 17, 2018 at 06:40:04PM +0200, Greg KH wrote:
> Adding the network and NFC developers as this really is a NFC driver
> bug, not a USB core issue...
> 
> On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
> > On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> > > Hello.
> > > I'm having troubles with this NFC card reader. It seems kernel driver
> > > pn533 is not working properly.
> > > At this moment I have my work stalled. I need to add NFC support to a
> > > software product and I can't get the device to work. NFC tools won't
> > > work, the device is not detected.
> > > 
> > > This is what I get from "dmesg":
> > > 
> > > [    4.182300] nfc: nfc_init: NFC Core ver 0.1
> > > [    4.182318] NET: Registered protocol family 39
> > > [    4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> > > [    4.193366] ------------[ cut here ]------------
> > > [    4.193366] transfer buffer not dma capable
> > > [    4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> > > usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > [    4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> > > snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> > > snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> > > snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> > > snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> > > intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> > > snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> > > mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> > > ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> > > videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> > > snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> > > intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> > > intel_rapl_perf tpm_crb psmouse
> > > [    4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> > > wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> > > sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> > > intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> > > rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> > > ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> > > crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> > > serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> > > libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> > > mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> > > sysimgblt fb_sys_fops ttm drm agpgart
> > > [    4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH #1
> > > [    4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> > > PRO A50-C, BIOS Version 7.50   09/26/2016
> > > [    4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > [    4.193468] RSP: 0018:ffffa3b44282f9f8 EFLAGS: 00010282
> > > [    4.193469] RAX: 0000000000000000 RBX: ffff981fc9e320c0 RCX: 0000000000000001
> > > [    4.193470] RDX: 0000000080000001 RSI: 0000000000000002 RDI: 00000000ffffffff
> > > [    4.193471] RBP: ffff981fd42f0000 R08: 0000000713ed01d2 R09: 000000000000001f
> > > [    4.193472] R10: 0000000000000344 R11: 000000000000f300 R12: 00000000014000c0
> > > [    4.193473] R13: 00000000fffffff5 R14: ffff981fd2592b98 R15: 00000000c0410280
> > > [    4.193475] FS:  00007f4fb98d0d40(0000) GS:ffff981fe6d00000(0000)
> > > knlGS:0000000000000000
> > > [    4.193476] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [    4.193477] CR2: 0000562b4a68f6e8 CR3: 00000004532d6004 CR4: 00000000003606e0
> > > [    4.193478] Call Trace:
> > > [    4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> > > [    4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> > > [    4.193495]  ? __kmalloc+0x19e/0x220
> > > [    4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> > > [    4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> > > [    4.193511]  driver_probe_device+0x2b9/0x460
> > > [    4.193514]  ? __driver_attach+0xb6/0xe0
> > > [    4.193516]  ? driver_probe_device+0x460/0x460
> > > [    4.193518]  ? bus_for_each_dev+0x76/0xc0
> > > [    4.193520]  ? bus_add_driver+0x152/0x230
> > > [    4.193522]  ? driver_register+0x6b/0xb0
> > > [    4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> > > [    4.193531]  ? 0xffffffffc13b6000
> > > [    4.193534]  ? do_one_initcall+0x48/0x13b
> > > [    4.193537]  ? free_unref_page_commit+0x6a/0x100
> > > [    4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> > > [    4.193542]  ? do_init_module+0x5a/0x210
> > > [    4.193544]  ? load_module+0x247a/0x29f0
> > > [    4.193549]  ? SyS_init_module+0x139/0x180
> > > [    4.193550]  ? SyS_init_module+0x139/0x180
> > > [    4.193554]  ? do_syscall_64+0x74/0x190
> > > [    4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > [    4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
> > > ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
> > > 97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
> > > 48 8b
> > > [    4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
> > > [    4.193612] usb 1-1: NFC: Reader power on cmd error -11
> > > [    4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
> > > [    4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
> > > [    4.193637] usbcore: registered new interface driver pn533_usb
> > > [    4.198216] usbcore: registered new interface driver usbhid
> > > 
> > > 
> > > Please find the full dmesg in the link below:
> > > https://pastebin.com/ck4sZuUY
> > 
> > Odd that this driver has ever worked.  Has it worked for you on older
> > kernels?
> > 
> > It looks like it is trying to send data off of the stack.  At first
> > glance, I see at least two places it is doing this, which is what is
> > causing the errors you are seeing.   I'll go audit the whole thing in a
> > few hours when I get a chance.
> > 
> > Are you able to build and test a kernel patch if I make one for you?
> 
> Here's a totally untested, and not even built, patch that I knocked up
> that should fix this type of issue.
> 
> I'll try to at least build it later tonight...

Ok, that was dumb, this version at least compiles :)

--------------

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH] NFC: pn533: don't send USB data off of the stack

It's amazing that this driver ever worked, but now that x86 doesn't
allow USB data to be sent off of the stack, it really does not work at
all.  Fix this up by properly allocating the data for the small
"commands" that get sent to the device.

The USB stack will free the buffer when the data has been transmitted,
that is why there is no kfree() to mirror the call to kmalloc().

Reported-by: Carlos Manuel Santos <cmmpsantos@gmail.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index e153e8b64bb8..a0542f86efcf 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -150,10 +150,17 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
 	struct pn533_usb_phy *phy = dev->phy;
 	static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
 	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
+	char *buffer;
 	int rc;
 
+	buffer = kmalloc(sizeof(ack), GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+	memcpy(buffer, ack, sizeof(ack));
+
 	phy->out_urb->transfer_buffer = (u8 *)ack;
 	phy->out_urb->transfer_buffer_length = sizeof(ack);
+	phy->out_urb->transfer_flags |= URB_FREE_BUFFER;
 	rc = usb_submit_urb(phy->out_urb, flags);
 
 	return rc;
@@ -170,6 +177,7 @@ static int pn533_usb_send_frame(struct pn533 *dev,
 
 	phy->out_urb->transfer_buffer = out->data;
 	phy->out_urb->transfer_buffer_length = out->len;
+	phy->out_urb->transfer_flags &= ~URB_FREE_BUFFER;
 
 	print_hex_dump_debug("PN533 TX: ", DUMP_PREFIX_NONE, 16, 1,
 			     out->data, out->len, false);
@@ -375,12 +383,18 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
 	/* Power on th reader (CCID cmd) */
 	u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
 		      0, 0, 0, 0, 0, 0, 3, 0, 0};
+	char *buffer;
 	int rc;
 	void *cntx;
 	struct pn533_acr122_poweron_rdr_arg arg;
 
 	dev_dbg(&phy->udev->dev, "%s\n", __func__);
 
+	buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+	memcpy(buffer, cmd, sizeof(cmd));
+
 	init_completion(&arg.done);
 	cntx = phy->in_urb->context;  /* backup context */
 
@@ -389,6 +403,7 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
 
 	phy->out_urb->transfer_buffer = cmd;
 	phy->out_urb->transfer_buffer_length = sizeof(cmd);
+	phy->out_urb->transfer_flags &= ~URB_FREE_BUFFER;
 
 	print_hex_dump_debug("ACR122 TX: ", DUMP_PREFIX_NONE, 16, 1,
 		       cmd, sizeof(cmd), false);

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Arend van Spriel]

On 5/17/2018 6:46 PM, Greg KH wrote:
> On Thu, May 17, 2018 at 06:40:04PM +0200, Greg KH wrote:
>> Adding the network and NFC developers as this really is a NFC driver
>> bug, not a USB core issue...
>>
>> On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
>>> On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
>>>> Hello.
>>>> I'm having troubles with this NFC card reader. It seems kernel driver
>>>> pn533 is not working properly.
>>>> At this moment I have my work stalled. I need to add NFC support to a
>>>> software product and I can't get the device to work. NFC tools won't
>>>> work, the device is not detected.
>>>>
>>>> This is what I get from "dmesg":
>>>>
>>>> [    4.182300] nfc: nfc_init: NFC Core ver 0.1
>>>> [    4.182318] NET: Registered protocol family 39
>>>> [    4.184676] hidraw: raw HID events driver (C) Jiri Kosina
>>>> [    4.193366] ------------[ cut here ]------------
>>>> [    4.193366] transfer buffer not dma capable
>>>> [    4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
>>>> usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
>>>> [    4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
>>>> snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
>>>> snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
>>>> snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
>>>> snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
>>>> intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
>>>> snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
>>>> mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
>>>> ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
>>>> videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
>>>> snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
>>>> intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
>>>> intel_rapl_perf tpm_crb psmouse
>>>> [    4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
>>>> wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
>>>> sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
>>>> intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
>>>> rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
>>>> ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
>>>> crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
>>>> serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
>>>> libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
>>>> mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
>>>> sysimgblt fb_sys_fops ttm drm agpgart
>>>> [    4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH #1
>>>> [    4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
>>>> PRO A50-C, BIOS Version 7.50   09/26/2016
>>>> [    4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
>>>> [    4.193468] RSP: 0018:ffffa3b44282f9f8 EFLAGS: 00010282
>>>> [    4.193469] RAX: 0000000000000000 RBX: ffff981fc9e320c0 RCX: 0000000000000001
>>>> [    4.193470] RDX: 0000000080000001 RSI: 0000000000000002 RDI: 00000000ffffffff
>>>> [    4.193471] RBP: ffff981fd42f0000 R08: 0000000713ed01d2 R09: 000000000000001f
>>>> [    4.193472] R10: 0000000000000344 R11: 000000000000f300 R12: 00000000014000c0
>>>> [    4.193473] R13: 00000000fffffff5 R14: ffff981fd2592b98 R15: 00000000c0410280
>>>> [    4.193475] FS:  00007f4fb98d0d40(0000) GS:ffff981fe6d00000(0000)
>>>> knlGS:0000000000000000
>>>> [    4.193476] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [    4.193477] CR2: 0000562b4a68f6e8 CR3: 00000004532d6004 CR4: 00000000003606e0
>>>> [    4.193478] Call Trace:
>>>> [    4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
>>>> [    4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
>>>> [    4.193495]  ? __kmalloc+0x19e/0x220
>>>> [    4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
>>>> [    4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
>>>> [    4.193511]  driver_probe_device+0x2b9/0x460
>>>> [    4.193514]  ? __driver_attach+0xb6/0xe0
>>>> [    4.193516]  ? driver_probe_device+0x460/0x460
>>>> [    4.193518]  ? bus_for_each_dev+0x76/0xc0
>>>> [    4.193520]  ? bus_add_driver+0x152/0x230
>>>> [    4.193522]  ? driver_register+0x6b/0xb0
>>>> [    4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
>>>> [    4.193531]  ? 0xffffffffc13b6000
>>>> [    4.193534]  ? do_one_initcall+0x48/0x13b
>>>> [    4.193537]  ? free_unref_page_commit+0x6a/0x100
>>>> [    4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
>>>> [    4.193542]  ? do_init_module+0x5a/0x210
>>>> [    4.193544]  ? load_module+0x247a/0x29f0
>>>> [    4.193549]  ? SyS_init_module+0x139/0x180
>>>> [    4.193550]  ? SyS_init_module+0x139/0x180
>>>> [    4.193554]  ? do_syscall_64+0x74/0x190
>>>> [    4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>>>> [    4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
>>>> ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
>>>> 97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
>>>> 48 8b
>>>> [    4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
>>>> [    4.193612] usb 1-1: NFC: Reader power on cmd error -11
>>>> [    4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
>>>> [    4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
>>>> [    4.193637] usbcore: registered new interface driver pn533_usb
>>>> [    4.198216] usbcore: registered new interface driver usbhid
>>>>
>>>>
>>>> Please find the full dmesg in the link below:
>>>> https://pastebin.com/ck4sZuUY
>>>
>>> Odd that this driver has ever worked.  Has it worked for you on older
>>> kernels?
>>>
>>> It looks like it is trying to send data off of the stack.  At first
>>> glance, I see at least two places it is doing this, which is what is
>>> causing the errors you are seeing.   I'll go audit the whole thing in a
>>> few hours when I get a chance.
>>>
>>> Are you able to build and test a kernel patch if I make one for you?
>>
>> Here's a totally untested, and not even built, patch that I knocked up
>> that should fix this type of issue.
>>
>> I'll try to at least build it later tonight...
>
> Ok, that was dumb, this version at least compiles :)

I think there is a bit more dumbness below... :-p

> --------------
>
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Subject: [PATCH] NFC: pn533: don't send USB data off of the stack
>
> It's amazing that this driver ever worked, but now that x86 doesn't
> allow USB data to be sent off of the stack, it really does not work at
> all.  Fix this up by properly allocating the data for the small
> "commands" that get sent to the device.
>
> The USB stack will free the buffer when the data has been transmitted,
> that is why there is no kfree() to mirror the call to kmalloc().
>
> Reported-by: Carlos Manuel Santos <cmmpsantos@gmail.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
> index e153e8b64bb8..a0542f86efcf 100644
> --- a/drivers/nfc/pn533/usb.c
> +++ b/drivers/nfc/pn533/usb.c
> @@ -150,10 +150,17 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
>   	struct pn533_usb_phy *phy = dev->phy;
>   	static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
>   	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
> +	char *buffer;
>   	int rc;
>
> +	buffer = kmalloc(sizeof(ack), GFP_KERNEL);
> +	if (!buffer)
> +		return -ENOMEM;
> +	memcpy(buffer, ack, sizeof(ack));
> +
>   	phy->out_urb->transfer_buffer = (u8 *)ack;

Shouldn't this be assigned to the allocated bufffer instead?

>   	phy->out_urb->transfer_buffer_length = sizeof(ack);
> +	phy->out_urb->transfer_flags |= URB_FREE_BUFFER;
>   	rc = usb_submit_urb(phy->out_urb, flags);
>
>   	return rc;
> @@ -170,6 +177,7 @@ static int pn533_usb_send_frame(struct pn533 *dev,
>
>   	phy->out_urb->transfer_buffer = out->data;
>   	phy->out_urb->transfer_buffer_length = out->len;
> +	phy->out_urb->transfer_flags &= ~URB_FREE_BUFFER;
>
>   	print_hex_dump_debug("PN533 TX: ", DUMP_PREFIX_NONE, 16, 1,
>   			     out->data, out->len, false);
> @@ -375,12 +383,18 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
>   	/* Power on th reader (CCID cmd) */
>   	u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
>   		      0, 0, 0, 0, 0, 0, 3, 0, 0};
> +	char *buffer;
>   	int rc;
>   	void *cntx;
>   	struct pn533_acr122_poweron_rdr_arg arg;
>
>   	dev_dbg(&phy->udev->dev, "%s\n", __func__);
>
> +	buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
> +	if (!buffer)
> +		return -ENOMEM;
> +	memcpy(buffer, cmd, sizeof(cmd));
> +
>   	init_completion(&arg.done);
>   	cntx = phy->in_urb->context;  /* backup context */
>
> @@ -389,6 +403,7 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
>
>   	phy->out_urb->transfer_buffer = cmd;

and same here.

Regards,
Arend

>   	phy->out_urb->transfer_buffer_length = sizeof(cmd);
> +	phy->out_urb->transfer_flags &= ~URB_FREE_BUFFER;
>
>   	print_hex_dump_debug("ACR122 TX: ", DUMP_PREFIX_NONE, 16, 1,
>   		       cmd, sizeof(cmd), false);
>

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Johannes Berg]

On Fri, 2018-05-18 at 10:56 +0200, Arend van Spriel wrote:
> 
> > +	buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
> > +	if (!buffer)
> > +		return -ENOMEM;
> > +	memcpy(buffer, cmd, sizeof(cmd));

It should also use kmemdup() ;-)

johannes

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

On Fri, May 18, 2018 at 10:56:25AM +0200, Arend van Spriel wrote:
> On 5/17/2018 6:46 PM, Greg KH wrote:
> > On Thu, May 17, 2018 at 06:40:04PM +0200, Greg KH wrote:
> > > Adding the network and NFC developers as this really is a NFC driver
> > > bug, not a USB core issue...
> > > 
> > > On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
> > > > On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> > > > > Hello.
> > > > > I'm having troubles with this NFC card reader. It seems kernel driver
> > > > > pn533 is not working properly.
> > > > > At this moment I have my work stalled. I need to add NFC support to a
> > > > > software product and I can't get the device to work. NFC tools won't
> > > > > work, the device is not detected.
> > > > > 
> > > > > This is what I get from "dmesg":
> > > > > 
> > > > > [    4.182300] nfc: nfc_init: NFC Core ver 0.1
> > > > > [    4.182318] NET: Registered protocol family 39
> > > > > [    4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> > > > > [    4.193366] ------------[ cut here ]------------
> > > > > [    4.193366] transfer buffer not dma capable
> > > > > [    4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> > > > > usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > > > [    4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> > > > > snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> > > > > snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> > > > > snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> > > > > snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> > > > > intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> > > > > snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> > > > > mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> > > > > ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> > > > > videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> > > > > snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> > > > > intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> > > > > intel_rapl_perf tpm_crb psmouse
> > > > > [    4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> > > > > wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> > > > > sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> > > > > intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> > > > > rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> > > > > ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> > > > > crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> > > > > serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> > > > > libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> > > > > mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> > > > > sysimgblt fb_sys_fops ttm drm agpgart
> > > > > [    4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH #1
> > > > > [    4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> > > > > PRO A50-C, BIOS Version 7.50   09/26/2016
> > > > > [    4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > > > [    4.193468] RSP: 0018:ffffa3b44282f9f8 EFLAGS: 00010282
> > > > > [    4.193469] RAX: 0000000000000000 RBX: ffff981fc9e320c0 RCX: 0000000000000001
> > > > > [    4.193470] RDX: 0000000080000001 RSI: 0000000000000002 RDI: 00000000ffffffff
> > > > > [    4.193471] RBP: ffff981fd42f0000 R08: 0000000713ed01d2 R09: 000000000000001f
> > > > > [    4.193472] R10: 0000000000000344 R11: 000000000000f300 R12: 00000000014000c0
> > > > > [    4.193473] R13: 00000000fffffff5 R14: ffff981fd2592b98 R15: 00000000c0410280
> > > > > [    4.193475] FS:  00007f4fb98d0d40(0000) GS:ffff981fe6d00000(0000)
> > > > > knlGS:0000000000000000
> > > > > [    4.193476] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > [    4.193477] CR2: 0000562b4a68f6e8 CR3: 00000004532d6004 CR4: 00000000003606e0
> > > > > [    4.193478] Call Trace:
> > > > > [    4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> > > > > [    4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> > > > > [    4.193495]  ? __kmalloc+0x19e/0x220
> > > > > [    4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> > > > > [    4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> > > > > [    4.193511]  driver_probe_device+0x2b9/0x460
> > > > > [    4.193514]  ? __driver_attach+0xb6/0xe0
> > > > > [    4.193516]  ? driver_probe_device+0x460/0x460
> > > > > [    4.193518]  ? bus_for_each_dev+0x76/0xc0
> > > > > [    4.193520]  ? bus_add_driver+0x152/0x230
> > > > > [    4.193522]  ? driver_register+0x6b/0xb0
> > > > > [    4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> > > > > [    4.193531]  ? 0xffffffffc13b6000
> > > > > [    4.193534]  ? do_one_initcall+0x48/0x13b
> > > > > [    4.193537]  ? free_unref_page_commit+0x6a/0x100
> > > > > [    4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> > > > > [    4.193542]  ? do_init_module+0x5a/0x210
> > > > > [    4.193544]  ? load_module+0x247a/0x29f0
> > > > > [    4.193549]  ? SyS_init_module+0x139/0x180
> > > > > [    4.193550]  ? SyS_init_module+0x139/0x180
> > > > > [    4.193554]  ? do_syscall_64+0x74/0x190
> > > > > [    4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > > > [    4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
> > > > > ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
> > > > > 97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
> > > > > 48 8b
> > > > > [    4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
> > > > > [    4.193612] usb 1-1: NFC: Reader power on cmd error -11
> > > > > [    4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
> > > > > [    4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
> > > > > [    4.193637] usbcore: registered new interface driver pn533_usb
> > > > > [    4.198216] usbcore: registered new interface driver usbhid
> > > > > 
> > > > > 
> > > > > Please find the full dmesg in the link below:
> > > > > https://pastebin.com/ck4sZuUY
> > > > 
> > > > Odd that this driver has ever worked.  Has it worked for you on older
> > > > kernels?
> > > > 
> > > > It looks like it is trying to send data off of the stack.  At first
> > > > glance, I see at least two places it is doing this, which is what is
> > > > causing the errors you are seeing.   I'll go audit the whole thing in a
> > > > few hours when I get a chance.
> > > > 
> > > > Are you able to build and test a kernel patch if I make one for you?
> > > 
> > > Here's a totally untested, and not even built, patch that I knocked up
> > > that should fix this type of issue.
> > > 
> > > I'll try to at least build it later tonight...
> > 
> > Ok, that was dumb, this version at least compiles :)
> 
> I think there is a bit more dumbness below... :-p
> 
> > --------------
> > 
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Subject: [PATCH] NFC: pn533: don't send USB data off of the stack
> > 
> > It's amazing that this driver ever worked, but now that x86 doesn't
> > allow USB data to be sent off of the stack, it really does not work at
> > all.  Fix this up by properly allocating the data for the small
> > "commands" that get sent to the device.
> > 
> > The USB stack will free the buffer when the data has been transmitted,
> > that is why there is no kfree() to mirror the call to kmalloc().
> > 
> > Reported-by: Carlos Manuel Santos <cmmpsantos@gmail.com>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > Cc: stable <stable@vger.kernel.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
> > index e153e8b64bb8..a0542f86efcf 100644
> > --- a/drivers/nfc/pn533/usb.c
> > +++ b/drivers/nfc/pn533/usb.c
> > @@ -150,10 +150,17 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
> >   	struct pn533_usb_phy *phy = dev->phy;
> >   	static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
> >   	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
> > +	char *buffer;
> >   	int rc;
> > 
> > +	buffer = kmalloc(sizeof(ack), GFP_KERNEL);
> > +	if (!buffer)
> > +		return -ENOMEM;
> > +	memcpy(buffer, ack, sizeof(ack));
> > +
> >   	phy->out_urb->transfer_buffer = (u8 *)ack;
> 
> Shouldn't this be assigned to the allocated bufffer instead?
> 
> >   	phy->out_urb->transfer_buffer_length = sizeof(ack);
> > +	phy->out_urb->transfer_flags |= URB_FREE_BUFFER;
> >   	rc = usb_submit_urb(phy->out_urb, flags);
> > 
> >   	return rc;
> > @@ -170,6 +177,7 @@ static int pn533_usb_send_frame(struct pn533 *dev,
> > 
> >   	phy->out_urb->transfer_buffer = out->data;
> >   	phy->out_urb->transfer_buffer_length = out->len;
> > +	phy->out_urb->transfer_flags &= ~URB_FREE_BUFFER;
> > 
> >   	print_hex_dump_debug("PN533 TX: ", DUMP_PREFIX_NONE, 16, 1,
> >   			     out->data, out->len, false);
> > @@ -375,12 +383,18 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
> >   	/* Power on th reader (CCID cmd) */
> >   	u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
> >   		      0, 0, 0, 0, 0, 0, 3, 0, 0};
> > +	char *buffer;
> >   	int rc;
> >   	void *cntx;
> >   	struct pn533_acr122_poweron_rdr_arg arg;
> > 
> >   	dev_dbg(&phy->udev->dev, "%s\n", __func__);
> > 
> > +	buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
> > +	if (!buffer)
> > +		return -ENOMEM;
> > +	memcpy(buffer, cmd, sizeof(cmd));
> > +
> >   	init_completion(&arg.done);
> >   	cntx = phy->in_urb->context;  /* backup context */
> > 
> > @@ -389,6 +403,7 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
> > 
> >   	phy->out_urb->transfer_buffer = cmd;
> 
> and same here.

Ugh, that's what I get for writing kernel patches while sitting in a
cafe...

Let me go try this again...

greg k-h

Re: [PATCH] NFC: pn533: don't send USB data off of the stack

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]

Hi Greg,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[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/Greg-KH/NFC-pn533-don-t-send-USB-data-off-of-the-stack/20180518-100416
config: i386-randconfig-x013-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/nfc/pn533/usb.c: In function 'pn533_usb_send_ack':
>> drivers/nfc/pn533/usb.c:163:15: error: invalid operands to binary | (have 'struct urb *' and 'int')
     phy->out_urb |= URB_FREE_BUFFER;
                  ^~
   drivers/nfc/pn533/usb.c: In function 'pn533_usb_send_frame':
   drivers/nfc/pn533/usb.c:180:15: error: invalid operands to binary & (have 'struct urb *' and 'int')
     phy->out_urb &= ~URB_FREE_BUFFER;
                  ^~
   drivers/nfc/pn533/usb.c: In function 'pn533_acr122_poweron_rdr':
   drivers/nfc/pn533/usb.c:406:15: error: invalid operands to binary | (have 'struct urb *' and 'int')
     phy->out_urb |= URB_FREE_BUFFER;
                  ^~

vim +163 drivers/nfc/pn533/usb.c

   147	
   148	static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
   149	{
   150		struct pn533_usb_phy *phy = dev->phy;
   151		static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
   152		/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
   153		char *buffer;
   154		int rc;
   155	
   156		buffer = kmalloc(sizeof(ack), GFP_KERNEL);
   157		if (!buffer)
   158			return -ENOMEM;
   159		memcpy(buffer, ack, sizeof(ack));
   160	
   161		phy->out_urb->transfer_buffer = (u8 *)ack;
   162		phy->out_urb->transfer_buffer_length = sizeof(ack);
 > 163		phy->out_urb |= URB_FREE_BUFFER;
   164		rc = usb_submit_urb(phy->out_urb, flags);
   165	
   166		return rc;
   167	}
   168	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34298 bytes --]