From: Stephen Boyd <sboyd@kernel.org>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>,
linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, mturquette@baylibre.com,
matthias.bgg@gmail.com
Subject: Re: [PATCH] clk: ralink: fix 'mt7621_gate_is_enabled()' function
Date: Fri, 10 Feb 2023 16:07:37 -0800 [thread overview]
Message-ID: <512be61587339db248f5d88dc5240ee3.sboyd@kernel.org> (raw)
In-Reply-To: <20230206083305.147582-1-sergio.paracuellos@gmail.com>
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@gmail.com>
> ---
Applied to clk-next
prev parent reply other threads:[~2023-02-11 0:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-06 8:33 [PATCH] clk: ralink: fix 'mt7621_gate_is_enabled()' function Sergio Paracuellos
2023-02-11 0:07 ` Stephen Boyd [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=512be61587339db248f5d88dc5240ee3.sboyd@kernel.org \
--to=sboyd@kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=mturquette@baylibre.com \
--cc=sergio.paracuellos@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox