netdev - Re: [PATCH] net-sysfs: allow changing sysfs carrier when interface 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4b700cbc93bc087115c1e400449bdff48c37298d.camel@infinera.com>
Date:   Thu, 2 Jun 2022 09:22:34 +0000
From:   Joakim Tjernlund <Joakim.Tjernlund@...inera.com>
To:     "kuba@...nel.org" <kuba@...nel.org>
CC:     "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] net-sysfs: allow changing sysfs carrier when interface is
 down

On Wed, 2022-06-01 at 18:01 -0700, Jakub Kicinski wrote:
> On Thu, 2 Jun 2022 02:35:23 +0200 Joakim Tjernlund wrote:
> > UP/DOWN and carrier are async events and it makes sense one can
> > adjust carrier in sysfs before bringing the interface up.
> 
> Can you explain your use case?

Sure, our HW has config/state changes that makes it impossible for net driver
to touch and registers or TX pkgs(can result in System Error exception in worst case.

So the user space app handlings this needs to make sure that even if say dchp
brings an I/F up, there can be no HW access by the driver.
To do that, carrier needs to be controlled before I/F is brought up.

Carrier reflects actual link status and this kan change at any time. I honestly
don't understand why you would prevent sysfs access to carrier just
because I/F is down? 

> 
> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@...inera.com>
> > Cc: stable@...r.kernel.org
> 
> Seems a little too risky of a change to push into stable.

The change is minimal and only allows access to carrier when I/F is also down.
I think this is a kernel bug and should go to stable too.

> 
> > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> > index a4ae65263384..3418ef7ef2d8 100644
> > --- a/net/core/net-sysfs.c
> > +++ b/net/core/net-sysfs.c
> > @@ -167,8 +167,6 @@ static DEVICE_ATTR_RO(broadcast);
> >  
> >  static int change_carrier(struct net_device *dev, unsigned long new_carrier)
> >  {
> > -	if (!netif_running(dev))
> > -		return -EINVAL;
> >  	return dev_change_carrier(dev, (bool)new_carrier);
> >  }