linux-kernel - Re: [PATCH] clk: ralink: fix 'mt7621_gate_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <512be61587339db248f5d88dc5240ee3.sboyd@kernel.org>
Date:   Fri, 10 Feb 2023 16:07:37 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Sergio Paracuellos <sergio.paracuellos@...il.com>,
        linux-clk@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, mturquette@...libre.com,
        matthias.bgg@...il.com
Subject: Re: [PATCH] clk: ralink: fix 'mt7621_gate_is_enabled()' function

Quoting Sergio Paracuellos (2023-02-06 00:33:05)
> Compiling clock driver with CONFIG_UBSAN enabled shows the following trace:
> 
> UBSAN: shift-out-of-bounds in drivers/clk/ralink/clk-mt7621.c:121:15
> shift exponent 131072 is too large for 32-bit type 'long unsigned int'
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.15.86 #0
> Stack : ...
> 
> Call Trace:
> [<80009a58>] show_stack+0x38/0x118
> [<8045ce04>] dump_stack_lvl+0x60/0x80
> [<80458868>] ubsan_epilogue+0x10/0x54
> [<804590e0>] __ubsan_handle_shift_out_of_bounds+0x118/0x190
> [<804c9a10>] mt7621_gate_is_enabled+0x98/0xa0
> [<804bb774>] clk_core_is_enabled+0x34/0x90
> [<80aad73c>] clk_disable_unused_subtree+0x98/0x1e4
> [<80aad6d4>] clk_disable_unused_subtree+0x30/0x1e4
> [<80aad6d4>] clk_disable_unused_subtree+0x30/0x1e4
> [<80aad900>] clk_disable_unused+0x78/0x120
> [<80002030>] do_one_initcall+0x54/0x1f0
> [<80a922a4>] kernel_init_freeable+0x280/0x31c
> [<808047c4>] kernel_init+0x20/0x118
> [<80003e58>] ret_from_kernel_thread+0x14/0x1c
> 
> Shifting a value (131032) larger than the type (32 bit unsigned integer)
> is undefined behaviour in C.
> 
> The problem is in 'mt7621_gate_is_enabled()' function which is using the
> 'BIT()' kernel macro with the bit index for the clock gate to check if the
> bit is set. When the clock gates structure is created driver is already
> setting 'bit_idx' using 'BIT()' macro, so we are wrongly applying an extra
> 'BIT()' mask here. Removing it solve the problem and makes this function
> correct. However when clock gating is correctly working, the kernel starts
> disabling those clocks that are not requested. Some drivers for this SoC
> are older than this clock driver itself. So to avoid the kernel to disable
> clocks that have been enabled until now, we must apply 'CLK_IS_CRITICAL'
> flag on gates initialization code.
> 
> Fixes: 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@...il.com>
> ---

Applied to clk-next