[PATCH] PCI: endpoint: pci-epf-ntb: cancel cmd_handler synchronously on cleanup

[PATCH] PCI: endpoint: pci-epf-ntb: cancel cmd_handler synchronously on cleanup

[Chengyu Liu]

epf_ntb_cmd_handler() re-arms itself unconditionally every 5ms at
reset_handler, with no guard at all.  epf_ntb_epc_cleanup_interface()
uses cancel_delayed_work(), which cannot stop a running callback: if
the cleanup (configfs unbind) happens while the handler is mid-flight,
the handler re-arms, and the delayed work fires after the devm-managed
ntb_epc - and the devm_ioremap'd ntb_epc->reg it dereferences - have
been released.  Use-after-free.

The per-attempt race window is small (sub-microsecond callback vs a 5ms
period), but configfs bind/unbind cycles can simply be repeated (root)
until the race is won.

Use cancel_delayed_work_sync(), which waits for a running callback and
prevents its re-arm from taking effect.

Found by static analysis.  The race was reproduced on a KASAN build
with a small module exercising this exact arm/cancel/free pattern
(no NTB hardware available); PoC and splat are below.

Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---

Reproducer notes (PoC source below; a module parameter selects the
shape): mode=2 (cancel_delayed_work + unconditional re-arm, this bug's shape) produces

    [    1.95] BUG: KASAN: slab-use-after-free in queue_delayed_work_on+0x27/0x70
    [    1.95] Workqueue: events pm_td_cb [pm_flush_poc]

after the teardown frees the container; mode=1
(cancel_delayed_work_sync, i.e. this fix) stays clean.

---------------------------- reproducer: pm_flush_poc.c ----------------------------
// SPDX-License-Identifier: GPL-2.0
/*
 * pm_teardown_poc - mechanism PoC for the drv-teardown-sync C/D variants
 * (F-026 ipoib / F-028 epf-ntb / F-029 habanalabs).
 *
 * Modes (module parameter):
 *   0  F-029 vuln shape:  flush_delayed_work + lockless guard
 *   1  control:           cancel_delayed_work_sync (>=6.9 swallows re-arm)
 *   2  F-028 vuln shape:  cancel_delayed_work (non-sync) + unconditional re-arm
 *   3  F-026 vuln shape:  cancel_delayed_work (non-sync) + guard check/queue
 *                         race window (test passes, teardown slips in before
 *                         the queue_delayed_work)
 *
 * Every vuln mode must produce KASAN slab-use-after-free when the orphaned
 * work fires after kfree; the control mode must stay clean.
 */
#include <linux/module.h>
#include <linux/workqueue.h>
#include <linux/slab.h>
#include <linux/delay.h>

static int mode;
module_param(mode, int, 0444);
MODULE_PARM_DESC(mode, "0=flush(F-029) 1=cancel_sync(ctrl) 2=cancel_nosync(F-028) 3=cancel_nosync+guard-race(F-026)");

struct pm_td_ctx {
	bool			fini_pending;
	struct delayed_work	work;
};

static struct pm_td_ctx *ctx;

static void pm_td_cb(struct work_struct *w)
{
	struct pm_td_ctx *c = container_of(to_delayed_work(w),
					   struct pm_td_ctx, work);

	pr_info("PM-POC: cb running, fini_pending=%d\n", c->fini_pending);

	switch (mode) {
	case 0:		/* F-029: lockless guard then long body then re-arm */
		if (c->fini_pending)
			return;
		msleep(300);
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed (flush shape)\n");
		break;
	case 2:		/* F-028: unconditional re-arm, cancel must catch running cb */
		msleep(300);	/* long body so cancel lands mid-run */
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed (no guard)\n");
		break;
	case 3:		/* F-026: guard check passes, THEN teardown slips in */
		if (c->fini_pending)
			return;
		/* widen the check->queue race window like ipoib's 724->725 */
		msleep(300);
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed past guard (test-queue race)\n");
		break;
	default:
		break;
	}
}

static int __init pm_poc_init(void)
{
	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
	if (!ctx)
		return -ENOMEM;
	ctx->fini_pending = false;
	INIT_DELAYED_WORK(&ctx->work, pm_td_cb);

	pr_info("PM-POC: arming, mode=%d\n", mode);
	schedule_delayed_work(&ctx->work, 0);

	msleep(50);	/* callback is mid-run now */

	/* teardown sequence for the selected variant */
	ctx->fini_pending = true;
	switch (mode) {
	case 1:
		cancel_delayed_work_sync(&ctx->work);
		break;
	case 0:
		flush_delayed_work(&ctx->work);
		break;
	default:
		cancel_delayed_work(&ctx->work);	/* non-sync: C variants */
		break;
	}

	pr_info("PM-POC: teardown done, freeing ctx\n");
	kfree(ctx);

	msleep(600);
	pr_info("PM-POC: SURVIVED (no KASAN splat)\n");
	return 0;
}

static void __exit pm_poc_exit(void) { }

module_init(pm_poc_init);
module_exit(pm_poc_exit);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("drv-teardown-sync C/D variant mechanism PoC (F-026/028/029)");

 drivers/pci/endpoint/functions/pci-epf-ntb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c
index 5314aca218..a0f0c8eb48 100644
--- a/drivers/pci/endpoint/functions/pci-epf-ntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c
@@ -1754,7 +1754,7 @@ static void epf_ntb_epc_cleanup_interface(struct epf_ntb *ntb,
 		return;
 
 	ntb_epc = ntb->epc[type];
-	cancel_delayed_work(&ntb_epc->cmd_handler);
+	cancel_delayed_work_sync(&ntb_epc->cmd_handler);
 	epf_ntb_db_mw_bar_cleanup(ntb, type);
 	epf_ntb_peer_spad_bar_clear(ntb_epc);
 	epf_ntb_config_sspad_bar_clear(ntb_epc);
-- 
2.25.1

Re: [PATCH] PCI: endpoint: pci-epf-ntb: cancel cmd_handler synchronously on cleanup

[sashiko-bot]

> epf_ntb_cmd_handler() re-arms itself unconditionally every 5ms at
> reset_handler, with no guard at all.  epf_ntb_epc_cleanup_interface()
> uses cancel_delayed_work(), which cannot stop a running callback: if
> the cleanup (configfs unbind) happens while the handler is mid-flight,
> the handler re-arms, and the delayed work fires after the devm-managed
> ntb_epc - and the devm_ioremap'd ntb_epc->reg it dereferences - have
> been released.  Use-after-free.
> 
> The per-attempt race window is small (sub-microsecond callback vs a 5ms
> period), but configfs bind/unbind cycles can simply be repeated (root)
> until the race is won.
> 
> Use cancel_delayed_work_sync(), which waits for a running callback and
> prevents its re-arm from taking effect.
> 
> Found by static analysis.  The race was reproduced on a KASAN build
> with a small module exercising this exact arm/cancel/free pattern
> (no NTB hardware available); PoC and splat are below.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103161622.1597925-1-lui2012qwq@gmail.com?part=1