linux-kernel - Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4E83CF74.1040701@gmail.com>
Date:	Thu, 29 Sep 2011 11:52:52 +1000
From:	Ryan Mallon <rmallon@...il.com>
To:	Dave Young <hidave.darkstar@...il.com>
CC:	Axel Lin <axel.lin@...il.com>, linux-kernel@...r.kernel.org,
	Liam Girdwood <lrg@...com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	alsa-devel@...a-project.org,
	Peter Hsiang <peter.hsiang@...im-ic.com>,
	Jesse Marroquin <jesse.marroquin@...im-ic.com>
Subject: Re: [PATCH 2/2] ASoC: Add BUG() assertion if max98095_get_bq_channel
 returns -EINVAL

On 29/09/11 11:35, Dave Young wrote:

> On Thu, Sep 29, 2011 at 7:19 AM, Ryan Mallon <rmallon@...il.com> wrote:
>> On 29/09/11 00:02, Axel Lin wrote:
>>> The callers use the return value of max98095_get_bq_channel as array index to
>>> access max98095->dai[] array.
>>> Add BUG() assertion for out of bound access of max98095->dai[] array.
>>
>> Same here, fix the problem in the callers.
>>
>> ----
>> Check the return value of max98095_get_bq_channel in the callers and
>> propagate any errors up. Remove the BUG_ON(channel > 1) since
>> max98095_get_bq_channel never returns a value larger than 1.
>>
>> Signed-off-by: Ryan Mallon <rmallon@...il.com>
>> ---
>>
>> diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c
>> index 668434d..55eccea 100644
>> --- a/sound/soc/codecs/max98095.c
>> +++ b/sound/soc/codecs/max98095.c
>> @@ -2014,7 +2014,8 @@ static int max98095_put_bq_enum(struct snd_kcontrol *kcontrol,
>>        int fs, best, best_val, i;
>>        int regmask, regsave;
>>
>> -       BUG_ON(channel > 1);
>> +       if (channel < 0)
>> +               return channel;
> 
> If use BUG() happens in  max98095_get_bq_channel, it will not return here?


Not quite sure what you mean?

If CONFIG_BUG was not enabled for the original version, then it would
not return at the BUG_ON and would either crash or cause odd behaviour
if it tried to index channel as -1.

My patch is removing the BUG_ON and replacing it with a proper check and
return. It doesn't need to check > 1 since max98095_get_bq_channel never
returns that.

My understanding is that device drivers, in general, should not call
BUG. BUG is for unrecoverable errors which leave the kernel in some
unstable state. Here we can just return an error code.

~Ryan

> 
>>
>>        if (!pdata || !max98095->bq_textcnt)
>>                return 0;
>> @@ -2069,6 +2070,9 @@ static int max98095_get_bq_enum(struct snd_kcontrol *kcontrol,
>>        int channel = max98095_get_bq_channel(kcontrol->id.name);
>>        struct max98095_cdata *cdata;
>>
>> +       if (channel < 0)
>> +               return channel;
>> +
>>        cdata = &max98095->dai[channel];
>>        ucontrol->value.enumerated.item[0] = cdata->bq_sel;
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/