linux-kernel - Re: [PATCH net] net: lan966x: fix kernel oops on ioctl when I/F is down

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: <Yj53rrvg4+DN68W4@lunn.ch>
Date:   Sat, 26 Mar 2022 03:17:18 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Walle <michael@...le.cc>
Cc:     Horatiu Vultur <horatiu.vultur@...rochip.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: lan966x: fix kernel oops on ioctl when I/F is
 down

On Sat, Mar 26, 2022 at 01:02:51AM +0100, Michael Walle wrote:
> A SIOCGMIIPHY ioctl will cause a kernel oops when the interface is down.
> Fix it by checking the state and if it's no running, return an error.

s/no/not/

I don't think it is just SIOCGMIIPHY. phy_has_hwtstamp(dev->phydev) is
probably also an issue. The phy is connected in open, and disconnected
in stop. So dev->phydev is not valid outside of that time.

But i'm also not sure it is guaranteed to be valid while the interface
is up. The driver uses phylink, so there could be an SFP attached to a
port, in which case, dev->phydev will not be set.

So rather than testing of running, it would be better to test if the
phydev is NULL or not.

       Andrew

> 
> Fixes: 735fec995b21 ("net: lan966x: Implement SIOCSHWTSTAMP and SIOCGHWTSTAMP")
> Signed-off-by: Michael Walle <michael@...le.cc>
> ---
>  drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index ec42e526f6fb..0adf49d19142 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -399,6 +399,9 @@ static int lan966x_port_ioctl(struct net_device *dev, struct ifreq *ifr,
>  {
>  	struct lan966x_port *port = netdev_priv(dev);
>  
> +	if (!netif_running(dev))
> +		return -EINVAL;
> +
>  	if (!phy_has_hwtstamp(dev->phydev) && port->lan966x->ptp) {
>  		switch (cmd) {
>  		case SIOCSHWTSTAMP:
> -- 
> 2.30.2
>