In set_ble_device func, if blist is NULL or ble is NULL,
the vendor and product isn't freed. We think it is not
reasonable that strdup(XXX) is used as set_ble_device
and store_ble functions' parameter.

Here we call strdup() in store_ble and set_ble_device
functions and the string will be free if functions fail.
Because constant string like "sdb" will be their parameter,
char * is changed to const char *. This is base on
upstream-queue branch in openSUSE/multipath-tools.

Signed-off-by: Lixiaokeng <lixiaok...@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqian...@huawei.com>

---
 libmultipath/blacklist.c | 81 ++++++++++++++++++++++------------------
 libmultipath/blacklist.h |  4 +-
 tests/blacklist.c        | 31 +++++++--------
 3 files changed, 59 insertions(+), 57 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index db58ccca..2ff7cd84 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -29,14 +29,19 @@ char *check_invert(char *str, bool *invert)
        return str;
 }

-int store_ble(vector blist, char * str, int origin)
+int store_ble(vector blist, const char * str, int origin)
 {
        struct blentry * ble;
        char *regex_str;
+       char *strdup_str = NULL;

        if (!str)
                return 0;

+       strdup_str = strdup(str);
+       if (!strdup_str)
+               return 1;
+
        if (!blist)
                goto out;

@@ -45,21 +50,21 @@ int store_ble(vector blist, char * str, int origin)
        if (!ble)
                goto out;

-       regex_str = check_invert(str, &ble->invert);
+       regex_str = check_invert(strdup_str, &ble->invert);
        if (regcomp(&ble->regex, regex_str, REG_EXTENDED|REG_NOSUB))
                goto out1;

        if (!vector_alloc_slot(blist))
                goto out1;

-       ble->str = str;
+       ble->str = strdup_str;
        ble->origin = origin;
        vector_set_slot(blist, ble);
        return 0;
 out1:
        FREE(ble);
 out:
-       FREE(str);
+       FREE(strdup_str);
        return 1;
 }

@@ -79,10 +84,12 @@ int alloc_ble_device(vector blist)
        return 0;
 }

-int set_ble_device(vector blist, char * vendor, char * product, int origin)
+int set_ble_device(vector blist, const char * vendor, const char * product, 
int origin)
 {
        struct blentry_device * ble;
        char *regex_str;
+       char *vendor_str = NULL;
+       char *product_str = NULL;

        if (!blist)
                return 1;
@@ -93,31 +100,40 @@ int set_ble_device(vector blist, char * vendor, char * 
product, int origin)
                return 1;

        if (vendor) {
-               regex_str = check_invert(vendor, &ble->vendor_invert);
-               if (regcomp(&ble->vendor_reg, regex_str,
-                           REG_EXTENDED|REG_NOSUB)) {
-                       FREE(vendor);
-                       if (product)
-                               FREE(product);
-                       return 1;
-               }
-               ble->vendor = vendor;
+               vendor_str = STRDUP(vendor);
+               if (!vendor_str)
+                       goto out;
+
+               regex_str = check_invert(vendor_str, &ble->vendor_invert);
+               if (regcomp(&ble->vendor_reg, regex_str, 
REG_EXTENDED|REG_NOSUB))
+                       goto out;
+
+               ble->vendor = vendor_str;
        }
        if (product) {
-               regex_str = check_invert(product, &ble->product_invert);
-               if (regcomp(&ble->product_reg, regex_str,
-                           REG_EXTENDED|REG_NOSUB)) {
-                       FREE(product);
-                       if (vendor) {
-                               ble->vendor = NULL;
-                               FREE(vendor);
-                       }
-                       return 1;
-               }
-               ble->product = product;
+               product_str = STRDUP(product);
+               if (!product_str)
+                       goto out1;
+
+               regex_str = check_invert(product_str, &ble->product_invert);
+               if (regcomp(&ble->product_reg, regex_str, 
REG_EXTENDED|REG_NOSUB))
+                       goto out1;
+
+               ble->product = product_str;
        }
        ble->origin = origin;
        return 0;
+out1:
+       if (vendor_str)
+               ble->vendor = NULL;
+out:
+       if (vendor_str)
+               FREE(vendor_str);
+
+       if (product_str)
+               FREE(product_str);
+
+       return 1;
 }

 static int
@@ -178,19 +194,12 @@ setup_default_blist (struct config * conf)
 {
        struct blentry * ble;
        struct hwentry *hwe;
-       char * str;
        int i;

-       str = STRDUP("!^(sd[a-z]|dasd[a-z]|nvme[0-9])");
-       if (!str)
-               return 1;
-       if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
+       if (store_ble(conf->blist_devnode, "!^(sd[a-z]|dasd[a-z]|nvme[0-9])", 
ORIGIN_DEFAULT))
                return 1;

-       str = STRDUP("(SCSI_IDENT_|ID_WWN)");
-       if (!str)
-               return 1;
-       if (store_ble(conf->elist_property, str, ORIGIN_DEFAULT))
+       if (store_ble(conf->elist_property, "(SCSI_IDENT_|ID_WWN)", 
ORIGIN_DEFAULT))
                return 1;

        vector_foreach_slot (conf->hwtable, hwe, i) {
@@ -202,9 +211,7 @@ setup_default_blist (struct config * conf)
                                return 1;
                        ble = VECTOR_SLOT(conf->blist_device,
                                          VECTOR_SIZE(conf->blist_device) - 1);
-                       if (set_ble_device(conf->blist_device,
-                                          STRDUP(hwe->vendor),
-                                          STRDUP(hwe->bl_product),
+                       if (set_ble_device(conf->blist_device, hwe->vendor, 
hwe->bl_product,
                                           ORIGIN_DEFAULT)) {
                                FREE(ble);
                                vector_del_slot(conf->blist_device, 
VECTOR_SIZE(conf->blist_device) - 1);
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index 4305857d..b08e0978 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -42,8 +42,8 @@ int filter_device (vector, vector, char *, char *, char *);
 int filter_path (struct config *, struct path *);
 int filter_property(struct config *, struct udev_device *, int, const char*);
 int filter_protocol(vector, vector, struct path *);
-int store_ble (vector, char *, int);
-int set_ble_device (vector, char *, char *, int);
+int store_ble (vector, const char *, int);
+int set_ble_device (vector, const char *, const char *, int);
 void free_blacklist (vector);
 void free_blacklist_device (vector);
 void merge_blacklist(vector);
diff --git a/tests/blacklist.c b/tests/blacklist.c
index d5c40898..84a3ba2f 100644
--- a/tests/blacklist.c
+++ b/tests/blacklist.c
@@ -94,67 +94,62 @@ static int setup(void **state)

        blist_devnode_sdb = vector_alloc();
        if (!blist_devnode_sdb ||
-           store_ble(blist_devnode_sdb, strdup("sdb"), ORIGIN_CONFIG))
+           store_ble(blist_devnode_sdb, "sdb", ORIGIN_CONFIG))
                return -1;
        blist_devnode_sdb_inv = vector_alloc();
        if (!blist_devnode_sdb_inv ||
-           store_ble(blist_devnode_sdb_inv, strdup("!sdb"), ORIGIN_CONFIG))
+           store_ble(blist_devnode_sdb_inv, "!sdb", ORIGIN_CONFIG))
                return -1;

        blist_all = vector_alloc();
-       if (!blist_all || store_ble(blist_all, strdup(".*"), ORIGIN_CONFIG))
+       if (!blist_all || store_ble(blist_all, ".*", ORIGIN_CONFIG))
                return -1;

        blist_device_foo_bar = vector_alloc();
        if (!blist_device_foo_bar || alloc_ble_device(blist_device_foo_bar) ||
-           set_ble_device(blist_device_foo_bar, strdup("foo"), strdup("bar"),
-                          ORIGIN_CONFIG))
+           set_ble_device(blist_device_foo_bar, "foo", "bar", ORIGIN_CONFIG))
                return -1;
        blist_device_foo_inv_bar = vector_alloc();
        if (!blist_device_foo_inv_bar ||
            alloc_ble_device(blist_device_foo_inv_bar) ||
-           set_ble_device(blist_device_foo_inv_bar, strdup("!foo"),
-                          strdup("bar"), ORIGIN_CONFIG))
+           set_ble_device(blist_device_foo_inv_bar, "!foo", "bar", 
ORIGIN_CONFIG))
                return -1;
        blist_device_foo_bar_inv = vector_alloc();
        if (!blist_device_foo_bar_inv ||
            alloc_ble_device(blist_device_foo_bar_inv) ||
-           set_ble_device(blist_device_foo_bar_inv, strdup("foo"),
-                          strdup("!bar"), ORIGIN_CONFIG))
+           set_ble_device(blist_device_foo_bar_inv, "foo", "!bar", 
ORIGIN_CONFIG))
                return -1;

        blist_device_all = vector_alloc();
        if (!blist_device_all || alloc_ble_device(blist_device_all) ||
-           set_ble_device(blist_device_all, strdup(".*"), strdup(".*"),
-                          ORIGIN_CONFIG))
+           set_ble_device(blist_device_all, ".*", ".*", ORIGIN_CONFIG))
                return -1;

        blist_wwid_xyzzy = vector_alloc();
        if (!blist_wwid_xyzzy ||
-           store_ble(blist_wwid_xyzzy, strdup("xyzzy"), ORIGIN_CONFIG))
+           store_ble(blist_wwid_xyzzy, "xyzzy", ORIGIN_CONFIG))
                return -1;
        blist_wwid_xyzzy_inv = vector_alloc();
        if (!blist_wwid_xyzzy_inv ||
-           store_ble(blist_wwid_xyzzy_inv, strdup("!xyzzy"), ORIGIN_CONFIG))
+           store_ble(blist_wwid_xyzzy_inv, "!xyzzy", ORIGIN_CONFIG))
                return -1;

        blist_protocol_fcp = vector_alloc();
        if (!blist_protocol_fcp ||
-           store_ble(blist_protocol_fcp, strdup("scsi:fcp"), ORIGIN_CONFIG))
+           store_ble(blist_protocol_fcp, "scsi:fcp", ORIGIN_CONFIG))
                return -1;
        blist_protocol_fcp_inv = vector_alloc();
        if (!blist_protocol_fcp_inv ||
-           store_ble(blist_protocol_fcp_inv, strdup("!scsi:fcp"),
-                     ORIGIN_CONFIG))
+           store_ble(blist_protocol_fcp_inv, "!scsi:fcp", ORIGIN_CONFIG))
                return -1;

        blist_property_wwn = vector_alloc();
        if (!blist_property_wwn ||
-           store_ble(blist_property_wwn, strdup("ID_WWN"), ORIGIN_CONFIG))
+           store_ble(blist_property_wwn, "ID_WWN", ORIGIN_CONFIG))
                return -1;
        blist_property_wwn_inv = vector_alloc();
        if (!blist_property_wwn_inv ||
-           store_ble(blist_property_wwn_inv, strdup("!ID_WWN"), ORIGIN_CONFIG))
+           store_ble(blist_property_wwn_inv, "!ID_WWN", ORIGIN_CONFIG))
                return -1;

        return 0;
-- 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to