[PATCH 1/2] ASoC: ops: fix signedness bug in snd_soc_put_volsw()
The "val" and "val2" variables need to signed for the checking to work as intended.
Fixes: 817f7c9335ec ("ASoC: ops: Reject out of bounds values in snd_soc_put_volsw()") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- sound/soc/soc-ops.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index dc0e7c8d31f3..0091fa96eb48 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -310,8 +310,9 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, unsigned int invert = mc->invert; int err; bool type_2r = false; - unsigned int val2 = 0; - unsigned int val, val_mask; + unsigned int val_mask; + int val2 = 0; + int val;
if (sign_bit) mask = BIT(sign_bit + 1) - 1;
The "val" variable needs to be signed for the checking to work correctly.
Fixes: 4f1e50d6a9cf ("ASoC: ops: Reject out of bounds values in snd_soc_put_volsw_sx()") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- sound/soc/soc-ops.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 0091fa96eb48..a615316a06ff 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -422,7 +422,8 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, int min = mc->min; unsigned int mask = (1U << (fls(min + max) - 1)) - 1; int err = 0; - unsigned int val, val_mask; + unsigned int val_mask; + int val;
val = ucontrol->value.integer.value[0]; if (mc->platform_max && val > mc->platform_max)
On Fri, Jan 28, 2022 at 12:42:04PM +0000, Mark Brown wrote:
On Fri, Jan 28, 2022 at 02:20:07PM +0300, Dan Carpenter wrote:
The "val" and "val2" variables need to signed for the checking to work as intended.
This means that the helpers won't support controls that use the top bit of a 32 bit register.
Fine, I can delete the checks for negative instead (I'm surprised you haven't already received a dozen bot emails about this).
I haven't been able to figure out where mc->min/max are set. In snd_soc_get_xr_sx() it checks whether "mc->min" is negative.
if (min < 0 && val > max) val |= ~mask;
Is that a bug? If mc->min is negative the math gets tricky.
regards, dan carpenter
On Fri, Jan 28, 2022 at 04:31:47PM +0300, Dan Carpenter wrote:
On Fri, Jan 28, 2022 at 12:42:04PM +0000, Mark Brown wrote:
This means that the helpers won't support controls that use the top bit of a 32 bit register.
Fine, I can delete the checks for negative instead (I'm surprised you haven't already received a dozen bot emails about this).
No, we need the checks for negatives since userspace supplies a signed value. The check needs to be done on the value in the input structure before we pull it out for mangling. I probably got bot emails but frankly these days almost all of them are some combination of barely legible and misdirected and there's plenty of people who like to fix these things if they're real.
I haven't been able to figure out where mc->min/max are set. In
They're the big tables of static assignments via macros in the drivers.
snd_soc_get_xr_sx() it checks whether "mc->min" is negative.
if (min < 0 && val > max) val |= ~mask;
Is that a bug? If mc->min is negative the math gets tricky.
No, it *is* valid if weird to have negative values for some controls.
On Fri, Jan 28, 2022 at 01:48:44PM +0000, Mark Brown wrote:
On Fri, Jan 28, 2022 at 04:31:47PM +0300, Dan Carpenter wrote:
On Fri, Jan 28, 2022 at 12:42:04PM +0000, Mark Brown wrote:
This means that the helpers won't support controls that use the top bit of a 32 bit register.
Fine, I can delete the checks for negative instead (I'm surprised you haven't already received a dozen bot emails about this).
No, we need the checks for negatives since userspace supplies a signed value. The check needs to be done on the value in the input structure before we pull it out for mangling. I probably got bot emails but frankly these days almost all of them are some combination of barely legible and misdirected and there's plenty of people who like to fix these things if they're real.
That's a bit trickier than I initially imagined so I'm going to leave this for smarter people than me...
regards, dan carpenter
participants (2)
-
Dan Carpenter -
Mark Brown