Re: [PATCH net-next 4/4] ethtool: runtime-resume netdev parent in ethnl_ops_begin

On 01.08.21 13:41, Heiner Kallweit wrote:

If a network device is runtime-suspended then:
- network device may be flagged as detached and all ethtool ops (even if not
  accessing the device) will fail because netif_device_present() returns
  false
- ethtool ops may fail because device is not accessible (e.g. because being
  in D3 in case of a PCI device)

It may not be desirable that userspace can't use even simple ethtool ops
that not access the device if interface or link is down. To be more friendly
to userspace let's ensure that device is runtime-resumed when executing the
respective ethtool op in kernel.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 net/ethtool/netlink.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

[...]

 
 void ethnl_ops_complete(struct net_device *dev)
 {
 	if (dev && dev->ethtool_ops->complete)
 		dev->ethtool_ops->complete(dev);
+
+	if (dev->dev.parent)
+		pm_runtime_put(dev->dev.parent);
 }
 
 /**

Hello Heiner,

Coverity complains that we checked dev != NULL earlier but now
unconditionally dereference it:


*** CID 1506213:  Null pointer dereferences  (FORWARD_NULL)
/net/ethtool/netlink.c: 67 in ethnl_ops_complete()
61     
62     void ethnl_ops_complete(struct net_device *dev)
63     {
64     	if (dev && dev->ethtool_ops->complete)
65     		dev->ethtool_ops->complete(dev);
66     

quoted

quoted

    CID 1506213:  Null pointer dereferences  (FORWARD_NULL)
    Dereferencing null pointer "dev".

67     	if (dev->dev.parent)
68     		pm_runtime_put(dev->dev.parent);
69     }
70     
71     /**
72      * ethnl_parse_header_dev_get() - parse request header