ASoC: max98090: Remove unneeded check in max98090_put_enab_tlv()

Variable sel is of unsigned int type, so sel < 0 is not required.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Fixes: 2fbe467bcbfc ("ASoC: max98090: Reject invalid values in custom control put()")
---
 sound/soc/codecs/max98090.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 19.05.2022 20:29, Mark Brown wrote:
> On Thu, May 19, 2022 at 08:10:12PM +0300, Alexey Khoroshilov wrote:
>> Variable sel is of unsigned int type, so sel < 0 is not required.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
>>  	val = (val >> mc->shift) & mask;
>>  
>> -	if (sel < 0 || sel > mc->max)
>> +	if (sel > mc->max)
> 
> The check needs to be moved, not removed.  The userspace ABI allows
> passing in of negative values.
> 

Would (sel > mc->max) be enough in this case anyway?

--
Alexey

On 19.05.2022 20:54, Mark Brown wrote:
> On Thu, May 19, 2022 at 08:49:48PM +0300, Alexey Khoroshilov wrote:
>> On 19.05.2022 20:29, Mark Brown wrote:
>>> On Thu, May 19, 2022 at 08:10:12PM +0300, Alexey Khoroshilov wrote:
> 
>>>> -	if (sel < 0 || sel > mc->max)
>>>> +	if (sel > mc->max)
> 
>>> The check needs to be moved, not removed.  The userspace ABI allows
>>> passing in of negative values.
> 
>> Would (sel > mc->max) be enough in this case anyway?
> 
> Oh, the check won't be working properly - it's just that like I say the
> fix is to move rather than remove it so it's operating on the signed
> value.
> 

Do you mean something like this?

static int max98090_put_enab_tlv(struct snd_kcontrol *kcontrol,
				struct snd_ctl_elem_value *ucontrol)
{
	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
	struct max98090_priv *max98090 = snd_soc_component_get_drvdata(component);
	struct soc_mixer_control *mc =
		(struct soc_mixer_control *)kcontrol->private_value;
	unsigned int mask = (1 << fls(mc->max)) - 1;
-       unsigned int sel = ucontrol->value.integer.value[0];
+       int sel_unchecked = ucontrol->value.integer.value[0];
+       unsigned int sel;
	unsigned int val = snd_soc_component_read(component, mc->reg);
	unsigned int *select;

	switch (mc->reg) {
	case M98090_REG_MIC1_INPUT_LEVEL:
		select = &(max98090->pa1en);
		break;
	case M98090_REG_MIC2_INPUT_LEVEL:
		select = &(max98090->pa2en);
		break;
	case M98090_REG_ADC_SIDETONE:
		select = &(max98090->sidetone);
		break;
	default:
		return -EINVAL;
	}

	val = (val >> mc->shift) & mask;

-       if (sel < 0 || sel > mc->max)
+       if (sel_unchecked < 0 || sel_unchecked > mc->max)
                return -EINVAL;
+       sel = sel_unchecked;

	*select = sel;

On Thu, May 19, 2022 at 09:27:25PM +0300, Alexey Khoroshilov wrote:
> On 19.05.2022 20:54, Mark Brown wrote:

> > Oh, the check won't be working properly - it's just that like I say the
> > fix is to move rather than remove it so it's operating on the signed
> > value.

> Do you mean something like this?

That looks about right.

On Thu, May 19, 2022 at 11:13:00PM +0300, Alexey Khoroshilov wrote:
> On 19.05.2022 23:07, Mark Brown wrote:
> > On Thu, May 19, 2022 at 09:27:25PM +0300, Alexey Khoroshilov wrote:

> >> Do you mean something like this?

> > That looks about right.

> Should I prepare a patch or you will do it yourself?

Please send it, you already wrote it - may as well get the credit too.