[PATCH 2/3] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()

Josh Poimboeuf posted 3 patches 1 month ago
[PATCH 2/3] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()
Posted by Josh Poimboeuf 1 month ago
If 'port_id' is negative, the shift counts in wcd9335_slimbus_irq() also
become negative, resulting in undefined behavior due to shift out of
bounds.

That appears to be not possible, but with UBSAN enabled, Clang's range
analysis isn't always able to determine that and generates undefined
behavior.

As a result the code generation isn't optimal, and undefined behavior
should be avoided regardless.  Improve code generation and remove the
undefined behavior by converting the signed variables to unsigned.

Fixes the following warning:

  sound/soc/codecs/wcd9335.o: warning: objtool: wcd9335_slimbus_irq() falls through to next function __cfi_wcd9335_set_channel_map()

This is very similar to a previous fix to wcd934x with commit
060aed9c0093 ("objtool, ASoC: codecs: wcd934x: Remove potential
undefined behavior in wcd934x_slim_irq_handler()").

Cc: Srinivas Kandagatla <srini@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 sound/soc/codecs/wcd9335.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
index 640e43ee1975..e3ca5ca6de3d 100644
--- a/sound/soc/codecs/wcd9335.c
+++ b/sound/soc/codecs/wcd9335.c
@@ -3907,7 +3907,7 @@ static irqreturn_t wcd9335_slimbus_irq(int irq, void *data)
 {
 	struct wcd9335_codec *wcd = data;
 	unsigned long status = 0;
-	int i, j, port_id;
+	unsigned int i, j, port_id;
 	unsigned int val, int_val = 0;
 	irqreturn_t ret = IRQ_NONE;
 	bool tx;
-- 
2.53.0
Re: [PATCH 2/3] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()
Posted by Srinivas Kandagatla 1 month ago

On 3/9/26 4:03 PM, Josh Poimboeuf wrote:
> If 'port_id' is negative, the shift counts in wcd9335_slimbus_irq() also
> become negative, resulting in undefined behavior due to shift out of
> bounds.
> 
> That appears to be not possible, but with UBSAN enabled, Clang's range
> analysis isn't always able to determine that and generates undefined
> behavior.
> 
> As a result the code generation isn't optimal, and undefined behavior
> should be avoided regardless.  Improve code generation and remove the
> undefined behavior by converting the signed variables to unsigned.
> 
> Fixes the following warning:
> 
>   sound/soc/codecs/wcd9335.o: warning: objtool: wcd9335_slimbus_irq() falls through to next function __cfi_wcd9335_set_channel_map()
> 
> This is very similar to a previous fix to wcd934x with commit
> 060aed9c0093 ("objtool, ASoC: codecs: wcd934x: Remove potential
> undefined behavior in wcd934x_slim_irq_handler()").
> 
> Cc: Srinivas Kandagatla <srini@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---

Am not sure if port_id will be ever negative. but the change itself
looks harmless,

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

--srini

>  sound/soc/codecs/wcd9335.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c
> index 640e43ee1975..e3ca5ca6de3d 100644
> --- a/sound/soc/codecs/wcd9335.c
> +++ b/sound/soc/codecs/wcd9335.c
> @@ -3907,7 +3907,7 @@ static irqreturn_t wcd9335_slimbus_irq(int irq, void *data)
>  {
>  	struct wcd9335_codec *wcd = data;
>  	unsigned long status = 0;
> -	int i, j, port_id;
> +	unsigned int i, j, port_id;
>  	unsigned int val, int_val = 0;
>  	irqreturn_t ret = IRQ_NONE;
>  	bool tx;
Re: [PATCH 2/3] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()
Posted by Mark Brown 1 month ago
On Mon, Mar 09, 2026 at 09:03:06AM -0700, Josh Poimboeuf wrote:
> If 'port_id' is negative, the shift counts in wcd9335_slimbus_irq() also
> become negative, resulting in undefined behavior due to shift out of
> bounds.

You've not copied me on the rest of the series so I don't know what's
going on with dependencies.  When sending a patch series it is important
to ensure that all the various maintainers understand what the
relationship between the patches as the expecation is that there will be
interdependencies.  Either copy everyone on the whole series or at least
copy them on the cover letter and explain what's going on.  If there are
no strong interdependencies then it's generally simplest to just send
the patches separately to avoid any possible confusion.
Re: [PATCH 2/3] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()
Posted by Josh Poimboeuf 1 month ago
On Mon, Mar 09, 2026 at 04:05:06PM +0000, Mark Brown wrote:
> On Mon, Mar 09, 2026 at 09:03:06AM -0700, Josh Poimboeuf wrote:
> > If 'port_id' is negative, the shift counts in wcd9335_slimbus_irq() also
> > become negative, resulting in undefined behavior due to shift out of
> > bounds.
> 
> You've not copied me on the rest of the series so I don't know what's
> going on with dependencies.  When sending a patch series it is important
> to ensure that all the various maintainers understand what the
> relationship between the patches as the expecation is that there will be
> interdependencies.  Either copy everyone on the whole series or at least
> copy them on the cover letter and explain what's going on.  If there are
> no strong interdependencies then it's generally simplest to just send
> the patches separately to avoid any possible confusion.

Sorry about that.  This is just a collection of objtool warnings fixes
and there are no interdependencies.

-- 
Josh