netdev - Re: [PATCH v3 03/13] net: phy: at803x: improve the WOL feature

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, 18 Oct 2021 20:41:05 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Luo Jie <luoj@...eaurora.org>
Cc:     hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
        kuba@...nel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, sricharan@...eaurora.org
Subject: Re: [PATCH v3 03/13] net: phy: at803x: improve the WOL feature

> @@ -348,18 +349,29 @@ static int at803x_set_wol(struct phy_device *phydev,
>  			phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i],
>  				      mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
>  
> +		/* Enable WOL function */
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_PHY_MMD3_WOL_CTRL,
> +				0, AT803X_WOL_EN);
> +		if (ret)
> +			return ret;
> +		/* Enable WOL interrupt */
>  		ret = phy_modify(phydev, AT803X_INTR_ENABLE, 0, AT803X_INTR_ENABLE_WOL);
>  		if (ret)
>  			return ret;
> -		value = phy_read(phydev, AT803X_INTR_STATUS);
>  	} else {
> +		/* Disable WoL function */
> +		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_PHY_MMD3_WOL_CTRL,
> +				AT803X_WOL_EN, 0);
> +		if (ret)
> +			return ret;
> +		/* Disable WOL interrupt */
>  		ret = phy_modify(phydev, AT803X_INTR_ENABLE, AT803X_INTR_ENABLE_WOL, 0);
>  		if (ret)
>  			return ret;
> -		value = phy_read(phydev, AT803X_INTR_STATUS);
>  	}
>  
> -	return ret;
> +	/* Clear WOL status */
> +	return phy_read(phydev, AT803X_INTR_STATUS);

It looks like you could be clearing other interrupt bits which have
not been serviced yet. Is it possible to clear just WoL?

Also, you are returning the contents of the interrupt status register?
You should probably be returning 0 if the read was successful.

    Andrew