A region on o2hb_all_regions must have an initialized config item and
name. o2hb_heartbeat_group_make_item() currently links the zeroed region
under o2hb_live_lock, drops the lock, and only then initializes hr_item.
The lock therefore protects the list without protecting initialization
from readers of the published item.
With global heartbeat enabled, a DLM_QUERY_REGION from the current
joining node can pass the existing-domain and protocol checks while a
configfs mkdir creates another region. The DLM locks are not taken by
the constructor, and the network worker does not take the configfs
subsystem mutex. The following ordering is possible:
Configfs mkdir O2net receive worker
-------------- --------------------
Allocate reg
Lock o2hb_live_lock
Link into o2hb_all_regions
Unlock o2hb_live_lock
dlm_query_region_handler()
dlm_match_regions()
o2hb_get_all_regions()
Lock o2hb_live_lock
Copy 32 bytes from NULL name
Unlock o2hb_live_lock
config_item_init_type_name()
The query thus dereferences NULL. Initialize hr_item before linking the
region so the existing list lock also orders name initialization before
any reader. Reject a failed name allocation, since the initialization
helper does not propagate that error.
Use config_item_put() on initialized constructor failures to release the
name and respect references retained by o2hb_region_pin(). Initialize
the list heads, mutex and delayed works needed by o2hb_region_release()
before any such put, and leave the list node self-linked after rollback.
Keep the existing handler drain before removing a failed published item.
KASAN report as below:
==================================================================
BUG: KASAN: null-ptr-deref in o2hb_get_all_regions+0xbb/0x1f0
Read of size 32 at addr 0000000000000000 by task kworker/u16:1/14
CPU: 2 UID: 0 PID: 14 Comm: kworker/u16:1 Not tainted 7.3.0-rc4-next-20260921-pmb-ocfs2-functional-v1+ #1 PREEMPT(lazy)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Workqueue: o2net o2net_rx_until_empty
Call Trace:
dump_stack_lvl+0x93/0xd0
kasan_report+0xe0/0x110
? o2hb_get_all_regions+0xbb/0x1f0
kasan_check_range+0x105/0x1b0
__asan_memcpy+0x23/0x60
o2hb_get_all_regions+0xbb/0x1f0
dlm_query_region_handler+0x4c2/0xc20
? __pfx_dlm_query_region_handler+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? srso_alias_return_thunk+0x5/0xfbef5
? trace_hardirqs_on+0x18/0x160
? srso_alias_return_thunk+0x5/0xfbef5
? kvm_clock_get_cycles+0x31/0x60
? srso_alias_return_thunk+0x5/0xfbef5
? __pfx_dlm_query_region_handler+0x10/0x10
o2net_rx_until_empty+0x1a50/0x3200
? reacquire_held_locks+0xdd/0x200
? __pfx_o2net_rx_until_empty+0x10/0x10
? lock_acquire+0x190/0x300
? process_one_work+0x935/0x1b40
? process_one_work+0x834/0x1b40
? srso_alias_return_thunk+0x5/0xfbef5
? lock_release+0xc8/0x290
? srso_alias_return_thunk+0x5/0xfbef5
process_one_work+0x9a8/0x1b40
? __pfx_process_one_work+0x10/0x10
? lock_acquire+0x190/0x300
? lock_is_held_type+0x8f/0x100
? srso_alias_return_thunk+0x5/0xfbef5
worker_thread+0x65c/0xe40
? srso_alias_return_thunk+0x5/0xfbef5
? __kthread_parkme+0x177/0x220
? srso_alias_return_thunk+0x5/0xfbef5
? __pfx_worker_thread+0x10/0x10
kthread+0x351/0x460
? srso_alias_return_thunk+0x5/0xfbef5
? __pfx_kthread+0x10/0x10
ret_from_fork+0x659/0x940
? __pfx_ret_from_fork+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? __switch_to+0x74f/0xf70
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
==================================================================
Fixes: 536f0741f324 ("ocfs2/cluster: Track number of global heartbeat regions")
Cc:
Signed-off-by: Cen Zhang
Assisted-by: LLM
---
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 8e6ead433228..9a4703bdc4e8 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -2140,6 +2140,18 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
}
reg->hr_node_num = O2NM_MAX_NODES;
+ INIT_LIST_HEAD(®->hr_all_item);
+ mutex_init(®->hr_arming_mutex);
+ reg->hr_stopping = true;
+ INIT_DELAYED_WORK(®->hr_write_timeout_work, o2hb_write_timeout);
+ INIT_DELAYED_WORK(®->hr_nego_timeout_work, o2hb_nego_timeout);
+ INIT_LIST_HEAD(®->hr_handler_list);
+ config_item_init_type_name(®->hr_item, name, &o2hb_region_type);
+ if (!config_item_name(®->hr_item)) {
+ ret = -ENOMEM;
+ goto put_item;
+ }
+
spin_lock(&o2hb_live_lock);
reg->hr_region_num = 0;
if (o2hb_global_heartbeat_active()) {
@@ -2148,26 +2160,19 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
if (reg->hr_region_num >= O2NM_MAX_REGIONS) {
spin_unlock(&o2hb_live_lock);
ret = -EFBIG;
- goto free;
+ goto put_item;
}
set_bit(reg->hr_region_num, o2hb_region_bitmap);
}
list_add_tail(®->hr_all_item, &o2hb_all_regions);
spin_unlock(&o2hb_live_lock);
- config_item_init_type_name(®->hr_item, name, &o2hb_region_type);
-
/* this is the same way to generate msg key as dlm, for local heartbeat,
* name is also the same, so make initial crc value different to avoid
* message key conflict.
*/
reg->hr_key = crc32_le(reg->hr_region_num + O2NM_MAX_REGIONS,
name, strlen(name));
- mutex_init(®->hr_arming_mutex);
- reg->hr_stopping = true;
- INIT_DELAYED_WORK(®->hr_write_timeout_work, o2hb_write_timeout);
- INIT_DELAYED_WORK(®->hr_nego_timeout_work, o2hb_nego_timeout);
- INIT_LIST_HEAD(®->hr_handler_list);
ret = o2net_register_handler(O2HB_NEGO_TIMEOUT_MSG, reg->hr_key,
sizeof(struct o2hb_nego_msg),
o2hb_nego_timeout_handler,
@@ -2190,10 +2195,13 @@ unregister_handler:
o2net_unregister_and_flush_handler_list(®->hr_handler_list);
remove_item:
spin_lock(&o2hb_live_lock);
- list_del(®->hr_all_item);
+ list_del_init(®->hr_all_item);
if (o2hb_global_heartbeat_active())
clear_bit(reg->hr_region_num, o2hb_region_bitmap);
spin_unlock(&o2hb_live_lock);
+put_item:
+ config_item_put(®->hr_item);
+ return ERR_PTR(ret);
free:
kfree(reg);
return ERR_PTR(ret);