[PATCH] ocfs2/cluster: cancel peer work synchronously on listener shutdown

[PATCH] ocfs2/cluster: cancel peer work synchronously on listener shutdown

[Cen Zhang]

Listener shutdown must cancel delayed peer work before destroying the
o2net workqueue. destroy_workqueue() drains the work that is queued,
but delayed work whose timer has not expired is invisible to it and is
queued onto the freed workqueue when the timer fires.

o2net_stop_listening() currently only disconnects peers that can still
be found in the node table, and o2net_disconnect_node() cancels the
delayed work without waiting for a running instance. Two paths leave
armed timers behind.

Node removal disconnects a peer before taking it out of the table. A
heartbeat-up callback sees the persistent error and rearms the connect
and expiry work in between, and the listener shutdown then skips the
peer because it is no longer in the table:

  Peer removal                       Heartbeat / listener shutdown
  ------------                       -----------------------------
  disconnect peer; cancel work
                                     heartbeat-up: rearm delayed work
  remove peer from node table
                                     stop listener
                                       synchronize heartbeat callbacks
                                       skip peer absent from node table
                                       destroy o2net workqueue
                                     delayed-work timer expires
                                       queue on freed workqueue

For a peer that is still in the table, o2net_connect_expired() may be
running while o2net_disconnect_node() cancels: once the disconnect
drops nn_lock, the expiry handler requeues itself and the connect work,
and the flush does not wait for those timers either.

Cancel the connect-expiry, connect and still-up work synchronously for
every peer slot after the disconnect. Expiry can queue connect work, so
cancel it first. Heartbeat callbacks are already disabled and drained
at this point, a removed peer cannot hold a socket container because
that pins the configfs item, and connect work for a removed peer stops
once its node lookup fails, so nothing rearms after the cancellations.

KASAN report, only the relevant frames are kept:

BUG: KASAN: slab-use-after-free in __queue_work+0x1189/0x1600
Read of size 4 at addr ffff8881169069c0 by task swapper/2/0

Call Trace:
 <IRQ>
 __queue_work+0x1189/0x1600
 call_timer_fn+0x176/0x540
 </IRQ>

Allocated by task 486:
 __kmalloc_noprof+0x2de/0x780
 __alloc_workqueue+0x61/0x17a0
 alloc_workqueue_noprof+0xcc/0x130
 o2net_start_listening+0x139/0x7c0
 o2nm_node_local_store+0x4c0/0x810
 configfs_write_iter+0x2f9/0x4f0
 vfs_write+0x639/0x1010
 ksys_write+0x111/0x200

Freed by task 485:
 kfree+0x308/0x580
 rcu_core+0x6be/0x1ed0

Last potentially related work creation:
 __call_rcu_common.constprop.0+0x80/0x280
 pwq_release_workfn+0x740/0xe10

The buggy address belongs to the object at ffff888116906800
 which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 448 bytes inside of
 freed 1024-byte region [ffff888116906800, ffff888116906c00)

Fixes: 98211489d414 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/ocfs2/cluster/tcp.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index 8b3830b601ba..b0cafda827bc 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -2200,10 +2200,24 @@ void o2net_stop_listening(struct o2nm_node *node)
 
 	for (i = 0; i < ARRAY_SIZE(o2net_nodes); i++) {
 		struct o2nm_node *node = o2nm_get_node_by_num(i);
+		struct o2net_node *nn = o2net_nn_from_num(i);
+
 		if (node) {
 			o2net_disconnect_node(node);
 			o2nm_node_put(node);
 		}
+
+		/*
+		 * Delayed work can be rearmed after a disconnect: a heartbeat
+		 * callback requeues it for a node that was disconnected but
+		 * not yet removed, and a running expiry handler requeues
+		 * itself and the connect work. Work waiting on its timer is
+		 * invisible to the flush below, so cancel it synchronously.
+		 * Expiry can queue connect work, so cancel it first.
+		 */
+		cancel_delayed_work_sync(&nn->nn_connect_expired);
+		cancel_delayed_work_sync(&nn->nn_connect_work);
+		cancel_delayed_work_sync(&nn->nn_still_up);
 	}
 
 	/* finish all work and tear down the work queue */