| Message ID | 1532010470-20582-2-git-send-email-paolo.pisati@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | ALSA: usb-audio: Check out-of-bounds access by corrupted buffer descriptor | expand |
On 19.07.2018 16:27, Paolo Pisati wrote: > From: Takashi Iwai <tiwai@suse.de> > > When a USB-audio device receives a maliciously adjusted or corrupted > buffer descriptor, the USB-audio driver may access an out-of-bounce > value at its parser. This was detected by syzkaller, something like: > > BUG: KASAN: slab-out-of-bounds in usb_audio_probe+0x27b2/0x2ab0 > Read of size 1 at addr ffff88006b83a9e8 by task kworker/0:1/24 > CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc1-42251-gebb2c2437d80 #224 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Workqueue: usb_hub_wq hub_event > Call Trace: > __dump_stack lib/dump_stack.c:16 > dump_stack+0x292/0x395 lib/dump_stack.c:52 > print_address_description+0x78/0x280 mm/kasan/report.c:252 > kasan_report_error mm/kasan/report.c:351 > kasan_report+0x22f/0x340 mm/kasan/report.c:409 > __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427 > snd_usb_create_streams sound/usb/card.c:248 > usb_audio_probe+0x27b2/0x2ab0 sound/usb/card.c:605 > usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932 > generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174 > usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457 > hub_port_connect drivers/usb/core/hub.c:4903 > hub_port_connect_change drivers/usb/core/hub.c:5009 > port_event drivers/usb/core/hub.c:5115 > hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195 > process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 > worker_thread+0x221/0x1850 kernel/workqueue.c:2253 > kthread+0x3a1/0x470 kernel/kthread.c:231 > ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 > > This patch adds the checks of out-of-bounce accesses at appropriate > places and bails out when it goes out of the given buffer. > > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Tested-by: Andrey Konovalov <andreyknvl@google.com> > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > (cherry picked from commit bfc81a8bc18e3c4ba0cbaa7666ff76be2f998991) > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > sound/usb/card.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/sound/usb/card.c b/sound/usb/card.c > index ab433a0..9189dbc 100644 > --- a/sound/usb/card.c > +++ b/sound/usb/card.c > @@ -218,6 +218,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > struct usb_interface_descriptor *altsd; > void *control_header; > int i, protocol; > + int rest_bytes; > > /* find audiocontrol interface */ > host_iface = &usb_ifnum_to_if(dev, ctrlif)->altsetting[0]; > @@ -232,6 +233,15 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > return -EINVAL; > } > > + rest_bytes = (void *)(host_iface->extra + host_iface->extralen) - > + control_header; > + > + /* just to be sure -- this shouldn't hit at all */ > + if (rest_bytes <= 0) { > + dev_err(&dev->dev, "invalid control header\n"); > + return -EINVAL; > + } > + > switch (protocol) { > default: > snd_printdd(KERN_WARNING "unknown interface protocol %#02x, assuming v1\n", > @@ -241,11 +251,21 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > case UAC_VERSION_1: { > struct uac1_ac_header_descriptor *h1 = control_header; > > + if (rest_bytes < sizeof(*h1)) { > + dev_err(&dev->dev, "too short v1 buffer descriptor\n"); > + return -EINVAL; > + } > + > if (!h1->bInCollection) { > snd_printk(KERN_INFO "skipping empty audio interface (v1)\n"); > return -EINVAL; > } > > + if (rest_bytes < h1->bLength) { > + dev_err(&dev->dev, "invalid buffer length (v1)\n"); > + return -EINVAL; > + } > + > if (h1->bLength < sizeof(*h1) + h1->bInCollection) { > snd_printk(KERN_ERR "invalid UAC_HEADER (v1)\n"); > return -EINVAL; >
On 19/07/18 15:27, Paolo Pisati wrote: > From: Takashi Iwai <tiwai@suse.de> > > When a USB-audio device receives a maliciously adjusted or corrupted > buffer descriptor, the USB-audio driver may access an out-of-bounce > value at its parser. This was detected by syzkaller, something like: > > BUG: KASAN: slab-out-of-bounds in usb_audio_probe+0x27b2/0x2ab0 > Read of size 1 at addr ffff88006b83a9e8 by task kworker/0:1/24 > CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc1-42251-gebb2c2437d80 #224 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Workqueue: usb_hub_wq hub_event > Call Trace: > __dump_stack lib/dump_stack.c:16 > dump_stack+0x292/0x395 lib/dump_stack.c:52 > print_address_description+0x78/0x280 mm/kasan/report.c:252 > kasan_report_error mm/kasan/report.c:351 > kasan_report+0x22f/0x340 mm/kasan/report.c:409 > __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427 > snd_usb_create_streams sound/usb/card.c:248 > usb_audio_probe+0x27b2/0x2ab0 sound/usb/card.c:605 > usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932 > generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174 > usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457 > hub_port_connect drivers/usb/core/hub.c:4903 > hub_port_connect_change drivers/usb/core/hub.c:5009 > port_event drivers/usb/core/hub.c:5115 > hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195 > process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 > worker_thread+0x221/0x1850 kernel/workqueue.c:2253 > kthread+0x3a1/0x470 kernel/kthread.c:231 > ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 > > This patch adds the checks of out-of-bounce accesses at appropriate > places and bails out when it goes out of the given buffer. > > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Tested-by: Andrey Konovalov <andreyknvl@google.com> > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > (cherry picked from commit bfc81a8bc18e3c4ba0cbaa7666ff76be2f998991) > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> > --- > sound/usb/card.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/sound/usb/card.c b/sound/usb/card.c > index ab433a0..9189dbc 100644 > --- a/sound/usb/card.c > +++ b/sound/usb/card.c > @@ -218,6 +218,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > struct usb_interface_descriptor *altsd; > void *control_header; > int i, protocol; > + int rest_bytes; > > /* find audiocontrol interface */ > host_iface = &usb_ifnum_to_if(dev, ctrlif)->altsetting[0]; > @@ -232,6 +233,15 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > return -EINVAL; > } > > + rest_bytes = (void *)(host_iface->extra + host_iface->extralen) - > + control_header; > + > + /* just to be sure -- this shouldn't hit at all */ > + if (rest_bytes <= 0) { > + dev_err(&dev->dev, "invalid control header\n"); > + return -EINVAL; > + } > + > switch (protocol) { > default: > snd_printdd(KERN_WARNING "unknown interface protocol %#02x, assuming v1\n", > @@ -241,11 +251,21 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > case UAC_VERSION_1: { > struct uac1_ac_header_descriptor *h1 = control_header; > > + if (rest_bytes < sizeof(*h1)) { > + dev_err(&dev->dev, "too short v1 buffer descriptor\n"); > + return -EINVAL; > + } > + > if (!h1->bInCollection) { > snd_printk(KERN_INFO "skipping empty audio interface (v1)\n"); > return -EINVAL; > } > > + if (rest_bytes < h1->bLength) { > + dev_err(&dev->dev, "invalid buffer length (v1)\n"); > + return -EINVAL; > + } > + > if (h1->bLength < sizeof(*h1) + h1->bInCollection) { > snd_printk(KERN_ERR "invalid UAC_HEADER (v1)\n"); > return -EINVAL; > Looks OK to me. Acked-by: Colin Ian King <colin.king@canonical.com>
On 07/19/2018 04:27 PM, Paolo Pisati wrote: > From: Takashi Iwai <tiwai@suse.de> CVE-2017-16529 > When a USB-audio device receives a maliciously adjusted or corrupted > buffer descriptor, the USB-audio driver may access an out-of-bounce > value at its parser. This was detected by syzkaller, something like: > > BUG: KASAN: slab-out-of-bounds in usb_audio_probe+0x27b2/0x2ab0 > Read of size 1 at addr ffff88006b83a9e8 by task kworker/0:1/24 > CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc1-42251-gebb2c2437d80 #224 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Workqueue: usb_hub_wq hub_event > Call Trace: > __dump_stack lib/dump_stack.c:16 > dump_stack+0x292/0x395 lib/dump_stack.c:52 > print_address_description+0x78/0x280 mm/kasan/report.c:252 > kasan_report_error mm/kasan/report.c:351 > kasan_report+0x22f/0x340 mm/kasan/report.c:409 > __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427 > snd_usb_create_streams sound/usb/card.c:248 > usb_audio_probe+0x27b2/0x2ab0 sound/usb/card.c:605 > usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932 > generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174 > usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266 > really_probe drivers/base/dd.c:413 > driver_probe_device+0x610/0xa00 drivers/base/dd.c:557 > __device_attach_driver+0x230/0x290 drivers/base/dd.c:653 > bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463 > __device_attach+0x26e/0x3d0 drivers/base/dd.c:710 > device_initial_probe+0x1f/0x30 drivers/base/dd.c:757 > bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523 > device_add+0xd0b/0x1660 drivers/base/core.c:1835 > usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457 > hub_port_connect drivers/usb/core/hub.c:4903 > hub_port_connect_change drivers/usb/core/hub.c:5009 > port_event drivers/usb/core/hub.c:5115 > hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195 > process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119 > worker_thread+0x221/0x1850 kernel/workqueue.c:2253 > kthread+0x3a1/0x470 kernel/kthread.c:231 > ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 > > This patch adds the checks of out-of-bounce accesses at appropriate > places and bails out when it goes out of the given buffer. > > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Tested-by: Andrey Konovalov <andreyknvl@google.com> > Cc: <stable@vger.kernel.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > (cherry picked from commit bfc81a8bc18e3c4ba0cbaa7666ff76be2f998991) > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> Acked-by: Juerg Haefliger <juergh@canonical.com> Applied to trusty master-next. ...Juerg > --- > sound/usb/card.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/sound/usb/card.c b/sound/usb/card.c > index ab433a0..9189dbc 100644 > --- a/sound/usb/card.c > +++ b/sound/usb/card.c > @@ -218,6 +218,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > struct usb_interface_descriptor *altsd; > void *control_header; > int i, protocol; > + int rest_bytes; > > /* find audiocontrol interface */ > host_iface = &usb_ifnum_to_if(dev, ctrlif)->altsetting[0]; > @@ -232,6 +233,15 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > return -EINVAL; > } > > + rest_bytes = (void *)(host_iface->extra + host_iface->extralen) - > + control_header; > + > + /* just to be sure -- this shouldn't hit at all */ > + if (rest_bytes <= 0) { > + dev_err(&dev->dev, "invalid control header\n"); > + return -EINVAL; > + } > + > switch (protocol) { > default: > snd_printdd(KERN_WARNING "unknown interface protocol %#02x, assuming v1\n", > @@ -241,11 +251,21 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) > case UAC_VERSION_1: { > struct uac1_ac_header_descriptor *h1 = control_header; > > + if (rest_bytes < sizeof(*h1)) { > + dev_err(&dev->dev, "too short v1 buffer descriptor\n"); > + return -EINVAL; > + } > + > if (!h1->bInCollection) { > snd_printk(KERN_INFO "skipping empty audio interface (v1)\n"); > return -EINVAL; > } > > + if (rest_bytes < h1->bLength) { > + dev_err(&dev->dev, "invalid buffer length (v1)\n"); > + return -EINVAL; > + } > + > if (h1->bLength < sizeof(*h1) + h1->bInCollection) { > snd_printk(KERN_ERR "invalid UAC_HEADER (v1)\n"); > return -EINVAL; >
diff --git a/sound/usb/card.c b/sound/usb/card.c index ab433a0..9189dbc 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -218,6 +218,7 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) struct usb_interface_descriptor *altsd; void *control_header; int i, protocol; + int rest_bytes; /* find audiocontrol interface */ host_iface = &usb_ifnum_to_if(dev, ctrlif)->altsetting[0]; @@ -232,6 +233,15 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) return -EINVAL; } + rest_bytes = (void *)(host_iface->extra + host_iface->extralen) - + control_header; + + /* just to be sure -- this shouldn't hit at all */ + if (rest_bytes <= 0) { + dev_err(&dev->dev, "invalid control header\n"); + return -EINVAL; + } + switch (protocol) { default: snd_printdd(KERN_WARNING "unknown interface protocol %#02x, assuming v1\n", @@ -241,11 +251,21 @@ static int snd_usb_create_streams(struct snd_usb_audio *chip, int ctrlif) case UAC_VERSION_1: { struct uac1_ac_header_descriptor *h1 = control_header; + if (rest_bytes < sizeof(*h1)) { + dev_err(&dev->dev, "too short v1 buffer descriptor\n"); + return -EINVAL; + } + if (!h1->bInCollection) { snd_printk(KERN_INFO "skipping empty audio interface (v1)\n"); return -EINVAL; } + if (rest_bytes < h1->bLength) { + dev_err(&dev->dev, "invalid buffer length (v1)\n"); + return -EINVAL; + } + if (h1->bLength < sizeof(*h1) + h1->bInCollection) { snd_printk(KERN_ERR "invalid UAC_HEADER (v1)\n"); return -EINVAL;