All of lore.kernel.org
 help / color / mirror / Atom feed
* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-26  9:50 Christo Gouws
  2019-04-27  7:01 ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Christo Gouws @ 2019-04-26  9:50 UTC (permalink / raw)
  To: linux-usb

Hi,

I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
the following crash in dmesg on Ubuntu 18.04
Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
seems to happen (using liveCD).


The output on the card seems to work, but none of the inputs work.

I've also now tested with latest kernel available on Arch Linux
Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
2019 x86_64 GNU/Linux

After some further testing, I found that this issue cropped in beween
v4.8.17 and v4.9-rc1.

v4.8.17   - Works fine.
v4.9-rc1+  - Produces crash

[    5.114276] ------------[ cut here ]------------
[    5.114277] transfer buffer not dma capable
[    5.114293] WARNING: CPU: 1 PID: 429 at drivers/usb/core/hcd.c:1586
usb_hcd_map_urb_for_dma+0x3eb/0x5a0
[    5.114294] Modules linked in: snd_seq_oss(+) snd_usb_toneport(+)
snd_usb_line6 videobuf2_common btusb snd_seq_midi btrtl irqbypass
videodev btbcm snd_hda_codec_conexant drm_kms_helper xpad
snd_seq_midi_event snd_hda_codec_generic media btintel cmdlinepart drm
ff_memless bluetooth snd_hda_intel i2c_algo_bit
drm_panel_orientation_quirks snd_rawmidi intel_spi_platform joydev
ipmi_devintf ipmi_msghandler input_leds snd_hda_codec ecdh_generic
cfg80211 cfbfillrect snd_hda_core cfbimgblt cfbcopyarea thinkpad_acpi
snd_hwdep rtsx_pci_ms fb_sys_fops syscopyarea snd_seq sysfillrect
sysimgblt intel_spi memstick mei_me spi_nor nvram fb snd_pcm mtd mei
fbdev snd_seq_device snd_timer ie31200_edac snd soundcore mac_hid
sch_fq_codel parport_pc ppdev lp sunrpc parport ip_tables x_tables
autofs4 hid_generic usbhid hid rtsx_pci_sdmmc psmouse ahci i2c_core
r8169 rtsx_pci libahci lpc_ich realtek wmi video
[    5.114332] CPU: 1 PID: 429 Comm: systemd-udevd Not tainted
4.20.8-042008-generic #201902121544
[    5.114333] Hardware name: LENOVO 20C600HHZA/20C600HHZA, BIOS
J9ET99WW (2.19 ) 05/05/2015
[    5.114336] RIP: 0010:usb_hcd_map_urb_for_dma+0x3eb/0x5a0
[    5.114337] Code: 48 01 f9 48 39 ce 73 7f 80 3d c6 03 eb 00 00 41
bd f5 ff ff ff 75 a3 48 c7 c7 00 e1 d5 8d c6 05 b0 03 eb 00 01 e8 07
32 95 ff <0f> 0b 8b 53 64 eb 89 81 4b 64 00 00 02 00 e9 40 fd ff ff 4c
8b 05
[    5.114339] RSP: 0018:ffffacc582077788 EFLAGS: 00010282
[    5.114340] RAX: 0000000000000000 RBX: ffff9eb0282b7c00 RCX: 0000000000000006
[    5.114341] RDX: 0000000000000007 RSI: 0000000000000092 RDI: ffff9eb02e056440
[    5.114343] RBP: ffffacc5820777a8 R08: 0000000000000001 R09: 0000000000000361
[    5.114344] R10: 0000000000000004 R11: 0000000000000000 R12: ffff9eb02bcde000
[    5.114345] R13: 00000000fffffff5 R14: 0000000000600000 R15: 0000000000000200
[    5.114347] FS:  00007fbbc4c6e8c0(0000) GS:ffff9eb02e040000(0000)
knlGS:0000000000000000
[    5.114348] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    5.114349] CR2: 000055d1c2aa6008 CR3: 000000042abf2006 CR4: 00000000001606e0
[    5.114350] Call Trace:
[    5.114355]  usb_hcd_submit_urb+0x497/0xc70
[    5.114358]  ? __switch_to_asm+0x40/0x70
[    5.114360]  ? __switch_to_asm+0x34/0x70
[    5.114362]  ? __switch_to_asm+0x40/0x70
[    5.114364]  ? __switch_to_asm+0x34/0x70
[    5.114367]  ? __switch_to+0x471/0x4e0
[    5.114369]  ? __switch_to_asm+0x34/0x70
[    5.114371]  ? usb_alloc_urb+0x1a/0x30
[    5.114373]  usb_submit_urb+0x2f2/0x5a0
[    5.114376]  usb_start_wait_urb+0x6e/0x180
[    5.114378]  usb_control_msg+0xdc/0x140
[    5.114383]  line6_read_data+0xec/0x200 [snd_usb_line6]
[    5.114386]  line6_read_serial_number+0x1b/0x20 [snd_usb_line6]
[    5.114389]  toneport_init+0x9a/0x220 [snd_usb_toneport]
[    5.114393]  line6_probe+0x2f1/0x460 [snd_usb_line6]
[    5.114395]  ? line6_toneport_disconnect+0x70/0x70 [snd_usb_toneport]
[    5.114398]  toneport_probe+0x35/0x40 [snd_usb_toneport]
[    5.114401]  usb_probe_interface+0xf1/0x300
[    5.114405]  really_probe+0xfe/0x3b0
[    5.114407]  driver_probe_device+0xba/0x100
[    5.114409]  __driver_attach+0xe4/0x110
[    5.114411]  ? driver_probe_device+0x100/0x100
[    5.114413]  bus_for_each_dev+0x79/0xc0
[    5.114417]  ? kmem_cache_alloc_trace+0x15e/0x1e0
[    5.114419]  driver_attach+0x1e/0x20
[    5.114422]  bus_add_driver+0x159/0x230
[    5.114424]  driver_register+0x70/0xc0
[    5.114426]  usb_register_driver+0x7f/0x140
[    5.114428]  ? 0xffffffffc02f1000
[    5.114431]  toneport_driver_init+0x23/0x1000 [snd_usb_toneport]
[    5.114434]  do_one_initcall+0x4a/0x1c4
[    5.114436]  ? _cond_resched+0x19/0x30
[    5.114438]  ? kmem_cache_alloc_trace+0x15e/0x1e0
[    5.114441]  do_init_module+0x60/0x220
[    5.114444]  load_module+0x16bb/0x1920
[    5.114447]  __do_sys_finit_module+0xbd/0x120
[    5.114449]  ? __do_sys_finit_module+0xbd/0x120
[    5.114452]  __x64_sys_finit_module+0x1a/0x20
[    5.114454]  do_syscall_64+0x5a/0x110
[    5.114456]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[    5.114458] RIP: 0033:0x7fbbc56f3219
[    5.114460] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00
00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 47 fc 0c 00 f7 d8 64 89
01 48
[    5.114461] RSP: 002b:00007ffcd2692b38 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[    5.114463] RAX: ffffffffffffffda RBX: 000055b7b7649160 RCX: 00007fbbc56f3219
[    5.114464] RDX: 0000000000000000 RSI: 00007fbbc55d7cad RDI: 000000000000000f
[    5.114465] RBP: 00007fbbc55d7cad R08: 0000000000000000 R09: 0000000000000000
[    5.114466] R10: 000000000000000f R11: 0000000000000246 R12: 0000000000000000
[    5.114467] R13: 000055b7b7625220 R14: 0000000000020000 R15: 000055b7b7649160
[    5.114470] ---[ end trace 4f385244c87aeb99 ]---
[    5.114474] snd_usb_toneport 1-6:1.0: receive length failed (error -11)
[    5.114779] snd_usb_toneport 1-6:1.0: read request failed (error -32)
[    5.114783] snd_usb_toneport 1-6:1.0: write request failed (error -11)
[    5.115836] snd_usb_toneport 1-6:1.0: Line 6 POD Studio UX1 now attached
[    5.115876] usbcore: registered new interface driver snd_usb_toneport

Kind Regards,
Christo Gouws

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
  2019-04-26  9:50 Christo Gouws
@ 2019-04-27  7:01 ` Greg KH
  2019-04-27 15:34   ` Alan Stern
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2019-04-27  7:01 UTC (permalink / raw)
  To: Christo Gouws; +Cc: linux-usb

On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> Hi,
> 
> I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> the following crash in dmesg on Ubuntu 18.04
> Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> 
> I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> seems to happen (using liveCD).
> 
> 
> The output on the card seems to work, but none of the inputs work.
> 
> I've also now tested with latest kernel available on Arch Linux
> Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> 2019 x86_64 GNU/Linux
> 
> After some further testing, I found that this issue cropped in beween
> v4.8.17 and v4.9-rc1.
> 
> v4.8.17   - Works fine.
> v4.9-rc1+  - Produces crash

Any chance you can use 'git bisect' to find the exact commit that caused
the failure?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
  2019-04-27  7:01 ` Greg KH
@ 2019-04-27 15:34   ` Alan Stern
  0 siblings, 0 replies; 13+ messages in thread
From: Alan Stern @ 2019-04-27 15:34 UTC (permalink / raw)
  To: Greg KH; +Cc: Christo Gouws, linux-usb

On Sat, 27 Apr 2019, Greg KH wrote:

> On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > Hi,
> > 
> > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > the following crash in dmesg on Ubuntu 18.04
> > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > 
> > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > seems to happen (using liveCD).
> > 
> > 
> > The output on the card seems to work, but none of the inputs work.
> > 
> > I've also now tested with latest kernel available on Arch Linux
> > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > 2019 x86_64 GNU/Linux
> > 
> > After some further testing, I found that this issue cropped in beween
> > v4.8.17 and v4.9-rc1.
> > 
> > v4.8.17   - Works fine.
> > v4.9-rc1+  - Produces crash
> 
> Any chance you can use 'git bisect' to find the exact commit that caused
> the failure?

No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
That routine passes an invalid buffer to usb_control_message().  
Instead it should allocate its own buffer for the USB transfer and then
copy the value to the caller's buffer.

There is a similar problem in line6_write_data().  Furthermore, both
routines do DMA to/from a buffer on the stack.

Alan Stern


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:07 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-27 18:07 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> On Sat, 27 Apr 2019, Greg KH wrote:
> 
> > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > Hi,
> > > 
> > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > the following crash in dmesg on Ubuntu 18.04
> > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > 
> > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > seems to happen (using liveCD).
> > > 
> > > 
> > > The output on the card seems to work, but none of the inputs work.
> > > 
> > > I've also now tested with latest kernel available on Arch Linux
> > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > 2019 x86_64 GNU/Linux
> > > 
> > > After some further testing, I found that this issue cropped in beween
> > > v4.8.17 and v4.9-rc1.
> > > 
> > > v4.8.17   - Works fine.
> > > v4.9-rc1+  - Produces crash
> > 
> > Any chance you can use 'git bisect' to find the exact commit that caused
> > the failure?
> 
> No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> That routine passes an invalid buffer to usb_control_message().  
> Instead it should allocate its own buffer for the USB transfer and then
> copy the value to the caller's buffer.
> 
> There is a similar problem in line6_write_data().  Furthermore, both
> routines do DMA to/from a buffer on the stack.

I have an old patch in my local tree for the dma buffer on the stack
issue, it's below.  I should clean it up and send it correctly one of
these days :)

From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 23 Jan 2019 11:01:46 +0100
Subject: [PATCH 3/3] toneport fixes
---
 sound/usb/line6/toneport.c |   23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
 /*
 	Setup Toneport device.
 */
-static void toneport_setup(struct usb_line6_toneport *toneport)
+static int toneport_setup(struct usb_line6_toneport *toneport)
 {
-	u32 ticks;
+	u32 *ticks;
 	struct usb_line6 *line6 = &toneport->line6;
 	struct usb_device *usbdev = line6->usbdev;
 
+	ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
+	if (!ticks)
+		return -ENOMEM;
+
 	/* sync time on device with host: */
 	/* note: 32-bit timestamps overflow in year 2106 */
-	ticks = (u32)ktime_get_real_seconds();
-	line6_write_data(line6, 0x80c6, &ticks, 4);
+	*ticks = (u32)ktime_get_real_seconds();
+	line6_write_data(line6, 0x80c6, ticks, 4);
+	kfree(ticks);
 
 	/* enable device: */
 	toneport_send_cmd(usbdev, 0x0301, 0x0000);
@@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
 			return err;
 	}
 
-	toneport_setup(toneport);
+	err = toneport_setup(toneport);
+	if (err)
+		return err;
 
 	/* register audio system: */
 	return snd_card_register(line6->card);
@@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
 */
 static int toneport_reset_resume(struct usb_interface *interface)
 {
-	toneport_setup(usb_get_intfdata(interface));
+	int err;
+
+	err = toneport_setup(usb_get_intfdata(interface));
+	if (err)
+		return err;
 	return line6_resume(interface);
 }
 #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:07 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2019-04-27 18:07 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> On Sat, 27 Apr 2019, Greg KH wrote:
> 
> > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > Hi,
> > > 
> > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > the following crash in dmesg on Ubuntu 18.04
> > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > 
> > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > seems to happen (using liveCD).
> > > 
> > > 
> > > The output on the card seems to work, but none of the inputs work.
> > > 
> > > I've also now tested with latest kernel available on Arch Linux
> > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > 2019 x86_64 GNU/Linux
> > > 
> > > After some further testing, I found that this issue cropped in beween
> > > v4.8.17 and v4.9-rc1.
> > > 
> > > v4.8.17   - Works fine.
> > > v4.9-rc1+  - Produces crash
> > 
> > Any chance you can use 'git bisect' to find the exact commit that caused
> > the failure?
> 
> No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> That routine passes an invalid buffer to usb_control_message().  
> Instead it should allocate its own buffer for the USB transfer and then
> copy the value to the caller's buffer.
> 
> There is a similar problem in line6_write_data().  Furthermore, both
> routines do DMA to/from a buffer on the stack.

I have an old patch in my local tree for the dma buffer on the stack
issue, it's below.  I should clean it up and send it correctly one of
these days :)

From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 23 Jan 2019 11:01:46 +0100
Subject: [PATCH 3/3] toneport fixes

---
 sound/usb/line6/toneport.c |   23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
 /*
 	Setup Toneport device.
 */
-static void toneport_setup(struct usb_line6_toneport *toneport)
+static int toneport_setup(struct usb_line6_toneport *toneport)
 {
-	u32 ticks;
+	u32 *ticks;
 	struct usb_line6 *line6 = &toneport->line6;
 	struct usb_device *usbdev = line6->usbdev;
 
+	ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
+	if (!ticks)
+		return -ENOMEM;
+
 	/* sync time on device with host: */
 	/* note: 32-bit timestamps overflow in year 2106 */
-	ticks = (u32)ktime_get_real_seconds();
-	line6_write_data(line6, 0x80c6, &ticks, 4);
+	*ticks = (u32)ktime_get_real_seconds();
+	line6_write_data(line6, 0x80c6, ticks, 4);
+	kfree(ticks);
 
 	/* enable device: */
 	toneport_send_cmd(usbdev, 0x0301, 0x0000);
@@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
 			return err;
 	}
 
-	toneport_setup(toneport);
+	err = toneport_setup(toneport);
+	if (err)
+		return err;
 
 	/* register audio system: */
 	return snd_card_register(line6->card);
@@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
 */
 static int toneport_reset_resume(struct usb_interface *interface)
 {
-	toneport_setup(usb_get_intfdata(interface));
+	int err;
+
+	err = toneport_setup(usb_get_intfdata(interface));
+	if (err)
+		return err;
 	return line6_resume(interface);
 }
 #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:24 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-27 18:24 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > On Sat, 27 Apr 2019, Greg KH wrote:
> > 
> > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > Hi,
> > > > 
> > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > the following crash in dmesg on Ubuntu 18.04
> > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > 
> > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > seems to happen (using liveCD).
> > > > 
> > > > 
> > > > The output on the card seems to work, but none of the inputs work.
> > > > 
> > > > I've also now tested with latest kernel available on Arch Linux
> > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > 2019 x86_64 GNU/Linux
> > > > 
> > > > After some further testing, I found that this issue cropped in beween
> > > > v4.8.17 and v4.9-rc1.
> > > > 
> > > > v4.8.17   - Works fine.
> > > > v4.9-rc1+  - Produces crash
> > > 
> > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > the failure?
> > 
> > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> > That routine passes an invalid buffer to usb_control_message().  
> > Instead it should allocate its own buffer for the USB transfer and then
> > copy the value to the caller's buffer.
> > 
> > There is a similar problem in line6_write_data().  Furthermore, both
> > routines do DMA to/from a buffer on the stack.
> 
> I have an old patch in my local tree for the dma buffer on the stack
> issue, it's below.  I should clean it up and send it correctly one of
> these days :)

But, in reading your response, it doesn't fix the reported issue here.
Let me go audit the whole driver and fix it up and add it to my original
patch...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:24 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2019-04-27 18:24 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > On Sat, 27 Apr 2019, Greg KH wrote:
> > 
> > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > Hi,
> > > > 
> > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > the following crash in dmesg on Ubuntu 18.04
> > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > 
> > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > seems to happen (using liveCD).
> > > > 
> > > > 
> > > > The output on the card seems to work, but none of the inputs work.
> > > > 
> > > > I've also now tested with latest kernel available on Arch Linux
> > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > 2019 x86_64 GNU/Linux
> > > > 
> > > > After some further testing, I found that this issue cropped in beween
> > > > v4.8.17 and v4.9-rc1.
> > > > 
> > > > v4.8.17   - Works fine.
> > > > v4.9-rc1+  - Produces crash
> > > 
> > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > the failure?
> > 
> > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> > That routine passes an invalid buffer to usb_control_message().  
> > Instead it should allocate its own buffer for the USB transfer and then
> > copy the value to the caller's buffer.
> > 
> > There is a similar problem in line6_write_data().  Furthermore, both
> > routines do DMA to/from a buffer on the stack.
> 
> I have an old patch in my local tree for the dma buffer on the stack
> issue, it's below.  I should clean it up and send it correctly one of
> these days :)

But, in reading your response, it doesn't fix the reported issue here.
Let me go audit the whole driver and fix it up and add it to my original
patch...


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:42 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-27 18:42 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 08:24:32PM +0200, Greg KH wrote:
> On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > > On Sat, 27 Apr 2019, Greg KH wrote:
> > > 
> > > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > > Hi,
> > > > > 
> > > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > > the following crash in dmesg on Ubuntu 18.04
> > > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > > 
> > > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > > seems to happen (using liveCD).
> > > > > 
> > > > > 
> > > > > The output on the card seems to work, but none of the inputs work.
> > > > > 
> > > > > I've also now tested with latest kernel available on Arch Linux
> > > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > > 2019 x86_64 GNU/Linux
> > > > > 
> > > > > After some further testing, I found that this issue cropped in beween
> > > > > v4.8.17 and v4.9-rc1.
> > > > > 
> > > > > v4.8.17   - Works fine.
> > > > > v4.9-rc1+  - Produces crash
> > > > 
> > > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > > the failure?
> > > 
> > > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> > > That routine passes an invalid buffer to usb_control_message().  
> > > Instead it should allocate its own buffer for the USB transfer and then
> > > copy the value to the caller's buffer.
> > > 
> > > There is a similar problem in line6_write_data().  Furthermore, both
> > > routines do DMA to/from a buffer on the stack.
> > 
> > I have an old patch in my local tree for the dma buffer on the stack
> > issue, it's below.  I should clean it up and send it correctly one of
> > these days :)
> 
> But, in reading your response, it doesn't fix the reported issue here.
> Let me go audit the whole driver and fix it up and add it to my original
> patch...

Ok, here's a patch that should be "complete".

Christo, can you test this out and let us know if it fixes the issue for
you or not?

thanks,

greg k-h

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

From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 23 Jan 2019 11:01:46 +0100
Subject: [PATCH] sound: USB: line6: use dynamic buffers

The line6 driver uses a lot of USB buffers off of the stack, which is
not allowed on many systems.  Fix this up by dynamically allocating the
buffers with kmalloc() which allows for proper DMA-able memory.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable <stable@vger.kernel.org>
---
 sound/usb/line6/driver.c   |   60 ++++++++++++++++++++++++++-------------------
 sound/usb/line6/podhd.c    |   21 +++++++++------
 sound/usb/line6/toneport.c |   23 ++++++++++++-----
 3 files changed, 64 insertions(+), 40 deletions(-)

--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -351,12 +351,16 @@ int line6_read_data(struct usb_line6 *li
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	unsigned char len;
+	unsigned char *len;
 	unsigned count;
 
 	if (address > 0xffff || datalen > 0xff)
 		return -EINVAL;
 
+	len = kmalloc(sizeof(*len), GFP_KERNEL);
+	if (!len)
+		return -ENOMEM;
+
 	/* query the serial number: */
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
 			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
@@ -365,7 +369,7 @@ int line6_read_data(struct usb_line6 *li
 
 	if (ret < 0) {
 		dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	/* Wait for data length. We'll get 0xff until length arrives. */
@@ -375,28 +379,29 @@ int line6_read_data(struct usb_line6 *li
 		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
 				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
 				      USB_DIR_IN,
-				      0x0012, 0x0000, &len, 1,
+				      0x0012, 0x0000, len, 1,
 				      LINE6_TIMEOUT * HZ);
 		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receive length failed (error %d)\n", ret);
-			return ret;
+			goto exit;
 		}
 
-		if (len != 0xff)
+		if (*len != 0xff)
 			break;
 	}
 
-	if (len == 0xff) {
+	ret = -EIO;
+	if (*len == 0xff) {
 		dev_err(line6->ifcdev, "read failed after %d retries\n",
 			count);
-		return -EIO;
-	} else if (len != datalen) {
+		goto exit;
+	} else if (*len != datalen) {
 		/* should be equal or something went wrong */
 		dev_err(line6->ifcdev,
 			"length mismatch (expected %d, got %d)\n",
-			(int)datalen, (int)len);
-		return -EIO;
+			(int)datalen, (int)*len);
+		goto exit;
 	}
 
 	/* receive the result: */
@@ -405,12 +410,12 @@ int line6_read_data(struct usb_line6 *li
 			      0x0013, 0x0000, data, datalen,
 			      LINE6_TIMEOUT * HZ);
 
-	if (ret < 0) {
+	if (ret < 0)
 		dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
-		return ret;
-	}
 
-	return 0;
+exit:
+	kfree(len);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_read_data);
 
@@ -422,12 +427,16 @@ int line6_write_data(struct usb_line6 *l
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	unsigned char status;
+	unsigned char *status;
 	int count;
 
 	if (address > 0xffff || datalen > 0xffff)
 		return -EINVAL;
 
+	status = kmalloc(sizeof(*status), GFP_KERNEL);
+	if (!status)
+		return -ENOMEM;
+
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
 			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 			      0x0022, address, data, datalen,
@@ -436,7 +445,7 @@ int line6_write_data(struct usb_line6 *l
 	if (ret < 0) {
 		dev_err(line6->ifcdev,
 			"write request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
@@ -447,28 +456,29 @@ int line6_write_data(struct usb_line6 *l
 				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
 				      USB_DIR_IN,
 				      0x0012, 0x0000,
-				      &status, 1, LINE6_TIMEOUT * HZ);
+				      status, 1, LINE6_TIMEOUT * HZ);
 
 		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receiving status failed (error %d)\n", ret);
-			return ret;
+			goto exit;
 		}
 
-		if (status != 0xff)
+		if (*status != 0xff)
 			break;
 	}
 
-	if (status == 0xff) {
+	if (*status == 0xff) {
 		dev_err(line6->ifcdev, "write failed after %d retries\n",
 			count);
-		return -EIO;
-	} else if (status != 0) {
+		ret = -EIO;
+	} else if (*status != 0) {
 		dev_err(line6->ifcdev, "write failed (error %d)\n", ret);
-		return -EIO;
+		ret = -EIO;
 	}
-
-	return 0;
+exit:
+	kfree(status);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_write_data);
 
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -225,28 +225,32 @@ static void podhd_startup_start_workqueu
 static int podhd_dev_start(struct usb_line6_podhd *pod)
 {
 	int ret;
-	u8 init_bytes[8];
+	u8 *init_bytes;
 	int i;
 	struct usb_device *usbdev = pod->line6.usbdev;
 
+	init_bytes = kmalloc(8, GFP_KERNEL);
+	if (!init_bytes)
+		return -ENOMEM;
+
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
 					0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 					0x11, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
 	if (ret < 0) {
 		dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	/* NOTE: looks like some kind of ping message */
 	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
 					USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
 					0x11, 0x0,
-					&init_bytes, 3, LINE6_TIMEOUT * HZ);
+					init_bytes, 3, LINE6_TIMEOUT * HZ);
 	if (ret < 0) {
 		dev_err(pod->line6.ifcdev,
 			"receive length failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	pod->firmware_version =
@@ -255,7 +259,7 @@ static int podhd_dev_start(struct usb_li
 	for (i = 0; i <= 16; i++) {
 		ret = line6_read_data(&pod->line6, 0xf000 + 0x08 * i, init_bytes, 8);
 		if (ret < 0)
-			return ret;
+			goto exit;
 	}
 
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
@@ -263,10 +267,9 @@ static int podhd_dev_start(struct usb_li
 					USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
 					1, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+exit:
+	kfree(init_bytes);
+	return ret;
 }
 
 static void podhd_startup_workqueue(struct work_struct *work)
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
 /*
 	Setup Toneport device.
 */
-static void toneport_setup(struct usb_line6_toneport *toneport)
+static int toneport_setup(struct usb_line6_toneport *toneport)
 {
-	u32 ticks;
+	u32 *ticks;
 	struct usb_line6 *line6 = &toneport->line6;
 	struct usb_device *usbdev = line6->usbdev;
 
+	ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
+	if (!ticks)
+		return -ENOMEM;
+
 	/* sync time on device with host: */
 	/* note: 32-bit timestamps overflow in year 2106 */
-	ticks = (u32)ktime_get_real_seconds();
-	line6_write_data(line6, 0x80c6, &ticks, 4);
+	*ticks = (u32)ktime_get_real_seconds();
+	line6_write_data(line6, 0x80c6, ticks, 4);
+	kfree(ticks);
 
 	/* enable device: */
 	toneport_send_cmd(usbdev, 0x0301, 0x0000);
@@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
 			return err;
 	}
 
-	toneport_setup(toneport);
+	err = toneport_setup(toneport);
+	if (err)
+		return err;
 
 	/* register audio system: */
 	return snd_card_register(line6->card);
@@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
 */
 static int toneport_reset_resume(struct usb_interface *interface)
 {
-	toneport_setup(usb_get_intfdata(interface));
+	int err;
+
+	err = toneport_setup(usb_get_intfdata(interface));
+	if (err)
+		return err;
 	return line6_resume(interface);
 }
 #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-27 18:42 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2019-04-27 18:42 UTC (permalink / raw)
  To: Alan Stern; +Cc: Christo Gouws, linux-usb

On Sat, Apr 27, 2019 at 08:24:32PM +0200, Greg KH wrote:
> On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > > On Sat, 27 Apr 2019, Greg KH wrote:
> > > 
> > > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > > Hi,
> > > > > 
> > > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > > the following crash in dmesg on Ubuntu 18.04
> > > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > > 
> > > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > > seems to happen (using liveCD).
> > > > > 
> > > > > 
> > > > > The output on the card seems to work, but none of the inputs work.
> > > > > 
> > > > > I've also now tested with latest kernel available on Arch Linux
> > > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > > 2019 x86_64 GNU/Linux
> > > > > 
> > > > > After some further testing, I found that this issue cropped in beween
> > > > > v4.8.17 and v4.9-rc1.
> > > > > 
> > > > > v4.8.17   - Works fine.
> > > > > v4.9-rc1+  - Produces crash
> > > > 
> > > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > > the failure?
> > > 
> > > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.  
> > > That routine passes an invalid buffer to usb_control_message().  
> > > Instead it should allocate its own buffer for the USB transfer and then
> > > copy the value to the caller's buffer.
> > > 
> > > There is a similar problem in line6_write_data().  Furthermore, both
> > > routines do DMA to/from a buffer on the stack.
> > 
> > I have an old patch in my local tree for the dma buffer on the stack
> > issue, it's below.  I should clean it up and send it correctly one of
> > these days :)
> 
> But, in reading your response, it doesn't fix the reported issue here.
> Let me go audit the whole driver and fix it up and add it to my original
> patch...

Ok, here's a patch that should be "complete".

Christo, can you test this out and let us know if it fixes the issue for
you or not?

thanks,

greg k-h

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

From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 23 Jan 2019 11:01:46 +0100
Subject: [PATCH] sound: USB: line6: use dynamic buffers

The line6 driver uses a lot of USB buffers off of the stack, which is
not allowed on many systems.  Fix this up by dynamically allocating the
buffers with kmalloc() which allows for proper DMA-able memory.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable <stable@vger.kernel.org>

---
 sound/usb/line6/driver.c   |   60 ++++++++++++++++++++++++++-------------------
 sound/usb/line6/podhd.c    |   21 +++++++++------
 sound/usb/line6/toneport.c |   23 ++++++++++++-----
 3 files changed, 64 insertions(+), 40 deletions(-)

--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -351,12 +351,16 @@ int line6_read_data(struct usb_line6 *li
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	unsigned char len;
+	unsigned char *len;
 	unsigned count;
 
 	if (address > 0xffff || datalen > 0xff)
 		return -EINVAL;
 
+	len = kmalloc(sizeof(*len), GFP_KERNEL);
+	if (!len)
+		return -ENOMEM;
+
 	/* query the serial number: */
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
 			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
@@ -365,7 +369,7 @@ int line6_read_data(struct usb_line6 *li
 
 	if (ret < 0) {
 		dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	/* Wait for data length. We'll get 0xff until length arrives. */
@@ -375,28 +379,29 @@ int line6_read_data(struct usb_line6 *li
 		ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
 				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
 				      USB_DIR_IN,
-				      0x0012, 0x0000, &len, 1,
+				      0x0012, 0x0000, len, 1,
 				      LINE6_TIMEOUT * HZ);
 		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receive length failed (error %d)\n", ret);
-			return ret;
+			goto exit;
 		}
 
-		if (len != 0xff)
+		if (*len != 0xff)
 			break;
 	}
 
-	if (len == 0xff) {
+	ret = -EIO;
+	if (*len == 0xff) {
 		dev_err(line6->ifcdev, "read failed after %d retries\n",
 			count);
-		return -EIO;
-	} else if (len != datalen) {
+		goto exit;
+	} else if (*len != datalen) {
 		/* should be equal or something went wrong */
 		dev_err(line6->ifcdev,
 			"length mismatch (expected %d, got %d)\n",
-			(int)datalen, (int)len);
-		return -EIO;
+			(int)datalen, (int)*len);
+		goto exit;
 	}
 
 	/* receive the result: */
@@ -405,12 +410,12 @@ int line6_read_data(struct usb_line6 *li
 			      0x0013, 0x0000, data, datalen,
 			      LINE6_TIMEOUT * HZ);
 
-	if (ret < 0) {
+	if (ret < 0)
 		dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
-		return ret;
-	}
 
-	return 0;
+exit:
+	kfree(len);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_read_data);
 
@@ -422,12 +427,16 @@ int line6_write_data(struct usb_line6 *l
 {
 	struct usb_device *usbdev = line6->usbdev;
 	int ret;
-	unsigned char status;
+	unsigned char *status;
 	int count;
 
 	if (address > 0xffff || datalen > 0xffff)
 		return -EINVAL;
 
+	status = kmalloc(sizeof(*status), GFP_KERNEL);
+	if (!status)
+		return -ENOMEM;
+
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
 			      USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 			      0x0022, address, data, datalen,
@@ -436,7 +445,7 @@ int line6_write_data(struct usb_line6 *l
 	if (ret < 0) {
 		dev_err(line6->ifcdev,
 			"write request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
@@ -447,28 +456,29 @@ int line6_write_data(struct usb_line6 *l
 				      USB_TYPE_VENDOR | USB_RECIP_DEVICE |
 				      USB_DIR_IN,
 				      0x0012, 0x0000,
-				      &status, 1, LINE6_TIMEOUT * HZ);
+				      status, 1, LINE6_TIMEOUT * HZ);
 
 		if (ret < 0) {
 			dev_err(line6->ifcdev,
 				"receiving status failed (error %d)\n", ret);
-			return ret;
+			goto exit;
 		}
 
-		if (status != 0xff)
+		if (*status != 0xff)
 			break;
 	}
 
-	if (status == 0xff) {
+	if (*status == 0xff) {
 		dev_err(line6->ifcdev, "write failed after %d retries\n",
 			count);
-		return -EIO;
-	} else if (status != 0) {
+		ret = -EIO;
+	} else if (*status != 0) {
 		dev_err(line6->ifcdev, "write failed (error %d)\n", ret);
-		return -EIO;
+		ret = -EIO;
 	}
-
-	return 0;
+exit:
+	kfree(status);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(line6_write_data);
 
--- a/sound/usb/line6/podhd.c
+++ b/sound/usb/line6/podhd.c
@@ -225,28 +225,32 @@ static void podhd_startup_start_workqueu
 static int podhd_dev_start(struct usb_line6_podhd *pod)
 {
 	int ret;
-	u8 init_bytes[8];
+	u8 *init_bytes;
 	int i;
 	struct usb_device *usbdev = pod->line6.usbdev;
 
+	init_bytes = kmalloc(8, GFP_KERNEL);
+	if (!init_bytes)
+		return -ENOMEM;
+
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
 					0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
 					0x11, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
 	if (ret < 0) {
 		dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	/* NOTE: looks like some kind of ping message */
 	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
 					USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
 					0x11, 0x0,
-					&init_bytes, 3, LINE6_TIMEOUT * HZ);
+					init_bytes, 3, LINE6_TIMEOUT * HZ);
 	if (ret < 0) {
 		dev_err(pod->line6.ifcdev,
 			"receive length failed (error %d)\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	pod->firmware_version =
@@ -255,7 +259,7 @@ static int podhd_dev_start(struct usb_li
 	for (i = 0; i <= 16; i++) {
 		ret = line6_read_data(&pod->line6, 0xf000 + 0x08 * i, init_bytes, 8);
 		if (ret < 0)
-			return ret;
+			goto exit;
 	}
 
 	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
@@ -263,10 +267,9 @@ static int podhd_dev_start(struct usb_li
 					USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
 					1, 0,
 					NULL, 0, LINE6_TIMEOUT * HZ);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+exit:
+	kfree(init_bytes);
+	return ret;
 }
 
 static void podhd_startup_workqueue(struct work_struct *work)
--- a/sound/usb/line6/toneport.c
+++ b/sound/usb/line6/toneport.c
@@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
 /*
 	Setup Toneport device.
 */
-static void toneport_setup(struct usb_line6_toneport *toneport)
+static int toneport_setup(struct usb_line6_toneport *toneport)
 {
-	u32 ticks;
+	u32 *ticks;
 	struct usb_line6 *line6 = &toneport->line6;
 	struct usb_device *usbdev = line6->usbdev;
 
+	ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
+	if (!ticks)
+		return -ENOMEM;
+
 	/* sync time on device with host: */
 	/* note: 32-bit timestamps overflow in year 2106 */
-	ticks = (u32)ktime_get_real_seconds();
-	line6_write_data(line6, 0x80c6, &ticks, 4);
+	*ticks = (u32)ktime_get_real_seconds();
+	line6_write_data(line6, 0x80c6, ticks, 4);
+	kfree(ticks);
 
 	/* enable device: */
 	toneport_send_cmd(usbdev, 0x0301, 0x0000);
@@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
 			return err;
 	}
 
-	toneport_setup(toneport);
+	err = toneport_setup(toneport);
+	if (err)
+		return err;
 
 	/* register audio system: */
 	return snd_card_register(line6->card);
@@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
 */
 static int toneport_reset_resume(struct usb_interface *interface)
 {
-	toneport_setup(usb_get_intfdata(interface));
+	int err;
+
+	err = toneport_setup(usb_get_intfdata(interface));
+	if (err)
+		return err;
 	return line6_resume(interface);
 }
 #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-28 10:50 ` Christo Gouws
  0 siblings, 0 replies; 13+ messages in thread
From: Christo Gouws @ 2019-04-28 10:50 UTC (permalink / raw)
  To: Greg KH; +Cc: Alan Stern, linux-usb

Hi,

Confirmed issue is fixed by patch.

Thank you so much!

Kind Regards,
Christo Gouws

On Sat, Apr 27, 2019 at 8:42 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Apr 27, 2019 at 08:24:32PM +0200, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> > > On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > > > On Sat, 27 Apr 2019, Greg KH wrote:
> > > >
> > > > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > > > the following crash in dmesg on Ubuntu 18.04
> > > > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > > >
> > > > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > > > seems to happen (using liveCD).
> > > > > >
> > > > > >
> > > > > > The output on the card seems to work, but none of the inputs work.
> > > > > >
> > > > > > I've also now tested with latest kernel available on Arch Linux
> > > > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > > > 2019 x86_64 GNU/Linux
> > > > > >
> > > > > > After some further testing, I found that this issue cropped in beween
> > > > > > v4.8.17 and v4.9-rc1.
> > > > > >
> > > > > > v4.8.17   - Works fine.
> > > > > > v4.9-rc1+  - Produces crash
> > > > >
> > > > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > > > the failure?
> > > >
> > > > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.
> > > > That routine passes an invalid buffer to usb_control_message().
> > > > Instead it should allocate its own buffer for the USB transfer and then
> > > > copy the value to the caller's buffer.
> > > >
> > > > There is a similar problem in line6_write_data().  Furthermore, both
> > > > routines do DMA to/from a buffer on the stack.
> > >
> > > I have an old patch in my local tree for the dma buffer on the stack
> > > issue, it's below.  I should clean it up and send it correctly one of
> > > these days :)
> >
> > But, in reading your response, it doesn't fix the reported issue here.
> > Let me go audit the whole driver and fix it up and add it to my original
> > patch...
>
> Ok, here's a patch that should be "complete".
>
> Christo, can you test this out and let us know if it fixes the issue for
> you or not?
>
> thanks,
>
> greg k-h
>
> ---------------
>
> From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Wed, 23 Jan 2019 11:01:46 +0100
> Subject: [PATCH] sound: USB: line6: use dynamic buffers
>
> The line6 driver uses a lot of USB buffers off of the stack, which is
> not allowed on many systems.  Fix this up by dynamically allocating the
> buffers with kmalloc() which allows for proper DMA-able memory.
>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable <stable@vger.kernel.org>
>
> ---
>  sound/usb/line6/driver.c   |   60 ++++++++++++++++++++++++++-------------------
>  sound/usb/line6/podhd.c    |   21 +++++++++------
>  sound/usb/line6/toneport.c |   23 ++++++++++++-----
>  3 files changed, 64 insertions(+), 40 deletions(-)
>
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -351,12 +351,16 @@ int line6_read_data(struct usb_line6 *li
>  {
>         struct usb_device *usbdev = line6->usbdev;
>         int ret;
> -       unsigned char len;
> +       unsigned char *len;
>         unsigned count;
>
>         if (address > 0xffff || datalen > 0xff)
>                 return -EINVAL;
>
> +       len = kmalloc(sizeof(*len), GFP_KERNEL);
> +       if (!len)
> +               return -ENOMEM;
> +
>         /* query the serial number: */
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
>                               USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> @@ -365,7 +369,7 @@ int line6_read_data(struct usb_line6 *li
>
>         if (ret < 0) {
>                 dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         /* Wait for data length. We'll get 0xff until length arrives. */
> @@ -375,28 +379,29 @@ int line6_read_data(struct usb_line6 *li
>                 ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
>                                       USB_TYPE_VENDOR | USB_RECIP_DEVICE |
>                                       USB_DIR_IN,
> -                                     0x0012, 0x0000, &len, 1,
> +                                     0x0012, 0x0000, len, 1,
>                                       LINE6_TIMEOUT * HZ);
>                 if (ret < 0) {
>                         dev_err(line6->ifcdev,
>                                 "receive length failed (error %d)\n", ret);
> -                       return ret;
> +                       goto exit;
>                 }
>
> -               if (len != 0xff)
> +               if (*len != 0xff)
>                         break;
>         }
>
> -       if (len == 0xff) {
> +       ret = -EIO;
> +       if (*len == 0xff) {
>                 dev_err(line6->ifcdev, "read failed after %d retries\n",
>                         count);
> -               return -EIO;
> -       } else if (len != datalen) {
> +               goto exit;
> +       } else if (*len != datalen) {
>                 /* should be equal or something went wrong */
>                 dev_err(line6->ifcdev,
>                         "length mismatch (expected %d, got %d)\n",
> -                       (int)datalen, (int)len);
> -               return -EIO;
> +                       (int)datalen, (int)*len);
> +               goto exit;
>         }
>
>         /* receive the result: */
> @@ -405,12 +410,12 @@ int line6_read_data(struct usb_line6 *li
>                               0x0013, 0x0000, data, datalen,
>                               LINE6_TIMEOUT * HZ);
>
> -       if (ret < 0) {
> +       if (ret < 0)
>                 dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
> -               return ret;
> -       }
>
> -       return 0;
> +exit:
> +       kfree(len);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(line6_read_data);
>
> @@ -422,12 +427,16 @@ int line6_write_data(struct usb_line6 *l
>  {
>         struct usb_device *usbdev = line6->usbdev;
>         int ret;
> -       unsigned char status;
> +       unsigned char *status;
>         int count;
>
>         if (address > 0xffff || datalen > 0xffff)
>                 return -EINVAL;
>
> +       status = kmalloc(sizeof(*status), GFP_KERNEL);
> +       if (!status)
> +               return -ENOMEM;
> +
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
>                               USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
>                               0x0022, address, data, datalen,
> @@ -436,7 +445,7 @@ int line6_write_data(struct usb_line6 *l
>         if (ret < 0) {
>                 dev_err(line6->ifcdev,
>                         "write request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> @@ -447,28 +456,29 @@ int line6_write_data(struct usb_line6 *l
>                                       USB_TYPE_VENDOR | USB_RECIP_DEVICE |
>                                       USB_DIR_IN,
>                                       0x0012, 0x0000,
> -                                     &status, 1, LINE6_TIMEOUT * HZ);
> +                                     status, 1, LINE6_TIMEOUT * HZ);
>
>                 if (ret < 0) {
>                         dev_err(line6->ifcdev,
>                                 "receiving status failed (error %d)\n", ret);
> -                       return ret;
> +                       goto exit;
>                 }
>
> -               if (status != 0xff)
> +               if (*status != 0xff)
>                         break;
>         }
>
> -       if (status == 0xff) {
> +       if (*status == 0xff) {
>                 dev_err(line6->ifcdev, "write failed after %d retries\n",
>                         count);
> -               return -EIO;
> -       } else if (status != 0) {
> +               ret = -EIO;
> +       } else if (*status != 0) {
>                 dev_err(line6->ifcdev, "write failed (error %d)\n", ret);
> -               return -EIO;
> +               ret = -EIO;
>         }
> -
> -       return 0;
> +exit:
> +       kfree(status);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(line6_write_data);
>
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -225,28 +225,32 @@ static void podhd_startup_start_workqueu
>  static int podhd_dev_start(struct usb_line6_podhd *pod)
>  {
>         int ret;
> -       u8 init_bytes[8];
> +       u8 *init_bytes;
>         int i;
>         struct usb_device *usbdev = pod->line6.usbdev;
>
> +       init_bytes = kmalloc(8, GFP_KERNEL);
> +       if (!init_bytes)
> +               return -ENOMEM;
> +
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
>                                         0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
>                                         0x11, 0,
>                                         NULL, 0, LINE6_TIMEOUT * HZ);
>         if (ret < 0) {
>                 dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         /* NOTE: looks like some kind of ping message */
>         ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
>                                         USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
>                                         0x11, 0x0,
> -                                       &init_bytes, 3, LINE6_TIMEOUT * HZ);
> +                                       init_bytes, 3, LINE6_TIMEOUT * HZ);
>         if (ret < 0) {
>                 dev_err(pod->line6.ifcdev,
>                         "receive length failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         pod->firmware_version =
> @@ -255,7 +259,7 @@ static int podhd_dev_start(struct usb_li
>         for (i = 0; i <= 16; i++) {
>                 ret = line6_read_data(&pod->line6, 0xf000 + 0x08 * i, init_bytes, 8);
>                 if (ret < 0)
> -                       return ret;
> +                       goto exit;
>         }
>
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> @@ -263,10 +267,9 @@ static int podhd_dev_start(struct usb_li
>                                         USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
>                                         1, 0,
>                                         NULL, 0, LINE6_TIMEOUT * HZ);
> -       if (ret < 0)
> -               return ret;
> -
> -       return 0;
> +exit:
> +       kfree(init_bytes);
> +       return ret;
>  }
>
>  static void podhd_startup_workqueue(struct work_struct *work)
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
>  /*
>         Setup Toneport device.
>  */
> -static void toneport_setup(struct usb_line6_toneport *toneport)
> +static int toneport_setup(struct usb_line6_toneport *toneport)
>  {
> -       u32 ticks;
> +       u32 *ticks;
>         struct usb_line6 *line6 = &toneport->line6;
>         struct usb_device *usbdev = line6->usbdev;
>
> +       ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
> +       if (!ticks)
> +               return -ENOMEM;
> +
>         /* sync time on device with host: */
>         /* note: 32-bit timestamps overflow in year 2106 */
> -       ticks = (u32)ktime_get_real_seconds();
> -       line6_write_data(line6, 0x80c6, &ticks, 4);
> +       *ticks = (u32)ktime_get_real_seconds();
> +       line6_write_data(line6, 0x80c6, ticks, 4);
> +       kfree(ticks);
>
>         /* enable device: */
>         toneport_send_cmd(usbdev, 0x0301, 0x0000);
> @@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
>                         return err;
>         }
>
> -       toneport_setup(toneport);
> +       err = toneport_setup(toneport);
> +       if (err)
> +               return err;
>
>         /* register audio system: */
>         return snd_card_register(line6->card);
> @@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
>  */
>  static int toneport_reset_resume(struct usb_interface *interface)
>  {
> -       toneport_setup(usb_get_intfdata(interface));
> +       int err;
> +
> +       err = toneport_setup(usb_get_intfdata(interface));
> +       if (err)
> +               return err;
>         return line6_resume(interface);
>  }
>  #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-28 10:50 ` Christo Gouws
  0 siblings, 0 replies; 13+ messages in thread
From: Christo Gouws @ 2019-04-28 10:50 UTC (permalink / raw)
  To: Greg KH; +Cc: Alan Stern, linux-usb

Hi,

Confirmed issue is fixed by patch.

Thank you so much!

Kind Regards,
Christo Gouws

On Sat, Apr 27, 2019 at 8:42 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Apr 27, 2019 at 08:24:32PM +0200, Greg KH wrote:
> > On Sat, Apr 27, 2019 at 08:07:28PM +0200, Greg KH wrote:
> > > On Sat, Apr 27, 2019 at 11:34:03AM -0400, Alan Stern wrote:
> > > > On Sat, 27 Apr 2019, Greg KH wrote:
> > > >
> > > > > On Fri, Apr 26, 2019 at 11:50:14AM +0200, Christo Gouws wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I have a Line6 Pod Studio UX1 card, but each time I plug it in, I get
> > > > > > the following crash in dmesg on Ubuntu 18.04
> > > > > > Linux my-pc 4.20.8-042008-generic #201902121544 SMP Tue Feb 12
> > > > > > 20:46:50 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
> > > > > >
> > > > > > I've also tested this with a Fedora 30 v5.0.6-300 kernel, but still
> > > > > > seems to happen (using liveCD).
> > > > > >
> > > > > >
> > > > > > The output on the card seems to work, but none of the inputs work.
> > > > > >
> > > > > > I've also now tested with latest kernel available on Arch Linux
> > > > > > Linux my-pc 5.0.9-arch1-1-ARCH #1 SMP PREEMPT Sat Apr 20 15:00:46 UTC
> > > > > > 2019 x86_64 GNU/Linux
> > > > > >
> > > > > > After some further testing, I found that this issue cropped in beween
> > > > > > v4.8.17 and v4.9-rc1.
> > > > > >
> > > > > > v4.8.17   - Works fine.
> > > > > > v4.9-rc1+  - Produces crash
> > > > >
> > > > > Any chance you can use 'git bisect' to find the exact commit that caused
> > > > > the failure?
> > > >
> > > > No need.  The bug is in line6_read_data() in sound/usb/line6/driver.c.
> > > > That routine passes an invalid buffer to usb_control_message().
> > > > Instead it should allocate its own buffer for the USB transfer and then
> > > > copy the value to the caller's buffer.
> > > >
> > > > There is a similar problem in line6_write_data().  Furthermore, both
> > > > routines do DMA to/from a buffer on the stack.
> > >
> > > I have an old patch in my local tree for the dma buffer on the stack
> > > issue, it's below.  I should clean it up and send it correctly one of
> > > these days :)
> >
> > But, in reading your response, it doesn't fix the reported issue here.
> > Let me go audit the whole driver and fix it up and add it to my original
> > patch...
>
> Ok, here's a patch that should be "complete".
>
> Christo, can you test this out and let us know if it fixes the issue for
> you or not?
>
> thanks,
>
> greg k-h
>
> ---------------
>
> From e2c743d1f900135c3e560cd9ea1647e4a1ebce7a Mon Sep 17 00:00:00 2001
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Wed, 23 Jan 2019 11:01:46 +0100
> Subject: [PATCH] sound: USB: line6: use dynamic buffers
>
> The line6 driver uses a lot of USB buffers off of the stack, which is
> not allowed on many systems.  Fix this up by dynamically allocating the
> buffers with kmalloc() which allows for proper DMA-able memory.
>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable <stable@vger.kernel.org>
>
> ---
>  sound/usb/line6/driver.c   |   60 ++++++++++++++++++++++++++-------------------
>  sound/usb/line6/podhd.c    |   21 +++++++++------
>  sound/usb/line6/toneport.c |   23 ++++++++++++-----
>  3 files changed, 64 insertions(+), 40 deletions(-)
>
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -351,12 +351,16 @@ int line6_read_data(struct usb_line6 *li
>  {
>         struct usb_device *usbdev = line6->usbdev;
>         int ret;
> -       unsigned char len;
> +       unsigned char *len;
>         unsigned count;
>
>         if (address > 0xffff || datalen > 0xff)
>                 return -EINVAL;
>
> +       len = kmalloc(sizeof(*len), GFP_KERNEL);
> +       if (!len)
> +               return -ENOMEM;
> +
>         /* query the serial number: */
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
>                               USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> @@ -365,7 +369,7 @@ int line6_read_data(struct usb_line6 *li
>
>         if (ret < 0) {
>                 dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         /* Wait for data length. We'll get 0xff until length arrives. */
> @@ -375,28 +379,29 @@ int line6_read_data(struct usb_line6 *li
>                 ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
>                                       USB_TYPE_VENDOR | USB_RECIP_DEVICE |
>                                       USB_DIR_IN,
> -                                     0x0012, 0x0000, &len, 1,
> +                                     0x0012, 0x0000, len, 1,
>                                       LINE6_TIMEOUT * HZ);
>                 if (ret < 0) {
>                         dev_err(line6->ifcdev,
>                                 "receive length failed (error %d)\n", ret);
> -                       return ret;
> +                       goto exit;
>                 }
>
> -               if (len != 0xff)
> +               if (*len != 0xff)
>                         break;
>         }
>
> -       if (len == 0xff) {
> +       ret = -EIO;
> +       if (*len == 0xff) {
>                 dev_err(line6->ifcdev, "read failed after %d retries\n",
>                         count);
> -               return -EIO;
> -       } else if (len != datalen) {
> +               goto exit;
> +       } else if (*len != datalen) {
>                 /* should be equal or something went wrong */
>                 dev_err(line6->ifcdev,
>                         "length mismatch (expected %d, got %d)\n",
> -                       (int)datalen, (int)len);
> -               return -EIO;
> +                       (int)datalen, (int)*len);
> +               goto exit;
>         }
>
>         /* receive the result: */
> @@ -405,12 +410,12 @@ int line6_read_data(struct usb_line6 *li
>                               0x0013, 0x0000, data, datalen,
>                               LINE6_TIMEOUT * HZ);
>
> -       if (ret < 0) {
> +       if (ret < 0)
>                 dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
> -               return ret;
> -       }
>
> -       return 0;
> +exit:
> +       kfree(len);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(line6_read_data);
>
> @@ -422,12 +427,16 @@ int line6_write_data(struct usb_line6 *l
>  {
>         struct usb_device *usbdev = line6->usbdev;
>         int ret;
> -       unsigned char status;
> +       unsigned char *status;
>         int count;
>
>         if (address > 0xffff || datalen > 0xffff)
>                 return -EINVAL;
>
> +       status = kmalloc(sizeof(*status), GFP_KERNEL);
> +       if (!status)
> +               return -ENOMEM;
> +
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
>                               USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
>                               0x0022, address, data, datalen,
> @@ -436,7 +445,7 @@ int line6_write_data(struct usb_line6 *l
>         if (ret < 0) {
>                 dev_err(line6->ifcdev,
>                         "write request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
> @@ -447,28 +456,29 @@ int line6_write_data(struct usb_line6 *l
>                                       USB_TYPE_VENDOR | USB_RECIP_DEVICE |
>                                       USB_DIR_IN,
>                                       0x0012, 0x0000,
> -                                     &status, 1, LINE6_TIMEOUT * HZ);
> +                                     status, 1, LINE6_TIMEOUT * HZ);
>
>                 if (ret < 0) {
>                         dev_err(line6->ifcdev,
>                                 "receiving status failed (error %d)\n", ret);
> -                       return ret;
> +                       goto exit;
>                 }
>
> -               if (status != 0xff)
> +               if (*status != 0xff)
>                         break;
>         }
>
> -       if (status == 0xff) {
> +       if (*status == 0xff) {
>                 dev_err(line6->ifcdev, "write failed after %d retries\n",
>                         count);
> -               return -EIO;
> -       } else if (status != 0) {
> +               ret = -EIO;
> +       } else if (*status != 0) {
>                 dev_err(line6->ifcdev, "write failed (error %d)\n", ret);
> -               return -EIO;
> +               ret = -EIO;
>         }
> -
> -       return 0;
> +exit:
> +       kfree(status);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(line6_write_data);
>
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -225,28 +225,32 @@ static void podhd_startup_start_workqueu
>  static int podhd_dev_start(struct usb_line6_podhd *pod)
>  {
>         int ret;
> -       u8 init_bytes[8];
> +       u8 *init_bytes;
>         int i;
>         struct usb_device *usbdev = pod->line6.usbdev;
>
> +       init_bytes = kmalloc(8, GFP_KERNEL);
> +       if (!init_bytes)
> +               return -ENOMEM;
> +
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
>                                         0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
>                                         0x11, 0,
>                                         NULL, 0, LINE6_TIMEOUT * HZ);
>         if (ret < 0) {
>                 dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         /* NOTE: looks like some kind of ping message */
>         ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
>                                         USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
>                                         0x11, 0x0,
> -                                       &init_bytes, 3, LINE6_TIMEOUT * HZ);
> +                                       init_bytes, 3, LINE6_TIMEOUT * HZ);
>         if (ret < 0) {
>                 dev_err(pod->line6.ifcdev,
>                         "receive length failed (error %d)\n", ret);
> -               return ret;
> +               goto exit;
>         }
>
>         pod->firmware_version =
> @@ -255,7 +259,7 @@ static int podhd_dev_start(struct usb_li
>         for (i = 0; i <= 16; i++) {
>                 ret = line6_read_data(&pod->line6, 0xf000 + 0x08 * i, init_bytes, 8);
>                 if (ret < 0)
> -                       return ret;
> +                       goto exit;
>         }
>
>         ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> @@ -263,10 +267,9 @@ static int podhd_dev_start(struct usb_li
>                                         USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT,
>                                         1, 0,
>                                         NULL, 0, LINE6_TIMEOUT * HZ);
> -       if (ret < 0)
> -               return ret;
> -
> -       return 0;
> +exit:
> +       kfree(init_bytes);
> +       return ret;
>  }
>
>  static void podhd_startup_workqueue(struct work_struct *work)
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -365,16 +365,21 @@ static bool toneport_has_source_select(s
>  /*
>         Setup Toneport device.
>  */
> -static void toneport_setup(struct usb_line6_toneport *toneport)
> +static int toneport_setup(struct usb_line6_toneport *toneport)
>  {
> -       u32 ticks;
> +       u32 *ticks;
>         struct usb_line6 *line6 = &toneport->line6;
>         struct usb_device *usbdev = line6->usbdev;
>
> +       ticks = kmalloc(sizeof(*ticks), GFP_KERNEL);
> +       if (!ticks)
> +               return -ENOMEM;
> +
>         /* sync time on device with host: */
>         /* note: 32-bit timestamps overflow in year 2106 */
> -       ticks = (u32)ktime_get_real_seconds();
> -       line6_write_data(line6, 0x80c6, &ticks, 4);
> +       *ticks = (u32)ktime_get_real_seconds();
> +       line6_write_data(line6, 0x80c6, ticks, 4);
> +       kfree(ticks);
>
>         /* enable device: */
>         toneport_send_cmd(usbdev, 0x0301, 0x0000);
> @@ -451,7 +456,9 @@ static int toneport_init(struct usb_line
>                         return err;
>         }
>
> -       toneport_setup(toneport);
> +       err = toneport_setup(toneport);
> +       if (err)
> +               return err;
>
>         /* register audio system: */
>         return snd_card_register(line6->card);
> @@ -463,7 +470,11 @@ static int toneport_init(struct usb_line
>  */
>  static int toneport_reset_resume(struct usb_interface *interface)
>  {
> -       toneport_setup(usb_get_intfdata(interface));
> +       int err;
> +
> +       err = toneport_setup(usb_get_intfdata(interface));
> +       if (err)
> +               return err;
>         return line6_resume(interface);
>  }
>  #endif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-28 16:00 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-28 16:00 UTC (permalink / raw)
  To: Christo Gouws; +Cc: Alan Stern, linux-usb

On Sun, Apr 28, 2019 at 12:50:04PM +0200, Christo Gouws wrote:
> Hi,
> 
> Confirmed issue is fixed by patch.

Wonderful, thanks for testing!  I'll go queue this up now, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma
@ 2019-04-28 16:00 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2019-04-28 16:00 UTC (permalink / raw)
  To: Christo Gouws; +Cc: Alan Stern, linux-usb

On Sun, Apr 28, 2019 at 12:50:04PM +0200, Christo Gouws wrote:
> Hi,
> 
> Confirmed issue is fixed by patch.

Wonderful, thanks for testing!  I'll go queue this up now, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-04-28 16:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-28 10:50 Line6 podstudio UX1 - driver crash on usb_hcd_map_urb_for_dma Christo Gouws
2019-04-28 10:50 ` Christo Gouws
  -- strict thread matches above, loose matches on Subject: below --
2019-04-28 16:00 Greg Kroah-Hartman
2019-04-28 16:00 ` Greg KH
2019-04-27 18:42 Greg Kroah-Hartman
2019-04-27 18:42 ` Greg KH
2019-04-27 18:24 Greg Kroah-Hartman
2019-04-27 18:24 ` Greg KH
2019-04-27 18:07 Greg Kroah-Hartman
2019-04-27 18:07 ` Greg KH
2019-04-26  9:50 Christo Gouws
2019-04-27  7:01 ` Greg KH
2019-04-27 15:34   ` Alan Stern

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.