Netdev List
 help / color / mirror / Atom feed
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: Richard Cochran <richardcochran@gmail.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org, Vadim Fedorenko <vadim.fedorenko@linux.dev>
Subject: [PATCH net v5] ptp: ocp: fix resource freeing order
Date: Mon,  8 Jun 2026 15:59:52 +0000	[thread overview]
Message-ID: <20260608155952.240304-1-vadim.fedorenko@linux.dev> (raw)

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. To avoid irq handler running on the other core
while ptp device unregistering, call synchronize_irq() after HW is
configured to stop producing irqs and no irqs are in-flight.

Fixes: a60fc3294a37 ("ptp: rework ptp_clock_unregister() to disable events")
Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
v4 -> v5:
* disable interrupts on ts0 and pps devices as well
v3 -> v4:
* address issues flagged by Sashiko by adding synchronize_irq() after
* disabling HW irq
v2 -> v3:
* fix code issue in v2
v1 -> v2:
* on v1 sashiko mentioned that some IRQs may fire after bp->ptp freed.
  it looks like a false positive, because ptp_clock_unregister() will
  explicitly disable all EXTTS channels before destroying ptp_clock
  structure. But there is a possibility of timestampers being active
  without PTP_PF_EXTTS function set. It can be done through sysfs files
  and it will change pins configuration directly, but can only be done
  for TS1..TS4. Disable them explicitly after detaching sysfs files.

Even though there is still technically a small chance that PTP_CLK_REQ_EXTTS
will be enabled again on PPS device before it is unregistered, it's
better to have this fix committed as current behavior leads to UAF and
crashes during device removals:

Jun  8 15:36:44 host  [ 1535.329384] ==================================================================
Jun  8 15:36:44 host  [ 1535.329425] BUG: KASAN: slab-use-after-free in ptp_ocp_enable+0x6ce/0x7e0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329455] Read of size 8 at addr ffff8881469a99d0 by task modprobe/6021
Jun  8 15:36:44 host  [ 1535.329474]
Jun  8 15:36:44 host  [ 1535.329484] CPU: 25 UID: 0 PID: 6021 Comm: modprobe Kdump: loaded Tainted: G            E       7.0.11-1.el10.x86_64 #1 PREEMPT(full)
Jun  8 15:36:44 host  [ 1535.329490] Tainted: [E]=UNSIGNED_MODULE
Jun  8 15:36:44 host  [ 1535.329491] Hardware name: HPE ProLiant DL380 Gen10/ProLiant DL380 Gen10, BIOS U30 01/16/2025
Jun  8 15:36:44 host  [ 1535.329494] Call Trace:
Jun  8 15:36:44 host  [ 1535.329495]  <TASK>
Jun  8 15:36:44 host  [ 1535.329498]  dump_stack_lvl+0x56/0x80
Jun  8 15:36:44 host  [ 1535.329504]  print_report+0x174/0x4e3
Jun  8 15:36:44 host  [ 1535.329507]  ? _raw_read_unlock_irqrestore+0x50/0x50
Jun  8 15:36:44 host  [ 1535.329513]  ? kasan_save_free_info+0x37/0x50
Jun  8 15:36:44 host  [ 1535.329517]  ? kfree+0x191/0x400
Jun  8 15:36:44 host  [ 1535.329523]  ? ptp_ocp_enable+0x6ce/0x7e0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329529]  kasan_report+0xb5/0xe0
Jun  8 15:36:44 host  [ 1535.329533]  ? ptp_ocp_enable+0x6ce/0x7e0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329540]  ptp_ocp_enable+0x6ce/0x7e0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329546]  ? kobject_put+0x175/0x4e0
Jun  8 15:36:44 host  [ 1535.329554]  ? signal_store+0x4f0/0x4f0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329560]  ? kobject_put+0x5d/0x4e0
Jun  8 15:36:44 host  [ 1535.329564]  ? mutex_lock+0x82/0xd0
Jun  8 15:36:44 host  [ 1535.329569]  ? __mutex_lock_slowpath+0x10/0x10
Jun  8 15:36:44 host  [ 1535.329574]  ptp_disable_all_events+0x22a/0x2e0
Jun  8 15:36:44 host  [ 1535.329578]  ? ptp_disable_pinfunc.isra.0+0x140/0x140
Jun  8 15:36:44 host  [ 1535.329582]  ? kobject_put+0x175/0x4e0
Jun  8 15:36:44 host  [ 1535.329586]  ptp_clock_unregister+0xaf/0x210
Jun  8 15:36:44 host  [ 1535.329589]  ptp_ocp_detach+0x350/0x480 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329596]  ptp_ocp_remove+0x14c/0x1e0 [ptp_ocp]
Jun  8 15:36:44 host  [ 1535.329604]  pci_device_remove+0x9e/0x1d0
Jun  8 15:36:44 host  [ 1535.329609]  device_release_driver_internal+0x37c/0x530
Jun  8 15:36:44 host  [ 1535.329615]  driver_detach+0xbb/0x170

---
 drivers/ptp/ptp_ocp.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index beacc2ffb166..735385539b9f 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2479,8 +2479,13 @@ ptp_ocp_ts_enable(void *priv, u32 req, bool enable)
 		iowrite32(1, &reg->intr_mask);
 		iowrite32(1, &reg->intr);
 	} else {
+		int irq_vec = pci_irq_vector(bp->pdev, ext->irq_vec);
+
 		iowrite32(0, &reg->intr_mask);
 		iowrite32(0, &reg->enable);
+		ioread32(&reg->intr_mask);
+		if (irq_vec > 0)
+			synchronize_irq(irq_vec);
 	}
 
 	return 0;
@@ -4867,6 +4872,22 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 	ptp_ocp_detach_sysfs(bp);
 	ptp_ocp_attr_group_del(bp);
 	timer_delete_sync(&bp->watchdog);
+	/* Disable interrupts on all timestampers */
+	if (bp->ts0)
+		ptp_ocp_ts_enable(bp->ts0, 0, false);
+	if (bp->ts1)
+		ptp_ocp_ts_enable(bp->ts1, 0, false);
+	if (bp->ts2)
+		ptp_ocp_ts_enable(bp->ts2, 0, false);
+	if (bp->ts3)
+		ptp_ocp_ts_enable(bp->ts3, 0, false);
+	if (bp->ts4)
+		ptp_ocp_ts_enable(bp->ts4, 0, false);
+	if (bp->pps)
+		ptp_ocp_ts_enable(bp->pps, ~0, false);
+	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);
@@ -4884,9 +4905,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


                 reply	other threads:[~2026-06-08 16:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260608155952.240304-1-vadim.fedorenko@linux.dev \
    --to=vadim.fedorenko@linux.dev \
    --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=rmk+kernel@armlinux.org.uk \
    --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