Re: [PATCH v9 net-next 12/12] net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from converted drivers

[Vladimir Oltean]

On Fri, Aug 04, 2023 at 09:21:28AM +0200, Eric Dumazet wrote:
> > +/**
> > + * dev_set_hwtstamp_phylib() - Change hardware timestamping of NIC
> > + *     or of attached phylib PHY
> > + * @dev: Network device
> > + * @cfg: Timestamping configuration structure
> > + * @extack: Netlink extended ack message structure, for error reporting
> > + *
> > + * Helper for enforcing a common policy that phylib timestamping, if available,
> > + * should take precedence in front of hardware timestamping provided by the
> > + * netdev. If the netdev driver needs to perform specific actions even for PHY
> > + * timestamping to work properly (a switch port must trap the timestamped
> > + * frames and not forward them), it must set IFF_SEE_ALL_HWTSTAMP_REQUESTS in
> > + * dev->priv_flags.
> > + */
> > +static int dev_set_hwtstamp_phylib(struct net_device *dev,
> > +                                  struct kernel_hwtstamp_config *cfg,
> > +                                  struct netlink_ext_ack *extack)
> > +{
> > +       const struct net_device_ops *ops = dev->netdev_ops;
> > +       bool phy_ts = phy_has_hwtstamp(dev->phydev);
> > +       struct kernel_hwtstamp_config old_cfg = {};
> > +       bool changed = false;
> > +       int err;
> > +
> > +       cfg->source = phy_ts ? HWTSTAMP_SOURCE_PHYLIB : HWTSTAMP_SOURCE_NETDEV;
> > +
> > +       if (!phy_ts || (dev->priv_flags & IFF_SEE_ALL_HWTSTAMP_REQUESTS)) {
> > +               err = ops->ndo_hwtstamp_get(dev, &old_cfg);
> 
> old_cfg.ifr is NULL at this point.
> 
> This causes a crash later in generic_hwtstamp_ioctl_lower()
> 
>     ifrr.ifr_ifru = kernel_cfg->ifr->ifr_ifru;

Thanks for reporting. My control flow is all wrong, it seems. I've sent
this patch for addressing the breakage:

https://lore.kernel.org/netdev/20230804134939.3109763-1-vladimir.oltean@xxxxxxx/