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

[Takashi Iwai <tiwai@suse.de>]

On Wed, 10 May 2023 19:39:10 +0200,
Oswald Buddenhagen wrote:
> 
> If these are hit, we've already trashed kernel memory by writing past
> the end of the allocated buffer. There is no recovery from that.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
> v2:
> - slightly more verbose description

Again, this is NAK.  First of all, if we really do care the overflow
seriously, we should check at each increment instead of after
breakage.  It shouldn't be too difficult at all.

Second, using BUG_ON() like this case is an overkill.  It was clearly
stated by Linus in the past a few times (although I can't find the
source right now).


thanks,

Takashi

> ---
>  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 2da1f9f1fb5a..5ae275d87c59 100644
> --- a/sound/pci/emu10k1/emufx.c
> +++ b/sound/pci/emu10k1/emufx.c
> @@ -1668,22 +1668,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);
> @@ -2272,16 +2269,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
>