linux-kernel - Re: [PATCH] drm/amd/display: Harden callers of division functions

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: <CAAhV-H7WyFqw4FDnMHN=AFOBvzSXDC+OWhjwrej1SFVat9E_xw@mail.gmail.com>
Date: Tue, 31 Dec 2024 16:45:37 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc: Alex Deucher <alexander.deucher@....com>, 
	Christian König <christian.koenig@....com>, 
	Xinhui Pan <Xinhui.Pan@....com>, Harry Wentland <harry.wentland@....com>, Leo Li <sunpeng.li@....com>, 
	Rodrigo Siqueira <Rodrigo.Siqueira@....com>, Nathan Chancellor <nathan@...nel.org>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>, loongarch@...ts.linux.dev, 
	amd-gfx@...ts.freedesktop.org, llvm@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/amd/display: Harden callers of division functions

Hi, Tiezhu,

On Tue, Dec 31, 2024 at 3:28 PM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>
> There are objtool warnings compiled with the latest mainline LLVM:
>
>   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
>   spl_fixpt_recip() falls through to next function spl_fixpt_sinc()
>
> Here are the call paths:
>
>   dc_fixpt_recip()
>     dc_fixpt_from_fraction()
>       complete_integer_division_u64()
>         div64_u64_rem()
>
>   spl_fixpt_recip()
>     spl_fixpt_from_fraction()
>       spl_complete_integer_division_u64()
>         spl_div64_u64_rem()
>
> This was introduced by a change in Clang from a few months:
>
>   [SimplifyCFG] Deduce paths unreachable if they cause div/rem UB)
>   https://github.com/llvm/llvm-project/commit/37932643abab
>
> Since the ASSERT does not do anything to prevent the divide by zero
> (just flags it with WARN_ON) and the rest of the code doesn't either,
> the callers of division functions should harden them against dividing
> by zero to avoid undefined behavior.
>
> Keep the current ASSERT for the aim of debugging, just add BUG() to
> stop control flow if the divisior is zero.
>
> Suggested-by: Nathan Chancellor <nathan@...nel.org>
> Suggested-by: Xi Ruoyao <xry111@...111.site>
> Suggested-by: Rui Wang <wangrui@...ngson.cn>
> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> Link: https://lore.kernel.org/lkml/20241220223403.GA2605890@ax162/
> ---
>  drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c  | 1 +
>  drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> index 88d3f9d7dd55..e15391e36b40 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> @@ -52,6 +52,7 @@ static inline unsigned long long complete_integer_division_u64(
>         unsigned long long result;
>
>         ASSERT(divisor);
> +       BUG_ON(!divisor);
ASSERT() calls WARN(), so the warning message will print twice, but I
don't have a better suggestion. :)

Huacai

>
>         result = div64_u64_rem(dividend, divisor, remainder);
>
> diff --git a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
> index 131f1e3949d3..ce2036950808 100644
> --- a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
> @@ -30,6 +30,7 @@ static inline unsigned long long spl_complete_integer_division_u64(
>         unsigned long long result;
>
>         SPL_ASSERT(divisor);
> +       BUG_ON(!divisor);
>
>         result = spl_div64_u64_rem(dividend, divisor, remainder);
>
> --
> 2.42.0
>
>