2012-02-14 10:21:27

by Xi Wang

Subject: [PATCH] ALSA: usb-audio: avoid integer overflow in create_fixed_stream_quirk()

A malicious USB device could feed in a large nr_rates value. This would
cause the subsequent call to kmemdup() to allocate a smaller buffer than
expected, leading to out-of-bounds access.

This patch validates the nr_rates value and reuses the limit introduced
in commit 4fa0e81b ("ALSA: usb-audio: fix possible hang and overflow
in parse_uac2_sample_rate_range()").

Signed-off-by: Xi Wang <[email protected]>
---
sound/usb/card.h | 1 +
sound/usb/format.c | 4 +---
sound/usb/quirks.c | 6 +++++-
3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index a39edcc..da5fa1a 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -1,6 +1,7 @@
#ifndef __USBAUDIO_CARD_H
#define __USBAUDIO_CARD_H

+#define MAX_NR_RATES 1024
#define MAX_PACKS 20
#define MAX_PACKS_HS (MAX_PACKS * 8) /* in high speed mode */
#define MAX_URBS 8
diff --git a/sound/usb/format.c b/sound/usb/format.c
index e09aba1..ddfef57 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -209,8 +209,6 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
return 0;
}

-#define MAX_UAC2_NR_RATES 1024
-
/*
* Helper function to walk the array of sample rate triplets reported by
* the device. The problem is that we need to parse whole array first to
@@ -255,7 +253,7 @@ static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
fp->rates |= snd_pcm_rate_to_rate_bit(rate);

nr_rates++;
- if (nr_rates >= MAX_UAC2_NR_RATES) {
+ if (nr_rates >= MAX_NR_RATES) {
snd_printk(KERN_ERR "invalid uac2 rates\n");
break;
}
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index a3ddac0..2781726 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -132,10 +132,14 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
unsigned *rate_table = NULL;

fp = kmemdup(quirk->data, sizeof(*fp), GFP_KERNEL);
- if (! fp) {
+ if (!fp) {
snd_printk(KERN_ERR "cannot memdup\n");
return -ENOMEM;
}
+ if (fp->nr_rates > MAX_NR_RATES) {
+ kfree(fp);
+ return -EINVAL;
+ }
if (fp->nr_rates > 0) {
rate_table = kmemdup(fp->rate_table,
sizeof(int) * fp->nr_rates, GFP_KERNEL);
--
1.7.5.4

2012-02-15 14:00:39

by Takashi Iwai

Subject: Re: [PATCH] ALSA: usb-audio: avoid integer overflow in create_fixed_stream_quirk()

At Tue, 14 Feb 2012 05:18:48 -0500,
Xi Wang wrote:
>
> A malicious USB device could feed in a large nr_rates value. This would
> cause the subsequent call to kmemdup() to allocate a smaller buffer than
> expected, leading to out-of-bounds access.
>
> This patch validates the nr_rates value and reuses the limit introduced
> in commit 4fa0e81b ("ALSA: usb-audio: fix possible hang and overflow
> in parse_uac2_sample_rate_range()").
>
> Signed-off-by: Xi Wang <[email protected]>

Thanks, applied now.


Takashi

> ---
> sound/usb/card.h | 1 +
> sound/usb/format.c | 4 +---
> sound/usb/quirks.c | 6 +++++-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index a39edcc..da5fa1a 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -1,6 +1,7 @@
> #ifndef __USBAUDIO_CARD_H
> #define __USBAUDIO_CARD_H
>
> +#define MAX_NR_RATES 1024
> #define MAX_PACKS 20
> #define MAX_PACKS_HS (MAX_PACKS * 8) /* in high speed mode */
> #define MAX_URBS 8
> diff --git a/sound/usb/format.c b/sound/usb/format.c
> index e09aba1..ddfef57 100644
> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -209,8 +209,6 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
> return 0;
> }
>
> -#define MAX_UAC2_NR_RATES 1024
> -
> /*
> * Helper function to walk the array of sample rate triplets reported by
> * the device. The problem is that we need to parse whole array first to
> @@ -255,7 +253,7 @@ static int parse_uac2_sample_rate_range(struct audioformat *fp, int nr_triplets,
> fp->rates |= snd_pcm_rate_to_rate_bit(rate);
>
> nr_rates++;
> - if (nr_rates >= MAX_UAC2_NR_RATES) {
> + if (nr_rates >= MAX_NR_RATES) {
> snd_printk(KERN_ERR "invalid uac2 rates\n");
> break;
> }
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index a3ddac0..2781726 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -132,10 +132,14 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
> unsigned *rate_table = NULL;
>
> fp = kmemdup(quirk->data, sizeof(*fp), GFP_KERNEL);
> - if (! fp) {
> + if (!fp) {
> snd_printk(KERN_ERR "cannot memdup\n");
> return -ENOMEM;
> }
> + if (fp->nr_rates > MAX_NR_RATES) {
> + kfree(fp);
> + return -EINVAL;
> + }
> if (fp->nr_rates > 0) {
> rate_table = kmemdup(fp->rate_table,
> sizeof(int) * fp->nr_rates, GFP_KERNEL);
> --
> 1.7.5.4
>