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

[Hui Wang]

On 3/26/22 00:46, Lucas tanure wrote:
> 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.
>
OK, got it. And I re-read the code and found It is the last entry in the 
array to introduce this call-trace, the CS35L41_NUM_OTP_ELEM is defined 
to be 100, but otp_map_1/2[] only defines 99 elements, so the last entry 
in the array are {0, 0, 0}, that is why the driver gets a 0 for map[i].size.

I will remove the CS35L41_NUM_OTP_ELEM and use ARRAY_SIZE() in the v2 patch.

Thanks,

Hui.

>>
>> Signed-off-by: Hui Wang <hui.wang at 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) {
>