[PATCH] ALSA: emu10k1: properly assert DSP init constraints

[PATCH] ALSA: emu10k1: properly assert DSP init constraints

[Oswald Buddenhagen]

If these are hit, we've already trashed kernel space. There is no
recovery from that.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emufx.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
index 6cf7c8b1de47..f6f05219f7fc 100644
--- a/sound/pci/emu10k1/emufx.c
+++ b/sound/pci/emu10k1/emufx.c
@@ -1773,22 +1773,19 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_GPR(var), A_GPR(vol), A_EXTIN(input))
 	 * ok, set up done..
 	 */
 
-	if (gpr > tmp) {
-		snd_BUG();
-		err = -EIO;
-		goto __err;
-	}
+	BUG_ON(gpr > tmp);
+	BUG_ON(nctl > SND_EMU10K1_GPR_CONTROLS);
+
 	/* clear remaining instruction memory */
 	while (ptr < 0x400)
 		A_OP(icode, &ptr, 0x0f, 0xc0, 0xc0, 0xcf, 0xc0);
 
 	icode->gpr_add_control_count = nctl;
 	icode->gpr_add_controls = controls;
 	emu->support_tlv = 1; /* support TLV */
 	err = snd_emu10k1_icode_poke(emu, icode, true);
 	emu->support_tlv = 0; /* clear again */
 
-__err:
 	kfree(controls);
 __err_ctrls:
 	kfree(icode->gpr_map);
@@ -2391,16 +2388,8 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu)
 	}
 	    
 
-	if (gpr > tmp) {
-		snd_BUG();
-		err = -EIO;
-		goto __err;
-	}
-	if (i > SND_EMU10K1_GPR_CONTROLS) {
-		snd_BUG();
-		err = -EIO;
-		goto __err;
-	}
+	BUG_ON(gpr > tmp);
+	BUG_ON(i > SND_EMU10K1_GPR_CONTROLS);
 	
 	/* clear remaining instruction memory */
 	while (ptr < 0x200)
-- 
2.40.0.152.g15d061e6df

Re: [PATCH] ALSA: emu10k1: properly assert DSP init constraints

[Takashi Iwai]

On Fri, 21 Apr 2023 16:10:06 +0200,
Oswald Buddenhagen wrote:
> 
> If these are hit, we've already trashed kernel space. There is no
> recovery from that.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Sorry, it's a big NO-NO.  BUG_ON() shouldn't be used here at all.

BUG_ON() is used for the case you inevitably must stop everything
immediately at this point.


thanks,

Takashi

Re: [PATCH] ALSA: emu10k1: properly assert DSP init constraints

[Oswald Buddenhagen]

On Fri, Apr 21, 2023 at 05:05:28PM +0200, Takashi Iwai wrote:
>On Fri, 21 Apr 2023 16:10:06 +0200,
>Oswald Buddenhagen wrote:
>> 
>> If these are hit, we've already trashed kernel space. There is no
>> recovery from that.
>> 
>> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
>Sorry, it's a big NO-NO.  BUG_ON() shouldn't be used here at all.
>
>BUG_ON() is used for the case you inevitably must stop everything
>immediately at this point.
>
yes, this is exactly what is intended, and i hoped that the commit 
message makes it clear enough why.

regards

Re: [PATCH] ALSA: emu10k1: properly assert DSP init constraints

[Takashi Iwai]

On Fri, 21 Apr 2023 17:08:50 +0200,
Oswald Buddenhagen wrote:
> 
> On Fri, Apr 21, 2023 at 05:05:28PM +0200, Takashi Iwai wrote:
> > On Fri, 21 Apr 2023 16:10:06 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> If these are hit, we've already trashed kernel space. There is no
> >> recovery from that.
> >> 
> >> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> > 
> > Sorry, it's a big NO-NO.  BUG_ON() shouldn't be used here at all.
> > 
> > BUG_ON() is used for the case you inevitably must stop everything
> > immediately at this point.
> > 
> yes, this is exactly what is intended, and i hoped that the commit
> message makes it clear enough why.

Not clear at all.  Please explain in more details if we really *HAVE
TO* use BUG_ON() there.


thanks,

Takashi