[v3,1/4] ASoC: cs35l45: Checks index of cs35l45_irqs[]

Commit Message

Vlad Karpovich Aug. 31, 2023, 4:20 p.m. UTC

From: Ricardo Rivera-Matos <[e-mail]>

Checks the index computed by the virq offset before printing the
error condition in cs35l45_spk_safe_err() handler.

Signed-off-by: Ricardo Rivera-Matos <[e-mail]>
Signed-off-by: Vlad Karpovich <[e-mail]>
---
 sound/soc/codecs/cs35l45.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Ricardo Rivera-Matos Sept. 7, 2023, 7:40 p.m. UTC | #1

> On Sep 1, 2023, at 3:48 AM, Charles Keepax <[e-mail]> wrote:
> 
> On Thu, Aug 31, 2023 at 11:20:39AM -0500, Vlad Karpovich wrote:
>> From: Ricardo Rivera-Matos <[e-mail]>
>> 
>> Checks the index computed by the virq offset before printing the
>> error condition in cs35l45_spk_safe_err() handler.
>> 
>> Signed-off-by: Ricardo Rivera-Matos <[e-mail]>
>> Signed-off-by: Vlad Karpovich <[e-mail]>
>> ---
>> sound/soc/codecs/cs35l45.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c
>> index 2ac4612402eb..02b1172d2647 100644
>> --- a/sound/soc/codecs/cs35l45.c
>> +++ b/sound/soc/codecs/cs35l45.c
>> @@ -1023,7 +1023,10 @@ static irqreturn_t cs35l45_spk_safe_err(int irq, void *data)
>> 
>> i = irq - regmap_irq_get_virq(cs35l45->irq_data, 0);
>> 
>> - dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name);
>> + if (i < 0 || i >= ARRAY_SIZE(cs35l45_irqs))
> 
> I am happy enough for this to be merged, since it clearly does
> no harm. So:
> 
> Acked-by: Charles Keepax <[e-mail]>
> 
> But I do still have a slight reservation of what is the point
> of this error check?  This handler is static and can only be
> called from within cs35l45.c and the only code that registers
> IRQs goes through the cs35l45_irqs array and registers IRQs
> from there, so how does this ever end up with i being out of
> bounds?
> 
> And whilst I would not add this to this patch. I do also think
> perhaps Ricardo had a point in his email, the IRQ handler
> should probably be renamed, since it handles more than just
> the spk_safe_err's, perhaps something like cs35l45_report_err.
> On why the watchdog and global error call this as well, that
> was a review comment on an earlier patch since the handlers for
> those errors only printed a message, they might as well be
> combined with the spk_safe error that also only printed a
> message. If at some point separate handling is added for them
> they can be split out.

Thanks Charles, I had missed that comment. It’s clear to me now.
> 
> Thanks,
> Charles

Acked-by: Ricardo Rivera-Matos <[e-mail] <mailto:[e-mail]>>

Mark Brown Sept. 11, 2023, 11:57 p.m. UTC | #2

On Thu, 31 Aug 2023 11:20:39 -0500, Vlad Karpovich wrote:
> Checks the index computed by the virq offset before printing the
> error condition in cs35l45_spk_safe_err() handler.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: cs35l45: Checks index of cs35l45_irqs[]
      commit: 44f37b6ce041c838cb2f49f08998c41f1ab3b08c
[2/4] ASoC: cs35l45: Analog PCM Volume and Amplifier Mode controls
      commit: 18050443b9fc4e809c077fbf0967349410e86117
[3/4] ASoC: cs35l45: Connect DSP to the monitoring signals
      commit: 3fecf69aa7fdf1910267dee1dbaa8ed865cf2cb6
[4/4] ASoC: cs35l45: Add AMP Enable Switch control
      commit: c3c9b17d27887f7b2f6b85d0a364b009b8436539

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Patch

diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c
index 2ac4612402eb..02b1172d2647 100644
--- a/sound/soc/codecs/cs35l45.c
+++ b/sound/soc/codecs/cs35l45.c
@@ -1023,7 +1023,10 @@  static irqreturn_t cs35l45_spk_safe_err(int irq, void *data)
 
 	i = irq - regmap_irq_get_virq(cs35l45->irq_data, 0);
 
-	dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name);
+	if (i < 0 || i >= ARRAY_SIZE(cs35l45_irqs))
+		dev_err(cs35l45->dev, "Unspecified global error condition (%d) detected!\n", irq);
+	else
+		dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name);
 
 	return IRQ_HANDLED;
 }