disk/part.c: ensure strings in struct disk_partition are valid after successful get_info

Not all ->get_info implementations necessarily populate all the string
members of struct disk_partition.

Currently, only part_get_info_by_type() (and thereby part_get_info)
ensure that the uuid strings are initialized; part_get_info_by_type()
and part_get_info_by_uuid() do not. In fact, the latter could lead to
a false positive match - if the ->get_info backend does not populate
info->uuid, stale contents in info could cause the strncasecmp() to
succeed.

None of the functions currently ensure that the ->name and ->type
strings are initialized.

Instead of forcing all callers of any of these functions to
pre-initialize info, or all implementations of the ->get_info method
to ensure there are valid C strings in all four fields, create a small
helper function and factor all invocations of ->get_info through that.

This also consolidates the -ENOSYS check and standardizes on using
log_debug() for reporting absence, instead of the current mix of
PRINTF and log_debug(). It does mean we have to special-case -ENOSYS
in the error cases inside the loops in the _by_uuid() and _by_name()
functions, but it's still a net win in #LOC.

Acked-by: Quentin Schulz <quentin.schulz@cherry.de>
Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
Tested-by: Anshul Dalal <anshuld@ti.com>

diff --git a/cmd/gpt.c b/cmd/gpt.c
index e18e5036a06a005fa8a3913dcae9d5003f6be6ee..84221881c39cf297498722ae2752a9171faa44d2 100644 (file)
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -724,7 +724,7 @@ static int gpt_enumerate(struct blk_desc *desc)
                        continue;
 
                for (i = 1; i < part_drv->max_entries; i++) {
-                       ret = part_drv->get_info(desc, i, &pinfo);
+                       ret = part_driver_get_info(part_drv, desc, i, &pinfo);
                        if (ret)
                                continue;
 
@@ -820,7 +820,7 @@ static int gpt_setenv(struct blk_desc *desc, const char *name)
                int i;
 
                for (i = 1; i < part_drv->max_entries; i++) {
-                       ret = part_drv->get_info(desc, i, &pinfo);
+                       ret = part_driver_get_info(part_drv, desc, i, &pinfo);
                        if (ret)
                                continue;
 

diff --git a/disk/part.c b/disk/part.c
index be2b45d5a2923fa8d29fd2891fa4e462661e06db..49a0fca6b8972f58110b35a1429b110269101c6a 100644 (file)
--- a/disk/part.c
+++ b/disk/part.c
@@ -72,6 +72,28 @@ struct part_driver *part_driver_lookup_type(struct blk_desc *desc)
        return NULL;
 }
 
+static void disk_partition_clr(struct disk_partition *info)
+{
+       /* The common case is no UUID support */
+       disk_partition_clr_uuid(info);
+       disk_partition_clr_type_guid(info);
+       info->name[0] = '\0';
+       info->type[0] = '\0';
+}
+
+int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
+                        struct disk_partition *info)
+{
+       if (!drv->get_info) {
+               log_debug("## Driver %s does not have the get_info() method\n",
+                         drv->name);
+               return -ENOSYS;
+       }
+
+       disk_partition_clr(info);
+       return drv->get_info(desc, part, info);
+}
+
 int part_get_type_by_name(const char *name)
 {
        struct part_driver *drv =
@@ -322,12 +344,9 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
                          struct disk_partition *info)
 {
        struct part_driver *drv;
+       int ret = -ENOENT;
 
        if (blk_enabled()) {
-               /* The common case is no UUID support */
-               disk_partition_clr_uuid(info);
-               disk_partition_clr_type_guid(info);
-
                if (part_type == PART_TYPE_UNKNOWN) {
                        drv = part_driver_lookup_type(desc);
                } else {
@@ -339,18 +358,16 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
                              desc->part_type);
                        return -EPROTONOSUPPORT;
                }
-               if (!drv->get_info) {
-                       PRINTF("## Driver %s does not have the get_info() method\n",
-                              drv->name);
-                       return -ENOSYS;
-               }
-               if (drv->get_info(desc, part, info) == 0) {
+
+               ret = part_driver_get_info(drv, desc, part, info);
+               if (ret && ret != -ENOSYS) {
+                       ret = -ENOENT;
+               } else {
                        PRINTF("## Valid %s partition found ##\n", drv->name);
-                       return 0;
                }
        }
 
-       return -ENOENT;
+       return ret;
 }
 
 int part_get_info(struct blk_desc *desc, int part,
@@ -657,15 +674,12 @@ int part_get_info_by_name(struct blk_desc *desc, const char *name,
        if (!part_drv)
                return -1;
 
-       if (!part_drv->get_info) {
-               log_debug("## Driver %s does not have the get_info() method\n",
-                         part_drv->name);
-               return -ENOSYS;
-       }
-
        for (i = 1; i < part_drv->max_entries; i++) {
-               ret = part_drv->get_info(desc, i, info);
+               ret = part_driver_get_info(part_drv, desc, i, info);
                if (ret != 0) {
+                       /* -ENOSYS means no ->get_info method. */
+                       if (ret == -ENOSYS)
+                               return ret;
                        /*
                         * Partition with this index can't be obtained, but
                         * further partitions might be, so keep checking.
@@ -695,15 +709,12 @@ int part_get_info_by_uuid(struct blk_desc *desc, const char *uuid,
        if (!part_drv)
                return -1;
 
-       if (!part_drv->get_info) {
-               log_debug("## Driver %s does not have the get_info() method\n",
-                         part_drv->name);
-               return -ENOSYS;
-       }
-
        for (i = 1; i < part_drv->max_entries; i++) {
-               ret = part_drv->get_info(desc, i, info);
+               ret = part_driver_get_info(part_drv, desc, i, info);
                if (ret != 0) {
+                       /* -ENOSYS means no ->get_info method. */
+                       if (ret == -ENOSYS)
+                               return ret;
                        /*
                         * Partition with this index can't be obtained, but
                         * further partitions might be, so keep checking.

diff --git a/include/part.h b/include/part.h
index 6caaa6526aa462214c3d0cbff58023394d0cc4a0..d940f8b5d0eedca91682ae6edb918c4f6d0003e1 100644 (file)
--- a/include/part.h
+++ b/include/part.h
@@ -509,6 +509,22 @@ struct part_driver {
        int (*test)(struct blk_desc *desc);
 };
 
+/**
+ * part_driver_get_info() - Call the part_driver's get_info method
+ *
+ * On success, string members of info are guaranteed to be properly
+ * initialized, though they may be empty.
+ *
+ * @drv:       Partition driver
+ * @desc:      Block device descriptor
+ * @part:      Partition number to read
+ * @info:      Returned partition information
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int part_driver_get_info(struct part_driver *drv, struct blk_desc *desc, int part,
+                        struct disk_partition *info);
+
 /* Declare a new U-Boot partition 'driver' */
 #define U_BOOT_PART_TYPE(__name)                                       \
        ll_entry_declare(struct part_driver, __name, part_driver)