From: Miki Patel <miki.patel132@gmail.com>
Date: Tue, 15 Jul 2025 11:07:16 +0000 (+0530)
Subject: librbd: images aren't closed in group_snap_*_by_record() on error
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=693eabf58cc19eba4c21e2ea7c0011643659b3c3;p=ceph.git

librbd: images aren't closed in group_snap_*_by_record() on error

Fixes memory leak and handles resource leak scenario when at leat one IoCtx is not
created successfully. This is done by returning error before opening any image.
Changes are made in group_snap_remove_by_record and group_snap_rollback_by_record

Fixes: https://tracker.ceph.com/issues/71961
Signed-off-by: Miki Patel <miki.patel132@gmail.com>
---

diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc
index 22e3ba86757da..30b2b979988dc 100644
--- a/src/librbd/api/Group.cc
+++ b/src/librbd/api/Group.cc
@@ -180,11 +180,11 @@ int group_snap_remove_by_record(librados::IoCtx& group_ioctx,
 				const std::string& group_header_oid) {
 
   CephContext *cct = (CephContext *)group_ioctx.cct();
+  std::vector<librados::IoCtx> ioctxs;
+  std::vector<librbd::ImageCtx*> ictxs;
   std::vector<C_SaferCond*> on_finishes;
   int r, ret_code;
 
-  std::vector<librbd::ImageCtx*> ictxs;
-
   cls::rbd::GroupSnapshotNamespace ne{group_ioctx.get_id(), group_id,
 				      group_snap.id};
 
@@ -192,15 +192,18 @@ int group_snap_remove_by_record(librados::IoCtx& group_ioctx,
   int snap_count = group_snap.snaps.size();
 
   for (int i = 0; i < snap_count; ++i) {
-    librbd::IoCtx image_io_ctx;
+    librados::IoCtx image_io_ctx;
     r = util::create_ioctx(group_ioctx, "image", group_snap.snaps[i].pool, {},
                            &image_io_ctx);
     if (r < 0) {
       return r;
     }
+    ioctxs.push_back(std::move(image_io_ctx));
+  }
 
+  for (int i = 0; i < snap_count; ++i) {
     librbd::ImageCtx* image_ctx = new ImageCtx("", group_snap.snaps[i].image_id,
-					       nullptr, image_io_ctx, false);
+					       nullptr, ioctxs[i], false);
 
     C_SaferCond* on_finish = new C_SaferCond;
 
@@ -286,11 +289,11 @@ int group_snap_rollback_by_record(librados::IoCtx& group_ioctx,
                                   const std::string& group_id,
                                   ProgressContext& pctx) {
   CephContext *cct = (CephContext *)group_ioctx.cct();
+  std::vector<librados::IoCtx> ioctxs;
+  std::vector<librbd::ImageCtx*> ictxs;
   std::vector<C_SaferCond*> on_finishes;
   int r, ret_code;
 
-  std::vector<librbd::ImageCtx*> ictxs;
-
   cls::rbd::GroupSnapshotNamespace ne{group_ioctx.get_id(), group_id,
                                       group_snap.id};
 
@@ -304,9 +307,12 @@ int group_snap_rollback_by_record(librados::IoCtx& group_ioctx,
     if (r < 0) {
       return r;
     }
+    ioctxs.push_back(std::move(image_io_ctx));
+  }
 
+  for (int i = 0; i < snap_count; ++i) {
     librbd::ImageCtx* image_ctx = new ImageCtx("", group_snap.snaps[i].image_id,
-                                               nullptr, image_io_ctx, false);
+                                               nullptr, ioctxs[i], false);
 
     C_SaferCond* on_finish = new C_SaferCond;