* [PATCH] ALSA: usb-audio: Fix out-of-bound error
@ 2017-11-15 8:07 Jaejoong Kim
2017-11-15 8:57 ` Jaejoong Kim
0 siblings, 1 reply; 4+ messages in thread
From: Jaejoong Kim @ 2017-11-15 8:07 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, stable; +Cc: alsa-devel, Jaejoong Kim
The snd_usb_copy_string_desc() retrieves the usb string corresponding to
the index number thought the usb_string(). And for NULL-terminated, insert
'0' by using the return value of usb_string() as the index of buffer to
hold the string.
The problem is that the usb_string() also returns the length of the string
read(>= 0), but it can also return a negative value, the error or status
value of usb_control_msg(). If iClockSource is '0' as shown below,
usb_string() will return -EINVAL. This will result in '0' being inserted
into buf[-22], and the following KASAN out-of-bound error message will be
output.
AudioControl Interface Descriptor:
bLength 8
bDescriptorType 36
bDescriptorSubtype 10 (CLOCK_SOURCE)
bClockID 1
bmAttributes 0x07 Internal programmable Clock (synced to SOF)
bmControls 0x07
Clock Frequency Control (read/write)
Clock Validity Control (read-only)
bAssocTerminal 0
iClockSource 0
To fix out-of-bound error, insert 0 only if the return value of
usb_string() is greater than 0.
==================================================================
BUG: KASAN: stack-out-of-bounds in parse_audio_unit+0x1327/0x1960 [snd_usb_audio]
Write of size 1 at addr ffff88007e66735a by task systemd-udevd/18376
CPU: 0 PID: 18376 Comm: systemd-udevd Not tainted 4.13.0+ #3
Hardware name: LG Electronics 15N540-RFLGL/White Tip Mountain, BIOS 15N5
Call Trace:
dump_stack+0x63/0x8d
print_address_description+0x70/0x290
? parse_audio_unit+0x1327/0x1960 [snd_usb_audio]
kasan_report+0x265/0x350
__asan_store1+0x4a/0x50
parse_audio_unit+0x1327/0x1960 [snd_usb_audio]
? save_stack+0xb5/0xd0
? save_stack_trace+0x1b/0x20
? save_stack+0x46/0xd0
? kasan_kmalloc+0xad/0xe0
? kmem_cache_alloc_trace+0xff/0x230
? snd_usb_create_mixer+0xb0/0x4b0 [snd_usb_audio]
? usb_audio_probe+0x4de/0xf40 [snd_usb_audio]
? usb_probe_interface+0x1f5/0x440
? driver_probe_device+0x3ed/0x660
? build_feature_ctl+0xb10/0xb10 [snd_usb_audio]
? save_stack_trace+0x1b/0x20
? init_object+0x69/0xa0
? snd_usb_find_csint_desc+0xa8/0xf0 [snd_usb_audio]
snd_usb_mixer_controls+0x1dc/0x370 [snd_usb_audio]
? build_audio_procunit+0x890/0x890 [snd_usb_audio]
? snd_usb_create_mixer+0xb0/0x4b0 [snd_usb_audio]
? kmem_cache_alloc_trace+0xff/0x230
? usb_ifnum_to_if+0xbd/0xf0
snd_usb_create_mixer+0x25b/0x4b0 [snd_usb_audio]
? snd_usb_create_stream+0x255/0x2c0 [snd_usb_audio]
usb_audio_probe+0x4de/0xf40 [snd_usb_audio]
? snd_usb_autosuspend.part.7+0x30/0x30 [snd_usb_audio]
? __pm_runtime_idle+0x90/0x90
? kernfs_activate+0xa6/0xc0
? usb_match_one_id_intf+0xdc/0x130
? __pm_runtime_set_status+0x2d4/0x450
usb_probe_interface+0x1f5/0x440
Cc: <stable@vger.kernel.org>
Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
The AudioControl Interface Descriptor in commit message is from
lsusb output with real usb audio DAC.
The usb audio product causing the OOB are as follows:
http://www.lg.com/uk/lg-friends/lg-AFD-1200
It only prints OOB error and usb audio works well. :)
sound/usb/mixer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index e630813..5a83c2c 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -204,7 +204,8 @@ static int snd_usb_copy_string_desc(struct mixer_build *state,
int index, char *buf, int maxlen)
{
int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
- buf[len] = 0;
+ if (len > 0)
+ buf[len] = 0;
return len;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: usb-audio: Fix out-of-bound error
2017-11-15 8:07 [PATCH] ALSA: usb-audio: Fix out-of-bound error Jaejoong Kim
@ 2017-11-15 8:57 ` Jaejoong Kim
2017-11-15 9:16 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Jaejoong Kim @ 2017-11-15 8:57 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, stable; +Cc: alsa-devel
2017. 11. 15. 오후 5:08에 "Jaejoong Kim" <climbbb.kim@gmail.com>님이 작성:
The snd_usb_copy_string_desc() retrieves the usb string corresponding to
the index number thought the usb_string(). And for NULL-terminated, insert
'0' by using the return value of usb_string() as the index of buffer to
hold the string.
wrong description for about '0'.
The problem is that the usb_string() also returns the length of the string
read(>= 0), but it can also return a negative value, the error or status
value of usb_control_msg(). If iClockSource is '0' as shown below,
usb_string() will return -EINVAL. This will result in '0' being inserted
into buf[-22], and the following KASAN out-of-bound error message will be
output.
I found UAC2_CLOCK_SOURCE is nop in the latest kernel. So commit message is
not fit in last kernel version.
(I test and patch in 4.13, sorry about that)
i will send v2 in the latest kernel
AudioControl Interface Descriptor:
bLength 8
bDescriptorType 36
bDescriptorSubtype 10 (CLOCK_SOURCE)
bClockID 1
bmAttributes 0x07 Internal programmable Clock (synced to SOF)
bmControls 0x07
Clock Frequency Control (read/write)
Clock Validity Control (read-only)
bAssocTerminal 0
iClockSource 0
To fix out-of-bound error, insert 0 only if the return value of
usb_string() is greater than 0.
==================================================================
BUG: KASAN: stack-out-of-bounds in parse_audio_unit+0x1327/0x1960
[snd_usb_audio]
Write of size 1 at addr ffff88007e66735a by task systemd-udevd/18376
CPU: 0 PID: 18376 Comm: systemd-udevd Not tainted 4.13.0+ #3
Hardware name: LG Electronics 15N540-RFLGL/White Tip
Mountain, BIOS 15N5
Call Trace:
dump_stack+0x63/0x8d
print_address_description+0x70/0x290
? parse_audio_unit+0x1327/0x1960 [snd_usb_audio]
kasan_report+0x265/0x350
__asan_store1+0x4a/0x50
parse_audio_unit+0x1327/0x1960 [snd_usb_audio]
? save_stack+0xb5/0xd0
? save_stack_trace+0x1b/0x20
? save_stack+0x46/0xd0
? kasan_kmalloc+0xad/0xe0
? kmem_cache_alloc_trace+0xff/0x230
? snd_usb_create_mixer+0xb0/0x4b0 [snd_usb_audio]
? usb_audio_probe+0x4de/0xf40 [snd_usb_audio]
? usb_probe_interface+0x1f5/0x440
? driver_probe_device+0x3ed/0x660
? build_feature_ctl+0xb10/0xb10 [snd_usb_audio]
? save_stack_trace+0x1b/0x20
? init_object+0x69/0xa0
? snd_usb_find_csint_desc+0xa8/0xf0 [snd_usb_audio]
snd_usb_mixer_controls+0x1dc/0x370 [snd_usb_audio]
? build_audio_procunit+0x890/0x890 [snd_usb_audio]
? snd_usb_create_mixer+0xb0/0x4b0 [snd_usb_audio]
? kmem_cache_alloc_trace+0xff/0x230
? usb_ifnum_to_if+0xbd/0xf0
snd_usb_create_mixer+0x25b/0x4b0 [snd_usb_audio]
? snd_usb_create_stream+0x255/0x2c0 [snd_usb_audio]
usb_audio_probe+0x4de/0xf40 [snd_usb_audio]
? snd_usb_autosuspend.part.7+0x30/0x30 [snd_usb_audio]
? __pm_runtime_idle+0x90/0x90
? kernfs_activate+0xa6/0xc0
? usb_match_one_id_intf+0xdc/0x130
? __pm_runtime_set_status+0x2d4/0x450
usb_probe_interface+0x1f5/0x440
Cc: <stable@vger.kernel.org>
Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
The AudioControl Interface Descriptor in commit message is from
lsusb output with real usb audio DAC.
The usb audio product causing the OOB are as follows:
http://www.lg.com/uk/lg-friends/lg-AFD-1200
It only prints OOB error and usb audio works well. :)
sound/usb/mixer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index e630813..5a83c2c 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -204,7 +204,8 @@ static int snd_usb_copy_string_desc(struct mixer_build
*state,
int index, char *buf, int maxlen)
{
int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
- buf[len] = 0;
+ if (len > 0)
+ buf[len] = 0;
return len;
}
thanks.
jaejoong
--
2.7.4
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: usb-audio: Fix out-of-bound error
2017-11-15 8:57 ` Jaejoong Kim
@ 2017-11-15 9:16 ` Takashi Iwai
2017-11-16 0:42 ` Jaejoong Kim
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2017-11-15 9:16 UTC (permalink / raw)
To: Jaejoong Kim; +Cc: Jaroslav Kysela, stable, alsa-devel
On Wed, 15 Nov 2017 09:57:56 +0100,
Jaejoong Kim wrote:
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index e630813..5a83c2c 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -204,7 +204,8 @@ static int snd_usb_copy_string_desc(struct mixer_build
> *state,
> int index, char *buf, int maxlen)
> {
> int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
> - buf[len] = 0;
> + if (len > 0)
> + buf[len] = 0;
> return len;
I'd rather put an explicit error bail-out, i.e.
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 91bc8f18791e..296a63a9d09c 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -204,6 +204,10 @@ static int snd_usb_copy_string_desc(struct mixer_build *state,
int index, char *buf, int maxlen)
{
int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
+
+ if (len < 0)
+ return len;
+
buf[len] = 0;
return len;
}
thanks,
Takashi
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ALSA: usb-audio: Fix out-of-bound error
2017-11-15 9:16 ` Takashi Iwai
@ 2017-11-16 0:42 ` Jaejoong Kim
0 siblings, 0 replies; 4+ messages in thread
From: Jaejoong Kim @ 2017-11-16 0:42 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jaroslav Kysela, stable, alsa-devel
Hi, Takashi
2017-11-15 18:16 GMT+09:00 Takashi Iwai <tiwai@suse.de>:
> On Wed, 15 Nov 2017 09:57:56 +0100,
> Jaejoong Kim wrote:
>>
>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>> index e630813..5a83c2c 100644
>> --- a/sound/usb/mixer.c
>> +++ b/sound/usb/mixer.c
>> @@ -204,7 +204,8 @@ static int snd_usb_copy_string_desc(struct mixer_build
>> *state,
>> int index, char *buf, int maxlen)
>> {
>> int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
>> - buf[len] = 0;
>> + if (len > 0)
>> + buf[len] = 0;
>> return len;
>
> I'd rather put an explicit error bail-out, i.e.
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 91bc8f18791e..296a63a9d09c 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -204,6 +204,10 @@ static int snd_usb_copy_string_desc(struct mixer_build *state,
> int index, char *buf, int maxlen)
> {
> int len = usb_string(state->chip->dev, index, buf, maxlen - 1);
> +
> + if (len < 0)
> + return len;
> +
> buf[len] = 0;
> return len;
> }
OK.
I will resend v2 patch with your suggestion.
>
>
> thanks,
>
> Takashi
thanks,
Jaejoong
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-16 0:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-15 8:07 [PATCH] ALSA: usb-audio: Fix out-of-bound error Jaejoong Kim
2017-11-15 8:57 ` Jaejoong Kim
2017-11-15 9:16 ` Takashi Iwai
2017-11-16 0:42 ` Jaejoong Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox