diff mbox

[MC,Next,Gen] sound/usb: Fix out of bounds access in media_entity_init()

Message ID 1449273629-4991-1-git-send-email-shuahkh@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shuah Khan Dec. 5, 2015, midnight UTC
Fix the out of bounds access in media_entity_init() found
by KASan. This is a result of media_mixer_init() failing
to allocate memory for all 3 of its pads before calling
media_entity_init(). Fix it to allocate memory for the
right struct media_mixer_ctl instead of struct media_ctl.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---

This patch fixes the mixer patch below:
https://patchwork.linuxtv.org/patch/31827/

 sound/usb/media.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Takashi Iwai Dec. 7, 2015, 8:15 a.m. UTC | #1
On Sat, 05 Dec 2015 01:00:29 +0100,
Shuah Khan wrote:
> 
> Fix the out of bounds access in media_entity_init() found
> by KASan. This is a result of media_mixer_init() failing
> to allocate memory for all 3 of its pads before calling
> media_entity_init(). Fix it to allocate memory for the
> right struct media_mixer_ctl instead of struct media_ctl.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> 
> This patch fixes the mixer patch below:
> https://patchwork.linuxtv.org/patch/31827/
> 
>  sound/usb/media.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/usb/media.c b/sound/usb/media.c
> index bebe27b..0cb44b9 100644
> --- a/sound/usb/media.c
> +++ b/sound/usb/media.c
> @@ -233,8 +233,8 @@ int media_mixer_init(struct snd_usb_audio *chip)
>  		if (mixer->media_mixer_ctl)
>  			continue;
>  
> -		/* allocate media_ctl */
> -		mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL);
> +		/* allocate media_mixer_ctl */
> +		mctl = kzalloc(sizeof(struct media_mixer_ctl), GFP_KERNEL);

Isn't it better to use sizeof(*mctl)?


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shuah Khan Dec. 7, 2015, 2:23 p.m. UTC | #2
On 12/07/2015 01:15 AM, Takashi Iwai wrote:
> On Sat, 05 Dec 2015 01:00:29 +0100,
> Shuah Khan wrote:
>>
>> Fix the out of bounds access in media_entity_init() found
>> by KASan. This is a result of media_mixer_init() failing
>> to allocate memory for all 3 of its pads before calling
>> media_entity_init(). Fix it to allocate memory for the
>> right struct media_mixer_ctl instead of struct media_ctl.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>
>> This patch fixes the mixer patch below:
>> https://patchwork.linuxtv.org/patch/31827/
>>
>>  sound/usb/media.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/usb/media.c b/sound/usb/media.c
>> index bebe27b..0cb44b9 100644
>> --- a/sound/usb/media.c
>> +++ b/sound/usb/media.c
>> @@ -233,8 +233,8 @@ int media_mixer_init(struct snd_usb_audio *chip)
>>  		if (mixer->media_mixer_ctl)
>>  			continue;
>>  
>> -		/* allocate media_ctl */
>> -		mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL);
>> +		/* allocate media_mixer_ctl */
>> +		mctl = kzalloc(sizeof(struct media_mixer_ctl), GFP_KERNEL);
> 
> Isn't it better to use sizeof(*mctl)?
> 

Yes. That is definitely less error prone than
sizeof(struct foo). I will fix it and send the
corrected patch.

thanks,
-- Shuah
diff mbox

Patch

diff --git a/sound/usb/media.c b/sound/usb/media.c
index bebe27b..0cb44b9 100644
--- a/sound/usb/media.c
+++ b/sound/usb/media.c
@@ -233,8 +233,8 @@  int media_mixer_init(struct snd_usb_audio *chip)
 		if (mixer->media_mixer_ctl)
 			continue;
 
-		/* allocate media_ctl */
-		mctl = kzalloc(sizeof(struct media_ctl), GFP_KERNEL);
+		/* allocate media_mixer_ctl */
+		mctl = kzalloc(sizeof(struct media_mixer_ctl), GFP_KERNEL);
 		if (!mctl)
 			return -ENOMEM;
 
@@ -244,6 +244,7 @@  int media_mixer_init(struct snd_usb_audio *chip)
 		mctl->media_pad[0].flags = MEDIA_PAD_FL_SINK;
 		mctl->media_pad[1].flags = MEDIA_PAD_FL_SOURCE;
 		mctl->media_pad[2].flags = MEDIA_PAD_FL_SOURCE;
+
 		media_entity_init(&mctl->media_entity, MEDIA_MIXER_PAD_MAX,
 				  mctl->media_pad);
 		ret =  media_device_register_entity(mctl->media_dev,