netdev - Re: [PATCH net] net: sparx5: fix error handling in sparx5_port

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:   Tue, 22 Nov 2022 11:10:10 +0100
From:   Steen Hegelund <steen.hegelund@...rochip.com>
To:     Liu Jian <liujian56@...wei.com>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <lars.povlsen@...rochip.com>, <daniel.machon@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>, <linux@...linux.org.uk>,
        <horatiu.vultur@...rochip.com>, <bjarni.jonasson@...rochip.com>
CC:     <netdev@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH net] net: sparx5: fix error handling in
 sparx5_port_open()

Hi Liu,

Oops, Incorrect tagging...

Thanks for your patch, the changes look good to me.  We also did a round of
testing on the platform, simulating errors to see this in effect.
 

On Tue, 2022-11-22 at 10:28 +0100, Steen Hegelund wrote:
> Hi Liu,
> 
> Thanks for your patch, the changes look good to me.  We also did a round of
> testing on the platform, simulating errors to see this in effect.
> 
> On Thu, 2022-11-17 at 20:59 +0800, Liu Jian wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> > 
> > If phylink_of_phy_connect() fails, the port should be disabled.
> > If sparx5_serdes_set()/phy_power_on() fails, the port should be
> > disabled and the phylink should be stopped and disconnected.
> > 
> > Fixes: 946e7fd5053a ("net: sparx5: add port module support")
> > Fixes: f3cad2611a77 ("net: sparx5: add hostmode with phylink support")
> > Signed-off-by: Liu Jian <liujian56@...wei.com>
> > ---
> >  .../net/ethernet/microchip/sparx5/sparx5_netdev.c  | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> > b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> > index 19516ccad533..d078156581d5 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
> > @@ -104,7 +104,7 @@ static int sparx5_port_open(struct net_device *ndev)
> >         err = phylink_of_phy_connect(port->phylink, port->of_node, 0);
> >         if (err) {
> >                 netdev_err(ndev, "Could not attach to PHY\n");
> > -               return err;
> > +               goto err_connect;
> >         }
> > 
> >         phylink_start(port->phylink);
> > @@ -116,10 +116,20 @@ static int sparx5_port_open(struct net_device *ndev)
> >                         err = sparx5_serdes_set(port->sparx5, port, &port-
> > > conf);
> >                 else
> >                         err = phy_power_on(port->serdes);
> > -               if (err)
> > +               if (err) {
> >                         netdev_err(ndev, "%s failed\n", __func__);
> > +                       goto out_power;
> > +               }
> >         }
> > 
> > +       return 0;
> > +
> > +out_power:
> > +       phylink_stop(port->phylink);
> > +       phylink_disconnect_phy(port->phylink);
> > +err_connect:
> > +       sparx5_port_enable(port, false);
> > +
> >         return err;
> >  }
> > 
> > --
> > 2.17.1
> > 
> 
> Tested-off-by: Bjarni Jonasson <bjarni.jonasson@...rochip.com>
> Reviewed-off-by: Lars Povlsen <steen.hegelund@...rochip.com>
> 
> BR
> Steen
> 

Tested-by: Bjarni Jonasson <bjarni.jonasson@...rochip.com>
Reviewed-by: Steen Hegelund <steen.hegelund@...rochip.com>

BR
Steen