linux-kernel - Re: [PATCH] ssb-main: Fix division by zero in ssb_calc_clock

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c6d01bf-1a0f-27de-54e1-4afdcf4bc8d5@lwfinger.net>
Date:   Wed, 30 Aug 2023 14:50:42 -0500
From:   Larry Finger <Larry.Finger@...inger.net>
To:     Rand Deeb <deeb.rand@...fident.ru>, Michael Buesch <m@...s.ch>,
        linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     lvc-project@...uxtesting.org, voskresenski.stanislav@...fident.ru
Subject: Re: [PATCH] ssb-main: Fix division by zero in ssb_calc_clock_rate()

On 8/30/23 03:27, Rand Deeb wrote:
> In ssb_calc_clock_rate(), the value of m1 may be zero because it is
> initialized using clkfactor_f6_resolv(). This function could return
> zero, so there is a possibility of dividing by zero, we fixed it by
> checking the values before dividing.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Rand Deeb <deeb.rand@...fident.ru>
> ---
>   drivers/ssb/main.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
> index 0a26984acb2c..e0776a16d04d 100644
> --- a/drivers/ssb/main.c
> +++ b/drivers/ssb/main.c
> @@ -903,13 +903,21 @@ u32 ssb_calc_clock_rate(u32 plltype, u32 n, u32 m)
>   		case SSB_CHIPCO_CLK_MC_BYPASS:
>   			return clock;
>   		case SSB_CHIPCO_CLK_MC_M1:
> -			return (clock / m1);
> +			if 
> +				return (clock / m1);
> +			break;
>   		case SSB_CHIPCO_CLK_MC_M1M2:
> -			return (clock / (m1 * m2));
> +			if ((m1 * m2) !=3D 0)
> +				return (clock / (m1 * m2));
> +			break;
>   		case SSB_CHIPCO_CLK_MC_M1M2M3:
> -			return (clock / (m1 * m2 * m3));
> +			if ((m1 * m2 * m3) !=3D 0)
> +				return (clock / (m1 * m2 * m3));
> +			break;
>   		case SSB_CHIPCO_CLK_MC_M1M3:
> -			return (clock / (m1 * m3));
> +			if ((m1 * m3) !=3D 0)
> +				return (clock / (m1 * m3));
> +			break;
>   		}
>   		return 0;
>   	case SSB_PLLTYPE_2:
> --=20
> 2.34.1

Rand,

I agree that clkfactor_f6_resolv() could return 0, but we have not been overrun 
with reports of divide by zero errors, which suggests that the branch is never 
taken. This patch will make your tool happy and is much simpler:

diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index ab080cf26c9f..b9934b9c2d70 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -837,7 +837,7 @@ static u32 clkfactor_f6_resolve(u32 v)
         case SSB_CHIPCO_CLK_F6_7:
                 return 7;
         }
-       return 0;
+       return 1;
  }

  /* Calculate the speed the backplane would run at a given set of clockcontrol 
values */

Your patch has some technical problems as well. The subject should be "ssb: Fix 
division ..." In addition, note that all your if statements have an extraneous 
"3D" as in "(m1 !=3D 0)". To me, that indicates that your mailer is not sending 
plain text.

Larry