public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Richard Cochran <richardcochran@gmail.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org
Subject: Re: [PATCH] ptp: ocp: fix resource freeing order
Date: Wed, 25 Mar 2026 14:37:16 +0000	[thread overview]
Message-ID: <acPzHPYF8y2Vh1DT@shell.armlinux.org.uk> (raw)
In-Reply-To: <20260325122005.jvis4ydmogfkrxl7@skbuf>

On Wed, Mar 25, 2026 at 02:20:05PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 25, 2026 at 12:05:10PM +0000, Vadim Fedorenko wrote:
> > Commit a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable
> > events") added a call to ptp_disable_all_events() which changes the
> > configuration of pins if they support EXTTS events. In ptp_ocp_detach()
> > pins resources are freed before ptp_clock_unregister() and it leads to
> > use-after-free during driver removal. Fix it by changing the order of
> > free/unregister calls.
> > 
> > Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
> > Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > ---
> >  drivers/ptp/ptp_ocp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> > index d88ab2f86b1b..16fd4804fc6a 100644
> > --- a/drivers/ptp/ptp_ocp.c
> > +++ b/drivers/ptp/ptp_ocp.c
> > @@ -4558,6 +4558,9 @@ ptp_ocp_detach(struct ptp_ocp *bp)
> >  	ptp_ocp_detach_sysfs(bp);
> >  	ptp_ocp_attr_group_del(bp);
> >  	timer_delete_sync(&bp->watchdog);
> > +	if (bp->ptp)
> > +		ptp_clock_unregister(bp->ptp);
> > +	kfree(bp->ptp_info.pin_config);
> >  	ptp_ocp_unregister_ext(bp->ts0);
> >  	ptp_ocp_unregister_ext(bp->ts1);
> >  	ptp_ocp_unregister_ext(bp->ts2);
> > @@ -4575,9 +4578,6 @@ ptp_ocp_detach(struct ptp_ocp *bp)
> >  		clk_hw_unregister_fixed_rate(bp->i2c_clk);
> >  	if (bp->n_irqs)
> >  		pci_free_irq_vectors(bp->pdev);
> > -	if (bp->ptp)
> > -		ptp_clock_unregister(bp->ptp);
> > -	kfree(bp->ptp_info.pin_config);
> >  	device_unregister(&bp->dev);
> >  }
> >  
> > -- 
> > 2.47.3
> >
> 
> The code wasn't safe even before the patch, since user space could have
> emitted a PTP ioctl that passed into the driver in between the
> ptp_ocp_unregister_ext() calls and the subsequent ptp_clock_unregister()
> call. It's just that the patch you blame made it more obvious.

Indeed.

We've been talking about "setup, then publish interfaces, unpublish
interfaces, tear down" for decades. I remember it coming up at the
Ottawa Linux Symposiums in the early 2000s when the driver model was
in its infancy. Yet, here we are in 2026, and the same problem
persists. :(

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

      reply	other threads:[~2026-03-25 14:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-25 12:05 [PATCH] ptp: ocp: fix resource freeing order Vadim Fedorenko
2026-03-25 12:20 ` Vladimir Oltean
2026-03-25 14:37   ` Russell King (Oracle) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acPzHPYF8y2Vh1DT@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=vadim.fedorenko@linux.dev \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox