dm: fix diagnostics about misplaced parts of struct dm_ioctl

* dm.c (dm_decode_dm_target_spec, dm_decode_dm_target_deps,
dm_decode_dm_name_list, dm_decode_dm_target_versions,
dm_decode_dm_target_msg, dm_decode_string, dm_known_ioctl): Fix
diagnostics about various misplaced parts of struct dm_ioctl.
* tests/ioctl_dm.c: Update expected output.

diff --git a/dm.c b/dm.c
index a69077b99b5f4b3c73503d9fc72f45b711e904f5..c6e8072f312ec4485fc59feb953c871938e663d9 100644 (file)
--- a/dm.c
+++ b/dm.c
@@ -140,7 +140,7 @@ dm_decode_dm_target_spec(struct tcb *const tcp, const kernel_ulong_t addr,
                sizeof(struct dm_target_spec);
        uint32_t i;
        uint32_t offset = ioc->data_start;
-       uint32_t offset_end;
+       uint32_t offset_end = 0;
 
        if (abbrev(tcp)) {
                if (ioc->target_count)
@@ -150,20 +150,23 @@ dm_decode_dm_target_spec(struct tcb *const tcp, const kernel_ulong_t addr,
        }
 
        for (i = 0; i < ioc->target_count; i++) {
-               struct dm_target_spec s;
+               tprints(", ");
+
+               if (i && offset <= offset_end)
+                       goto misplaced;
 
                offset_end = offset + target_spec_size;
 
                if (offset_end <= offset || offset_end > ioc->data_size)
                        goto misplaced;
 
-               tprints(", ");
-
                if (i >= max_strlen) {
                        tprints("...");
                        break;
                }
 
+               struct dm_target_spec s;
+
                if (umove_or_printaddr(tcp, addr + offset, &s))
                        break;
 
@@ -186,15 +189,13 @@ dm_decode_dm_target_spec(struct tcb *const tcp, const kernel_ulong_t addr,
                        offset += s.next;
                else
                        offset = ioc->data_start + s.next;
-
-               if (offset <= offset_end)
-                       goto misplaced;
        }
 
        return;
 
 misplaced:
-       tprints(", /* misplaced struct dm_target_spec */ ...");
+       tprints("???");
+       tprints(" /* misplaced struct dm_target_spec */");
 }
 
 bool
@@ -211,6 +212,16 @@ static void
 dm_decode_dm_target_deps(struct tcb *const tcp, const kernel_ulong_t addr,
                         const struct dm_ioctl *const ioc)
 {
+       if (ioc->data_start == ioc->data_size)
+               return;
+
+       tprints(", ");
+
+       if (abbrev(tcp)) {
+               tprints("...");
+               return;
+       }
+
        static const uint32_t target_deps_dev_offs =
                offsetof(struct dm_target_deps, dev);
        uint64_t dev_buf;
@@ -219,13 +230,6 @@ dm_decode_dm_target_deps(struct tcb *const tcp, const kernel_ulong_t addr,
        uint32_t offset_end = offset + target_deps_dev_offs;
        uint32_t space;
 
-       if (abbrev(tcp)) {
-               tprints(", ...");
-               return;
-       }
-
-       tprints(", ");
-
        if (offset_end <= offset || offset_end > ioc->data_size)
                goto misplaced;
 
@@ -247,7 +251,8 @@ dm_decode_dm_target_deps(struct tcb *const tcp, const kernel_ulong_t addr,
        return;
 
 misplaced:
-       tprints("/* misplaced struct dm_target_deps */ ...");
+       tprints("???");
+       tprints(" /* misplaced struct dm_target_deps */");
 }
 
 static void
@@ -258,22 +263,28 @@ dm_decode_dm_name_list(struct tcb *const tcp, const kernel_ulong_t addr,
                offsetof(struct dm_name_list, name);
        struct dm_name_list s;
        uint32_t offset = ioc->data_start;
-       uint32_t offset_end;
+       uint32_t offset_end = 0;
        uint32_t count;
 
+       if (ioc->data_start == ioc->data_size)
+               return;
+
        if (abbrev(tcp)) {
                tprints(", ...");
                return;
        }
 
        for (count = 0;; count++) {
+               tprints(", ");
+
+               if (count && offset <= offset_end)
+                       goto misplaced;
+
                offset_end = offset + name_list_name_offs;
 
                if (offset_end <= offset || offset_end > ioc->data_size)
                        goto misplaced;
 
-               tprints(", ");
-
                if (count >= max_strlen) {
                        tprints("...");
                        break;
@@ -281,10 +292,6 @@ dm_decode_dm_name_list(struct tcb *const tcp, const kernel_ulong_t addr,
 
                if (umove_or_printaddr(tcp, addr + offset, &s))
                        break;
-               if (!count && !s.dev) {
-                       tprints("/* no devices present */");
-                       break;
-               }
 
                tprints("{dev=");
                print_dev_t(s.dev);
@@ -298,14 +305,13 @@ dm_decode_dm_name_list(struct tcb *const tcp, const kernel_ulong_t addr,
                        break;
 
                offset += s.next;
-               if (offset <= offset_end)
-                       goto misplaced;
        }
 
        return;
 
 misplaced:
-       tprints(", /* misplaced struct dm_name_list */ ...");
+       tprints("???");
+       tprints(" /* misplaced struct dm_name_list */");
 }
 
 static void
@@ -316,22 +322,28 @@ dm_decode_dm_target_versions(struct tcb *const tcp, const kernel_ulong_t addr,
                offsetof(struct dm_target_versions, name);
        struct dm_target_versions s;
        uint32_t offset = ioc->data_start;
-       uint32_t offset_end;
+       uint32_t offset_end = 0;
        uint32_t count;
 
+       if (ioc->data_start == ioc->data_size)
+               return;
+
        if (abbrev(tcp)) {
                tprints(", ...");
                return;
        }
 
        for (count = 0;; count++) {
+               tprints(", ");
+
+               if (count && offset <= offset_end)
+                       goto misplaced;
+
                offset_end = offset + target_vers_name_offs;
 
                if (offset_end <= offset || offset_end > ioc->data_size)
                        goto misplaced;
 
-               tprints(", ");
-
                if (count >= max_strlen) {
                        tprints("...");
                        break;
@@ -350,35 +362,37 @@ dm_decode_dm_target_versions(struct tcb *const tcp, const kernel_ulong_t addr,
                        break;
 
                offset += s.next;
-               if (offset <= offset_end)
-                       goto misplaced;
        }
 
        return;
 
 misplaced:
-       tprints(", /* misplaced struct dm_target_versions */ ...");
+       tprints("???");
+       tprints(" /* misplaced struct dm_target_versions */");
 }
 
 static void
 dm_decode_dm_target_msg(struct tcb *const tcp, const kernel_ulong_t addr,
                        const struct dm_ioctl *const ioc)
 {
-       static const uint32_t target_msg_message_offs =
-               offsetof(struct dm_target_msg, message);
-       uint32_t offset = ioc->data_start;
-       uint32_t offset_end = offset + target_msg_message_offs;
+       if (ioc->data_start == ioc->data_size)
+               return;
+
+       tprints(", ");
 
        if (abbrev(tcp)) {
-               tprints(", ...");
+               tprints("...");
                return;
        }
 
+       static const uint32_t target_msg_message_offs =
+               offsetof(struct dm_target_msg, message);
+       uint32_t offset = ioc->data_start;
+       uint32_t offset_end = offset + target_msg_message_offs;
+
        if (offset_end > offset && offset_end <= ioc->data_size) {
                struct dm_target_msg s;
 
-               tprints(", ");
-
                if (umove_or_printaddr(tcp, addr + offset, &s))
                        return;
 
@@ -387,7 +401,8 @@ dm_decode_dm_target_msg(struct tcb *const tcp, const kernel_ulong_t addr,
                            QUOTE_0_TERMINATED);
                tprints("}");
        } else {
-               tprints(", /* misplaced struct dm_target_msg */");
+               tprints("???");
+               tprints(" /* misplaced struct dm_target_msg */");
        }
 }
 
@@ -395,19 +410,22 @@ static void
 dm_decode_string(struct tcb *const tcp, const kernel_ulong_t addr,
                 const struct dm_ioctl *const ioc)
 {
-       uint32_t offset = ioc->data_start;
+       tprints(", ");
 
        if (abbrev(tcp)) {
-               tprints(", ...");
+               tprints("...");
                return;
        }
 
-       if (offset < ioc->data_size) {
-               tprints(", string=");
+       uint32_t offset = ioc->data_start;
+
+       if (offset <= ioc->data_size) {
+               tprints("string=");
                printstr_ex(tcp, addr + offset, ioc->data_size - offset,
                            QUOTE_0_TERMINATED);
        } else {
-               tprints(", /* misplaced string */");
+               tprints("???");
+               tprints(" /* misplaced string */");
        }
 }
 
@@ -485,20 +503,20 @@ dm_known_ioctl(struct tcb *const tcp, const unsigned int code,
         * ioctl fields
         */
        if (ioc->version[0] != DM_VERSION_MAJOR) {
-               tprints(", /* Unsupported device mapper ABI version */ ...");
+               tprints(" /* unsupported device mapper ABI version */");
                goto skip;
        }
 
        tprintf(", data_size=%u", ioc->data_size);
 
-       if (dm_ioctl_has_params(code))
-               tprintf(", data_start=%u", ioc->data_start);
-
        if (ioc->data_size < offsetof(struct dm_ioctl, data)) {
-               tprints(", /* Incorrect data_size */ ...");
+               tprints(" /* data_size too small */");
                goto skip;
        }
 
+       if (dm_ioctl_has_params(code))
+               tprintf(", data_start=%u", ioc->data_start);
+
        dm_decode_device(code, ioc);
        dm_decode_values(tcp, code, ioc);
        dm_decode_flags(ioc);

diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c
index 08f036c6a2237e23d1ccb2d7cf34b59861efb71e..403d795cec06c40cc85a5aef290f272ca552182f 100644 (file)
--- a/tests/ioctl_dm.c
+++ b/tests/ioctl_dm.c
@@ -234,15 +234,15 @@ main(void)
        dm_arg->version[1] = 0xbadc0dee;
        dm_arg->version[2] = 0xbadc0def;
        ioctl(-1, DM_VERSION, dm_arg);
-       printf("ioctl(-1, DM_VERSION, {version=%u.%u.%u, "
-              "/* Unsupported device mapper ABI version */ ...}) = "
+       printf("ioctl(-1, DM_VERSION, {version=%u.%u.%u"
+              " /* unsupported device mapper ABI version */}) = "
               "-1 EBADF (%m)\n", 0xbadc0ded, 0xbadc0dee, 0xbadc0def);
 
        /* Incorrect data_size */
        init_s(dm_arg, 14, 64);
        ioctl(-1, DM_VERSION, dm_arg);
-       printf("ioctl(-1, DM_VERSION, {version=4.1.2, data_size=14, "
-              "/* Incorrect data_size */ ...}) = -1 EBADF (%m)\n");
+       printf("ioctl(-1, DM_VERSION, {version=4.1.2, data_size=14"
+              " /* data_size too small */}) = -1 EBADF (%m)\n");
 
        /* Unterminated name/uuid */
        init_s(dm_arg, min_sizeof_dm_ioctl, 0);
@@ -395,8 +395,8 @@ main(void)
               "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", "
               "target_count=1234, flags=0, "
 # if VERBOSE
-              "/* misplaced struct dm_target_spec */ ..."
-# else /* !VERBOSE */
+              "??? /* misplaced struct dm_target_spec */"
+# else
               "..."
 # endif /* VERBOSE */
               "}) = -1 EBADF (%m)\n", sizeof(*dm_arg), 0xfffffff8);
@@ -491,7 +491,7 @@ main(void)
        print_dm_target_spec(&dm_arg_open3->target1, 15);
        printf("\"\\377\"}, ");
        print_dm_target_spec(&dm_arg_open3->target1, 42);
-       printf("\"\\1\\2\"}, /* misplaced struct dm_target_spec */ ...");
+       printf("\"\\1\\2\"}, ??? /* misplaced struct dm_target_spec */");
 # else /* !VERBOSE */
        printf("...");
 # endif /* VERBOSE */
@@ -575,7 +575,7 @@ main(void)
               "{version=4.1.2, data_size=%zu, data_start=%zu, "
               "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, "
 # if VERBOSE
-              "/* misplaced struct dm_target_msg */"
+              "??? /* misplaced struct dm_target_msg */"
 # else /* !VERBOSE */
               "..."
 # endif /* VERBOSE */
@@ -590,7 +590,7 @@ main(void)
               "{version=4.1.2, data_size=%zu, data_start=%u, "
               "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, "
 # if VERBOSE
-              "/* misplaced struct dm_target_msg */"
+              "??? /* misplaced struct dm_target_msg */"
 # else /* !VERBOSE */
               "..."
 # endif /* VERBOSE */
@@ -706,7 +706,7 @@ main(void)
               "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", event_nr=0, "
               "flags=0, "
 # if VERBOSE
-              "/* misplaced string */"
+              "??? /* misplaced string */"
 # else /* !VERBOSE */
               "..."
 # endif /* VERBOSE */
@@ -757,10 +757,12 @@ main(void)
               "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", "
               "target_count=4294967295, flags=0, "
 # if VERBOSE
-              "{sector_start=0, length=0, target_type=\"\", string=\"\"}, "
-              "/* misplaced struct dm_target_spec */ "
+              "{sector_start=0, length=0, target_type=\"\", string=\"\"}"
+              ", ??? /* misplaced struct dm_target_spec */"
+# else
+              "..."
 # endif /* VERBOSE */
-              "...}) = -1 EBADF (%m)\n",
+              "}) = -1 EBADF (%m)\n",
               s.ioc.data_size, s.ioc.data_start);
 
        puts("+++ exited with 0 +++");