netdev - Re: [PATCH net 1/4] igc: Report speed and duplex as unknown when device is runtime suspended

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: <20210201140421.33c176a0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 1 Feb 2021 14:04:21 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Neftin, Sasha" <sasha.neftin@...el.com>
Cc:     Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        netdev@...r.kernel.org, sassmann@...hat.com,
        "Lifshits, Vitaly" <vitaly.lifshits@...el.com>,
        "Keller, Jacob E" <jacob.e.keller@...el.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>
Subject: Re: [PATCH net 1/4] igc: Report speed and duplex as unknown when
 device is runtime suspended

On Sun, 31 Jan 2021 12:22:25 +0200 Neftin, Sasha wrote:
> On 1/30/2021 20:12, Jakub Kicinski wrote:
> > On Sat, 30 Jan 2021 16:00:06 +0200 Neftin, Sasha wrote:  
> >> What is another rd32(IGC_STATUS) you meant? in  igc_ethtool_get_regs?  
> > 
> > Yes.
> >   
> >> While the device in D3 state there is no configuration space registers
> >> access.  
> > 
> > That's to say similar stack trace will be generated to the one fixed
> > here, if someone runs ethtool -d, correct? I don't see anything
> > checking runtime there either.
> >   
> yes.
> This problem crosses many drivers. (not only igb, igc,...)
> 
> specific to this one (igc), can we check 'netif_running at begin of the 
> _get_regs method:
> if (!netif_running(netdev))
> 	return;
> what do you think? (only OS can put device to the D3)

That'd address the particular issue we noticed in the 5min review of
this patch, but similar, less obvious problems may still be lurking?
I wish I knew more about PM so I could suggest a solution. It'd be
ideal to avoid the rtnl_lock calls in resume, so that the driver can
just wake up the device from within the callbacks. Maybe embedded
experts can chime in and suggest how it's usually handled..