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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxxxxxxx>
>
> ---
> 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
