[PATCH v2] ocfs2: fix use-after-free in o2hb_region_dev_store

[Deepanshu Kartikey <kartikey406@gmail.com>]

Two tasks can concurrently write to the same region's "dev" configfs
attribute, each through its own open fd. configfs does not serialize
separate fds against each other here.

The only guard against a second writer is an unlocked check near the
top of o2hb_region_dev_store():

    if (reg->hr_bdev_file)
            return -EINVAL;

This check happens well before hr_bdev_file is actually assigned;
several validation steps and an fd lookup sit in between. If a second
task runs this same check before the first task has assigned
hr_bdev_file, it also reads NULL and also proceeds.

Both tasks then call o2hb_map_slot_data(), which allocates
reg->hr_slot_data, and later, on any failure, o2hb_unmap_slot_data(),
which frees it. Since hr_slot_data is also unprotected, one task can
free it through its failure path while the other task is still
populating the array, producing the use-after-free reported by syzbot.

Race flow:

  T1: dev_store()                      T2: dev_store()
  if (reg->hr_bdev_file)  // NULL      if (reg->hr_bdev_file)  // NULL
  ...                                  ...
  reg->hr_bdev_file = ...
  o2hb_map_slot_data()
    alloc reg->hr_slot_data
    populating slots...
                                       reg->hr_bdev_file = ...
                                       o2hb_map_slot_data()
                                       failure -> out:
                                         o2hb_unmap_slot_data()
                                           frees reg->hr_slot_data
  writes to freed slot data  <-- UAF

Fix this by adding a per-region mutex, taken at the top of
o2hb_region_dev_store() before any region state is modified and held
until the function returns.

Fixes: a7f6a5fb4bde ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Reported-by: syzbot+3025e3e8fc0b928af8f5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3025e3e8fc0b928af8f5
Tested-by: syzbot+3025e3e8fc0b928af8f5@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/6a9b3f7a.2e649fcc.1e1977.0001.GAE@google.com/T/ [v1]
Assisted-by: Claude
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v2:
 - Expanded the description with the race flow
 - Updated Fixes: tag
 - Use tab for indentation
 - Dropped locking in o2hb_region_release(); configfs already
   serializes store and rmdir
 - Take the lock before the hr_node_num assignment, using
   guard(mutex) so every early return releases it
---
 fs/ocfs2/cluster/heartbeat.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 1c3def99bb07..e20ef0459f54 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -273,6 +273,8 @@ struct o2hb_region {
 
 	/* last hb status, 0 for success, other value for error. */
 	int			hr_last_hb_status;
+	/* Serializes concurrent writers of the "dev" attribute */
+	struct mutex		hr_dev_write_mutex;
 };
 
 static inline struct block_device *reg_bdev(struct o2hb_region *reg)
@@ -1879,6 +1881,8 @@ static ssize_t o2hb_region_dev_store(struct config_item *item,
 	ssize_t ret = -EINVAL;
 	int live_threshold;
 
+	guard(mutex)(&reg->hr_dev_write_mutex);
+
 	if (reg->hr_bdev_file)
 		return -EINVAL;
 
@@ -2149,6 +2153,8 @@ static struct config_item *o2hb_heartbeat_group_make_item(struct config_group *g
 
 	config_item_init_type_name(&reg->hr_item, name, &o2hb_region_type);
 
+	mutex_init(&reg->hr_dev_write_mutex);
+
 	/* 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.
-- 
2.43.0