From: Ilya Dryomov <idryomov@gmail.com>
Date: Fri, 3 Jun 2016 15:24:48 +0000 (+0200)
Subject: krbd: don't segfault if images are unmapped concurrently
X-Git-Tag: v11.0.0~295^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F9517%2Fhead;p=ceph.git

krbd: don't segfault if images are unmapped concurrently

"rbd map c" can die from a NULL dereference on any of this_pool,
this_image or this_snap in wait_for_udev_add():

    <image a is mapped>
    rbd map c
                                    rbd map b
                                    rbd unmap a
                                    rbd unmap b

However unlikely, this segfault is triggered by the rbd/concurrent.sh
workunit on a regular basis.

Similarly, "rbd showmapped" can die if an image to be listed is
unmapped.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/src/krbd.cc b/src/krbd.cc
index d480a5ac6605..99fbf208dc69 100644
--- a/src/krbd.cc
+++ b/src/krbd.cc
@@ -204,9 +204,9 @@ static int wait_for_udev_add(struct udev_monitor *mon, const char *pool,
         const char *this_snap = udev_device_get_sysattr_value(dev,
                                                               "current_snap");
 
-        if (strcmp(this_pool, pool) == 0 &&
-            strcmp(this_image, image) == 0 &&
-            strcmp(this_snap, snap) == 0) {
+        if (this_pool && strcmp(this_pool, pool) == 0 &&
+            this_image && strcmp(this_image, image) == 0 &&
+            this_snap && strcmp(this_snap, snap) == 0) {
           bus_dev = dev;
           continue;
         }
@@ -595,14 +595,18 @@ static int unmap_image(struct krbd_ctx *ctx, const char *pool,
   return do_unmap(ctx->udev, devno, id);
 }
 
-static void dump_one_image(Formatter *f, TextTable *tbl,
-                           const char *id, const char *pool,
-                           const char *image, const char *snap)
+static bool dump_one_image(Formatter *f, TextTable *tbl,
+                           struct udev_device *dev)
 {
-  assert(id && pool && image && snap);
-
+  const char *id = udev_device_get_sysname(dev);
+  const char *pool = udev_device_get_sysattr_value(dev, "pool");
+  const char *image = udev_device_get_sysattr_value(dev, "name");
+  const char *snap = udev_device_get_sysattr_value(dev, "current_snap");
   string kname = get_kernel_rbd_name(id);
 
+  if (!pool || !image || !snap)
+    return false;
+
   if (f) {
     f->open_object_section(id);
     f->dump_string("pool", pool);
@@ -613,6 +617,8 @@ static void dump_one_image(Formatter *f, TextTable *tbl,
   } else {
     *tbl << id << pool << image << snap << kname << TextTable::endrow;
   }
+
+  return true;
 }
 
 static int do_dump(struct udev *udev, Formatter *f, TextTable *tbl)
@@ -638,18 +644,10 @@ static int do_dump(struct udev *udev, Formatter *f, TextTable *tbl)
     struct udev_device *dev;
 
     dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(l));
-    if (!dev) {
-      r = -ENOMEM;
-      goto out_enm;
+    if (dev) {
+      have_output |= dump_one_image(f, tbl, dev);
+      udev_device_unref(dev);
     }
-
-    dump_one_image(f, tbl, udev_device_get_sysname(dev),
-                   udev_device_get_sysattr_value(dev, "pool"),
-                   udev_device_get_sysattr_value(dev, "name"),
-                   udev_device_get_sysattr_value(dev, "current_snap"));
-
-    have_output = true;
-    udev_device_unref(dev);
   }
 
   r = have_output;