netdev - Re: [PATCH net-next v5 2/2] net: phy: broadcom: Add PTP support for some Broadcom PHYs.

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: <20220521020456.fkgx7s5ymtxd5y2q@bsd-mbp.local>
Date:   Fri, 20 May 2022 19:04:56 -0700
From:   Jonathan Lemon <jonathan.lemon@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
        linux@...linux.org.uk, bcm-kernel-feedback-list@...adcom.com,
        kernel-team@...com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, edumazet@...gle.com
Subject: Re: [PATCH net-next v5 2/2] net: phy: broadcom: Add PTP support for
 some Broadcom PHYs.

On Fri, May 20, 2022 at 10:24:25AM -0700, Florian Fainelli wrote:
> 
> 
> On 5/18/2022 3:39 PM, Jonathan Lemon wrote:
> > This adds PTP support for BCM54210E Broadcom PHYs, in particular,
> > the BCM54213PE, as used in the Rasperry PI CM4.  It has only been
> > tested on that hardware.
> > 
> > Signed-off-by: Jonathan Lemon <jonathan.lemon@...il.com>
> > ---
> [snip]
> 
> Looks good to me, just one question below:
> 
> > +static void bcm_ptp_init(struct bcm_ptp_private *priv)
> > +{
> > +	priv->nse_ctrl = NSE_GMODE_EN;
> > +
> > +	mutex_init(&priv->mutex);
> > +	skb_queue_head_init(&priv->tx_queue);
> > +
> > +	priv->mii_ts.rxtstamp = bcm_ptp_rxtstamp;
> > +	priv->mii_ts.txtstamp = bcm_ptp_txtstamp;
> > +	priv->mii_ts.hwtstamp = bcm_ptp_hwtstamp;
> > +	priv->mii_ts.ts_info = bcm_ptp_ts_info;
> > +
> > +	priv->phydev->mii_ts = &priv->mii_ts;
> > +
> > +	INIT_DELAYED_WORK(&priv->out_work, bcm_ptp_fsync_work);
> 
> Do we need to make sure that we cancel the workqueue in an bcm_ptp_exit()
> function?
> 
> I would imagine that the Ethernet MAC attached to that PHY device having
> stopped its receiver and transmitter should ensure no more packets coming in
> or out, however since this is a delayed/asynchronous work, do not we need to
> protect against use after free?

The workqueue is just mamually creatimg a 1PPS pulse on the SYNC_OUT
pin, no packet activity.  Arguably, the .suspend hook could stop all work,
but that seems out of scope here? (and this phy does not suspend/resume)
-- 
Jonathan   (one handed typist for a few weeks)