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@xxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxx>
Cc: Samuel Ortiz <sameo@xxxxxxxxxxxxxxx>
Cc: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
Cc: stable <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
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);
