The xpad driver has several races with respect to URB submission which
make it easy to end up with submission while active:
[ cut here ]
WARNING: CPU: 3 PID: 3563 at drivers/usb/core/urb.c:339
usb_submit_urb+0x2ad/0x5a0()
URB 8804078ac240 submitted while active
Modules linked in: fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun
nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_rpfilter ip6t_REJECT
nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge ebtable_filter
ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_security iptable_raw bnep xpadsnd_hda_codec_realtek
snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec
snd_hda_core snd_hwdep snd_seq intel_rapl iosf_mbi snd_seq_device
x86_pkg_temp_thermal coretemp snd_pcm kvm uvcvideo iTCO_wdt iTCO_vendor_support
iwlwifi videobuf2_vmalloc videobuf2_core videobuf2_memops
v4l2_common btusb videodev btrtl btbcm thinkpad_acpi snd_timer btintel
mei_me rtsx_pci_ms cfg80211 bluetooth pcspkr mei media memstick joydev snd
tpm_tis shpchp ie31200_edac i2c_i801 tpm lpc_ich edac_core nfsd rfkill wmi
soundcore auth_rpcgss nfs_acl lockd grace sunrpc binfmt_misc dm_crypt i915 8021q
garp stp llc mrp i2c_algo_bit drm_kms_helper drm rtsx_pci_sdmmc mmc_core e1000e
crct10dif_pclmul crc32_pclmul crc32c_intel rtsx_pci ghash_clmulni_intel
ptp serio_raw pps_core mfd_core video
CPU: 3 PID: 3563 Comm: led_test.sh Not tainted 4.2.0-rc4-xpad+ #14
Hardware name: LENOVO 20BFS0EC00/20BFS0EC00, BIOS GMET62WW (2.10 )
03/19/2014
17a45bc6 8800c9a0fbd8 81758a11
8800c9a0fc30 8800c9a0fc18 8109b656
0002 8804078ac240 00d0 8800c9806d60
Call Trace:
[] dump_stack+0x45/0x57
[] warn_slowpath_common+0x86/0xc0
[] warn_slowpath_fmt+0x55/0x70
[] ? do_truncate+0x88/0xc0
[] usb_submit_urb+0x2ad/0x5a0
[] ? mntput+0x24/0x40
[] ? terminate_walk+0xc7/0xe0
[] xpad_send_led_command+0xc7/0x110 [xpad]
[] xpad_led_set+0x15/0x20 [xpad]
[] led_set_brightness+0x88/0xc0
[] brightness_store+0x7e/0xc0
[] dev_attr_store+0x18/0x30
[] sysfs_kf_write+0x37/0x40
[] kernfs_fop_write+0x11d/0x170
[] __vfs_write+0x37/0x100
[] ? __sb_start_write+0x58/0x110
[] ? security_file_permission+0x3d/0xc0
[] vfs_write+0xa6/0x1a0
[] ? filp_close+0x5a/0x80
[] SyS_write+0x55/0xc0
[] entry_SYSCALL_64_fastpath+0x12/0x71
---[ end trace f573b768c94a66d6 ]---
This is easily reproducible with
while true; do
for i in $(seq 0 5); do
echo $i > /sys/class/leds/xpad0/subsystem/xpad0/brightness
done
done
xpad_send_led_command attempts to protect against races by locking
around usb_submit_urb. This is not sufficent since usb_submit_urb
is asynchronous; the urb is considered to be in use until the callback
(xpad_irq_out) returns appropriately. Additionally, there is no
protection at all in xpad_play_effect which uses the same urb. Fix this
by creating a queue for urb submission. This will serialize requests to
ensure only one is submitted at a time.
Signed-off-by: Laura Abbott
---
v2: Created a proper queue for events instead of just dropping them
---
drivers/input/joystick/xpad.c | 273 ++
1 file changed, 194 insertions(+), 79 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index f8850f9..e94cc49 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -100,6 +100,9 @@
#define XTYPE_XBOXONE 3
#define XTYPE_UNKNOWN 4
+#define OUT_IRQ_SUBMITTED 0
+#define OUT_IRQ_QUEUE_EMPTY1
+
static bool dpad_to_buttons;
module_param(dpad_to_buttons, bool, S_IRUGO);
MODULE_PARM_DESC(dpad_to_buttons, "Map D-PAD to buttons rather than axes for
unknown pads");
@@ -334,7 +337,9 @@ struct usb_xpad {
struct urb *irq_out;/* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
- struct mutex odata_mutex;
+ unsigned long odata_flags;
+ spinlock_t queue_lock;
+ struct list_head odata_queue; /* queue of commands */
#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct xpad_led *led;
@@ -347,6 +352,94 @@ struct usb_xpad {
unsigned long led_no; /* led to lit on xbox360 controllers */
};
+struct xpad_queue_item {
+ char odata[XPAD_PKT_LEN];
+ int transfer_length;
+ struct list_head entry;
+};
+
+static void __xpad_remove_urb(struct usb_xpad *xpad, struct xpad_queue_item
*item)
+{
+ list_del(&item->entry);
+ kfree(item);
+}
+
+/*
+ * This function must be called with the queue lock held
+ *
+ * returns
+ * 0 - urb successfully submitted
+ * OUT_IRQ_QUEUE_EMPTY - queue was empty
+ * negative - error submittin
