[alsa-devel] Undefined behaviour in ac97_codec.c - shift exponent 68 is too large for 32-bit type 'int'

[Takashi Iwai]

On Fri, 23 Nov 2018 10:16:53 +0100,
Meelis Roos wrote:
> 
> I updated one of my old laptops (ECS Desknote 532 with Transmeta CPU) to newest kernel
> (4.20.0-rc3-00145-gedeca3a769ad) and turned on UBSAN checks. Got the following UBSAN
> warning multiple times per boot.
> 
> The soundcard:
> 00:04.0 Multimedia audio controller [0401]: ULi Electronics Inc. M5455 PCI AC-Link Controller Audio Device [10b9:5455] (rev 10)
>         Subsystem: Elitegroup Computer Systems M5455 PCI AC-Link Controller Audio Device [1019:0f56]
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 64 (16000ns min), Cache Line Size: 128 bytes
>         Interrupt: pin A routed to IRQ 5
>         Region 0: I/O ports at e400 [size=256]
>         Region 1: Memory at febfe000 (32-bit, non-prefetchable) [size=4K]
>         Capabilities: <access denied>
>         Kernel driver in use: snd_intel8x0
>         Kernel modules: snd_intel8x0
> 
> /proc/asound/cards:
> 
>  0 [M5455          ]: ICH - ALi M5455
>                       ALi M5455 with ALC655 at irq 5
> 
> gcc version 8.2.0 (Debian 8.2.0-9)
> 
> [   15.688683] snd_intel8x0 0000:00:04.0: intel8x0_measure_ac97_clock: measured 58318 usecs (2808 samples)
> [   15.689033] snd_intel8x0 0000:00:04.0: clocking to 48000
> ...
> [   19.667746] ================================================================================
> [   19.668078] UBSAN: Undefined behaviour in sound/pci/ac97/ac97_codec.c:836:7
> [   19.668268] shift exponent 68 is too large for 32-bit type 'int'

Wow, this is an old bug.

The patch below should fix the problem.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: ac97: Fix incorrect bit shift at AC97-SPSA control
 write

The function snd_ac97_put_spsa() gets the bit shift value from the
associated private_value, but it extracts too much; the current code
extracts 8 bit values in bits 8-15, but this is a combination of two
nibbles (bits 8-11 and bits 12-15) for left and right shifts.
Due to the incorrect bits extraction, the actual shift may go beyond
the 32bit value, as spotted recently by UBSAN check:
 UBSAN: Undefined behaviour in sound/pci/ac97/ac97_codec.c:836:7
 shift exponent 68 is too large for 32-bit type 'int'

This patch fixes the shift value extraction by masking the properly
with 0x0f instead of 0xff.

Reported-by: Meelis Roos <mroos at linux.ee>
Cc: <stable at vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/ac97/ac97_codec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
index f4459d1a9d67..27b468f057dd 100644
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -824,7 +824,7 @@ static int snd_ac97_put_spsa(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_
 {
 	struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
 	int reg = kcontrol->private_value & 0xff;
-	int shift = (kcontrol->private_value >> 8) & 0xff;
+	int shift = (kcontrol->private_value >> 8) & 0x0f;
 	int mask = (kcontrol->private_value >> 16) & 0xff;
 	// int invert = (kcontrol->private_value >> 24) & 0xff;
 	unsigned short value, old, new;
-- 
2.19.1