From d0a9fba3e3a20db0d77108f253c566f9e9060ca2 Mon Sep 17 00:00:00 2001
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Date: Thu, 25 Sep 2025 20:01:06 +0530
Subject: [PATCH] librbd: fix segfault when removing non-existent group

Removing a non-existent group triggers a segfault in
librbd::mirror::GroupGetInfoRequest::send(). The issue is caused by a missing
return after finish(), which allows execution to fall through into
GroupGetInfoRequest::get_id() and access invalid memory.

Also, makesure to ignore ENOENT throughout the method Group::remove()
except at cls_client::dir_get_id()

Credits to Ilya Dryomov <idryomov@gmail.com>

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
 qa/workunits/rbd/rbd_mirror_group_simple.sh | 18 ++++++++++++++++++
 src/librbd/api/Group.cc                     |  7 ++++---
 src/librbd/mirror/GroupGetInfoRequest.cc    |  1 +
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/qa/workunits/rbd/rbd_mirror_group_simple.sh b/qa/workunits/rbd/rbd_mirror_group_simple.sh
index fcbb8d52a0a..a2ea0192d16 100755
--- a/qa/workunits/rbd/rbd_mirror_group_simple.sh
+++ b/qa/workunits/rbd/rbd_mirror_group_simple.sh
@@ -1358,6 +1358,23 @@ test_empty_group()
   check_daemon_running "${secondary_cluster}"
 }
 
+# test removal of a non existing group
+declare -a test_remove_non_existing_group_1=("${CLUSTER2}" ${pool0} "group_blabla")
+
+test_remove_non_existing_group_scenarios=1
+
+test_remove_non_existing_group()
+{
+  local primary_cluster=$1
+  local pool=$2
+  local non_existing_group=$3
+
+  # command fails with the following message now
+  # rbd: remove error: (2) No such file or directory
+  # 2025-09-26T14:21:54.194+0530 7f94208f3d00 -1 librbd::api::Group: remove: error getting the group id: (2) No such file or directory
+  expect_failure "error getting the group id" rbd --cluster=${primary_cluster} group remove ${pool}/${non_existing_group}
+}
+
 #check that omap keys have been correctly deleted
 declare -a test_empty_group_omap_keys_1=("${CLUSTER2}" "${CLUSTER1}" "${pool0}")
 
@@ -3732,6 +3749,7 @@ run_all_tests()
 {
   run_test_all_scenarios test_empty_group
   run_test_all_scenarios test_empty_groups
+  run_test_all_scenarios test_remove_non_existing_group
   # This next test requires support for dynamic groups TODO
   # run_test_all_scenarios test_mirrored_group_remove_all_images
   # This next test also requires dynamic groups - TODO enable
diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc
index 2264ad9b6f4..6c46a81d414 100644
--- a/src/librbd/api/Group.cc
+++ b/src/librbd/api/Group.cc
@@ -395,8 +395,9 @@ int Group<I>::remove(librados::IoCtx& io_ctx, const char *group_name)
   std::string group_id;
   int r = cls_client::dir_get_id(&io_ctx, RBD_GROUP_DIRECTORY,
 				 std::string(group_name), &group_id);
-  if (r < 0 && r != -ENOENT) {
-    lderr(cct) << "error getting id of group" << dendl;
+  if (r < 0) {
+    lderr(cct) << "error getting the group id: "
+               << cpp_strerror(r) << dendl;
     return r;
   }
 
@@ -417,7 +418,7 @@ int Group<I>::remove(librados::IoCtx& io_ctx, const char *group_name)
   }
 
   r = Mirror<I>::group_disable(io_ctx, group_name, false);
-  if (r < 0 && r != -EOPNOTSUPP) {
+  if (r < 0 && r != -ENOENT && r != -EOPNOTSUPP) {
     lderr(cct) << "failed to disable mirroring: " << cpp_strerror(r) << dendl;
     return r;
   }
diff --git a/src/librbd/mirror/GroupGetInfoRequest.cc b/src/librbd/mirror/GroupGetInfoRequest.cc
index 25997d2987f..96936bb0e63 100644
--- a/src/librbd/mirror/GroupGetInfoRequest.cc
+++ b/src/librbd/mirror/GroupGetInfoRequest.cc
@@ -29,6 +29,7 @@ void GroupGetInfoRequest<I>::send() {
   if (m_group_name.empty() && m_group_id.empty()) {
     lderr(cct) << "both group name and group id cannot be empty" << dendl;
     finish(-EINVAL);
+    return;
   }
 
   if (m_group_id.empty()) {
-- 
2.39.5