diff mbox series

[2/2] ASoC: cs35l41: Fix a shift-out-of-bounds warning found by UBSAN

Message ID 20220324081839.62009-2-hui.wang@canonical.com
State Accepted
Commit 0b3d5d2e358ca6772fc3662fca27acb12a682fbf
Headers show
Series [1/2] ASoC: cs35l41: Add one more variable in the debug log | expand

Commit Message

Hui Wang March 24, 2022, 8:18 a.m. UTC
We enabled UBSAN in the ubuntu kernel, and the cs35l41 driver triggers
a warning calltrace like below:

cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 8, word_offset=23, bit_sum mod 32=0, otp_map[i].size = 24
cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 0, word_offset=24, bit_sum mod 32=24, otp_map[i].size = 0
================================================================================
UBSAN: shift-out-of-bounds in linux-kernel-src/sound/soc/codecs/cs35l41-lib.c:836:8
shift exponent 64 is too large for 64-bit type 'long unsigned int'
CPU: 10 PID: 595 Comm: systemd-udevd Not tainted 5.15.0-23-generic #23
Hardware name: LENOVO \x02MFG_IN_GO/\x02MFG_IN_GO, BIOS N3GET19W (1.00 ) 03/11/2022
Call Trace:
 <TASK>
 show_stack+0x52/0x58
 dump_stack_lvl+0x4a/0x5f
 dump_stack+0x10/0x12
 ubsan_epilogue+0x9/0x45
 __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef
 ? regmap_unlock_mutex+0xe/0x10
 cs35l41_otp_unpack.cold+0x1c6/0x2b2 [snd_soc_cs35l41_lib]
 cs35l41_hda_probe+0x24f/0x33a [snd_hda_scodec_cs35l41]
 cs35l41_hda_i2c_probe+0x65/0x90 [snd_hda_scodec_cs35l41_i2c]

When both bitoffset and otp_map[i].size are 0, the line 836 will
result in GENMASK(-1, 0), this triggers the shift-out-of-bounds
calltrace.

Here add a checking, if both bitoffset and otp_map[i].size are 0,
do not run GENMASK() and directly set otp_val to 0, this will not
bring any function change on the driver but could avoid the calltrace.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/soc/codecs/cs35l41-lib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Lucas tanure March 25, 2022, 4:46 p.m. UTC | #1
On 3/24/22 08:18, Hui Wang wrote:
> We enabled UBSAN in the ubuntu kernel, and the cs35l41 driver triggers
> a warning calltrace like below:
> 
> cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 8, word_offset=23, bit_sum mod 32=0, otp_map[i].size = 24
> cs35l41-hda i2c-CSC3551:00-cs35l41-hda.0: bitoffset= 0, word_offset=24, bit_sum mod 32=24, otp_map[i].size = 0
> ================================================================================
> UBSAN: shift-out-of-bounds in linux-kernel-src/sound/soc/codecs/cs35l41-lib.c:836:8
> shift exponent 64 is too large for 64-bit type 'long unsigned int'
> CPU: 10 PID: 595 Comm: systemd-udevd Not tainted 5.15.0-23-generic #23
> Hardware name: LENOVO \x02MFG_IN_GO/\x02MFG_IN_GO, BIOS N3GET19W (1.00 ) 03/11/2022
> Call Trace:
>   <TASK>
>   show_stack+0x52/0x58
>   dump_stack_lvl+0x4a/0x5f
>   dump_stack+0x10/0x12
>   ubsan_epilogue+0x9/0x45
>   __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef
>   ? regmap_unlock_mutex+0xe/0x10
>   cs35l41_otp_unpack.cold+0x1c6/0x2b2 [snd_soc_cs35l41_lib]
>   cs35l41_hda_probe+0x24f/0x33a [snd_hda_scodec_cs35l41]
>   cs35l41_hda_i2c_probe+0x65/0x90 [snd_hda_scodec_cs35l41_i2c]
> 
> When both bitoffset and otp_map[i].size are 0, the line 836 will
> result in GENMASK(-1, 0), this triggers the shift-out-of-bounds
> calltrace.
> 
> Here add a checking, if both bitoffset and otp_map[i].size are 0,
> do not run GENMASK() and directly set otp_val to 0, this will not
> bring any function change on the driver but could avoid the calltrace.
Here would be better to return an error and fail the probe, as this is
a not expected case.

> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>   sound/soc/codecs/cs35l41-lib.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
> index d0a480c40231..aa6823fbd1a4 100644
> --- a/sound/soc/codecs/cs35l41-lib.c
> +++ b/sound/soc/codecs/cs35l41-lib.c
> @@ -831,12 +831,14 @@ int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap)
>   					GENMASK(bit_offset + otp_map[i].size - 33, 0)) <<
>   					(32 - bit_offset);
>   			bit_offset += otp_map[i].size - 32;
> -		} else {
> +		} else if (bit_offset + otp_map[i].size - 1 >= 0) {
>   			otp_val = (otp_mem[word_offset] &
>   				   GENMASK(bit_offset + otp_map[i].size - 1, bit_offset)
>   				  ) >> bit_offset;
>   			bit_offset += otp_map[i].size;
> -		}
> +		} else /* both bit_offset and otp_map[i].size are 0 */
> +			otp_val = 0;
> +
>   		bit_sum += otp_map[i].size;
>   
>   		if (bit_offset == 32) {
diff mbox series

Patch

diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
index d0a480c40231..aa6823fbd1a4 100644
--- a/sound/soc/codecs/cs35l41-lib.c
+++ b/sound/soc/codecs/cs35l41-lib.c
@@ -831,12 +831,14 @@  int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap)
 					GENMASK(bit_offset + otp_map[i].size - 33, 0)) <<
 					(32 - bit_offset);
 			bit_offset += otp_map[i].size - 32;
-		} else {
+		} else if (bit_offset + otp_map[i].size - 1 >= 0) {
 			otp_val = (otp_mem[word_offset] &
 				   GENMASK(bit_offset + otp_map[i].size - 1, bit_offset)
 				  ) >> bit_offset;
 			bit_offset += otp_map[i].size;
-		}
+		} else /* both bit_offset and otp_map[i].size are 0 */
+			otp_val = 0;
+
 		bit_sum += otp_map[i].size;
 
 		if (bit_offset == 32) {