netdev - Re: [PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero to PTR

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]

Date:   Mon, 5 Dec 2022 14:42:21 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Divya Koppera <Divya.Koppera@...rochip.com>
Cc:     hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        richardcochran@...il.com, UNGLinuxDriver@...rochip.com,
        Madhuri.Sripada@...rochip.com
Subject: Re: [PATCH v4 net-next 2/2] net: phy: micrel: Fix warn: passing zero
 to PTR_ERR

On Mon, Dec 05, 2022 at 04:05:50PM +0530, Divya Koppera wrote:
> Handle the NULL pointer case
> 
> Fixes New smatch warnings:
> drivers/net/phy/micrel.c:2613 lan8814_ptp_probe_once() warn: passing zero to 'PTR_ERR'
> 
> vim +/PTR_ERR +2613 drivers/net/phy/micrel.c
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
> Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy")
> Signed-off-by: Divya Koppera <Divya.Koppera@...rochip.com>
> ---
> v3 -> v4:
> - Split the patch for different warnings
> - Renamed variable from shared_priv to shared.
> 
> v2 -> v3:
> - Changed subject line from net to net-next
> - Removed config check for ptp and clock configuration
>   instead added null check for ptp_clock
> - Fixed one more warning related to initialisaton.
> 
> v1 -> v2:
> - Handled NULL pointer case
> - Changed subject line with net-next to net
> ---
>  drivers/net/phy/micrel.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 1bcdb828db56..0399f3830700 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -2971,12 +2971,13 @@ static int lan8814_config_intr(struct phy_device *phydev)
>  
>  static void lan8814_ptp_init(struct phy_device *phydev)
>  {
> +	struct lan8814_shared_priv *shared = phydev->shared->priv;
>  	struct kszphy_priv *priv = phydev->priv;
>  	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
>  	u32 temp;
>  
> -	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
> -	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
> +	/* Check if PHC support is missing at the configuration level */
> +	if (!shared->ptp_clock)
>  		return;

Can you somehow keep the IS_ENABLED() ? It gets evaluated at compile
time. The optimizer can see the function will always return here, and
all the code that follows is pointless, and so remove it. By turning
this into a runtime test, you have made the image bigger.

     Andrew