linux-kernel - Re: [PATCH -next] net: mvpp2: Use div64_ul instead of do

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: <cbcdb354-04de-2a9a-1754-c32dd014e859@gmail.com>
Date:   Wed, 17 Nov 2021 20:00:35 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Yang Li <yang.lee@...ux.alibaba.com>, davem@...emloft.net
Cc:     kuba@...nel.org, mw@...ihalf.com, linux@...linux.org.uk,
        ast@...nel.org, daniel@...earbox.net, hawk@...nel.org,
        john.fastabend@...il.com, andrii@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, kpsingh@...nel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH -next] net: mvpp2: Use div64_ul instead of do_div



On 11/17/21 7:05 PM, Yang Li wrote:
> do_div() does a 64-by-32 division. Here the divisor is an
> unsigned long which on some platforms is 64 bit wide. So use
> div64_ul instead of do_div to avoid a possible truncation.
> 
> Eliminate the following coccicheck warning:
> ./drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:2742:1-7: WARNING:
> do_div() does a 64-by-32 division, please consider using div64_ul
> instead.
> 
> Reported-by: Abaci Robot <abaci@...ux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@...ux.alibaba.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index df6c793..41244a5 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -2739,7 +2739,7 @@ static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
>  {
>  	u64 tmp = (u64)cycles * USEC_PER_SEC;
>  
> -	do_div(tmp, clk_hz);
> +	tmp = div64_ul(tmp, clk_hz);
>  
>  	return tmp > U32_MAX ? U32_MAX : tmp;
>  }
> 

This is silly, clk_hz fits in a u32, why pretends it is 64bit ?

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index df6c793f4b1b04eb1f812ce2a3a82a39cde0f68b..1bea316bf13ac42ad65ba326fc1d9e206546766e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -2726,7 +2726,7 @@ static void mvpp2_tx_pkts_coal_set(struct mvpp2_port *port,
        }
 }
 
-static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz)
+static u32 mvpp2_usec_to_cycles(u32 usec, u32 clk_hz)
 {
        u64 tmp = (u64)clk_hz * usec;
 
@@ -2735,7 +2735,7 @@ static u32 mvpp2_usec_to_cycles(u32 usec, unsigned long clk_hz)
        return tmp > U32_MAX ? U32_MAX : tmp;
 }
 
-static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
+static u32 mvpp2_cycles_to_usec(u32 cycles, u32 clk_hz)
 {
        u64 tmp = (u64)cycles * USEC_PER_SEC;
 
@@ -2748,7 +2748,7 @@ static u32 mvpp2_cycles_to_usec(u32 cycles, unsigned long clk_hz)
 static void mvpp2_rx_time_coal_set(struct mvpp2_port *port,
                                   struct mvpp2_rx_queue *rxq)
 {
-       unsigned long freq = port->priv->tclk;
+       u32 freq = port->priv->tclk;
        u32 val = mvpp2_usec_to_cycles(rxq->time_coal, freq);
 
        if (val > MVPP2_MAX_ISR_RX_THRESHOLD) {
@@ -2764,7 +2764,7 @@ static void mvpp2_rx_time_coal_set(struct mvpp2_port *port,
 
 static void mvpp2_tx_time_coal_set(struct mvpp2_port *port)
 {
-       unsigned long freq = port->priv->tclk;
+       u32 freq = port->priv->tclk;
        u32 val = mvpp2_usec_to_cycles(port->tx_time_coal, freq);
 
        if (val > MVPP2_MAX_ISR_TX_THRESHOLD) {