commit ec49f07b0dc89720f4a74a1212e106990099d2d6 Author: Zdenek Kabelac Date: Thu Dec 6 23:37:21 2012 +0100 mirrors: fix leak in device_is_usable mirror check Function _ignore_blocked_mirror_devices was not release allocated strings images_health and log_health. In error paths it was also not releasing dm_task structure. Swaped return code of _ignore_blocked_mirror_devices and use 1 as success. In _parse_mirror_status use log_error if memory allocation fails and few more errors so they are no going unnoticed as debug messages. On error path always clear return values and free strings. For dev_create_file use cache mem pool to avoid memleak. diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index 40f719e..3f74c2d 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -158,6 +158,10 @@ static int _parse_mirror_status(char *mirror_status_str, char **args, **log_args; unsigned num_devs, log_argc; + *images_health = NULL; + *log_health = NULL; + *log_dev = 0; + if (!dm_split_words(mirror_status_str, 1, 0, &p) || !(num_devs = (unsigned) atoi(p))) /* On errors, we must assume the mirror is to be avoided */ @@ -176,19 +180,31 @@ static int _parse_mirror_status(char *mirror_status_str, log_argc, 0, log_args) < log_argc) return_0; - *log_health = NULL; - *log_dev = 0; if (!strcmp(log_args[0], "disk")) { - if (!(*log_health = dm_strdup(log_args[2]))) - return_0; - if (sscanf(log_args[1], "%d:%d", &major, &minor) != 2) - return_0; + if (!(*log_health = dm_strdup(log_args[2]))) { + log_error("Allocation of log string failed."); + return 0; + } + if (sscanf(log_args[1], "%d:%d", &major, &minor) != 2) { + log_error("Parsing of log's major minor failed."); + goto out; + } *log_dev = MKDEV((dev_t)major, minor); } - if (!(*images_health = dm_strdup(args[2 + num_devs]))) - return_0; + + if (!(*images_health = dm_strdup(args[2 + num_devs]))) { + log_error("Allocation of images string failed."); + goto out; + } return 1; + +out: + dm_free(*log_health); + *log_health = NULL; + *log_dev = 0; + + return 0; } /* @@ -227,11 +243,12 @@ static int _ignore_blocked_mirror_devices(struct device *dev, uint64_t s,l; char *params, *target_type = NULL; void *next = NULL; - struct dm_task *dmt; + struct dm_task *dmt = NULL; + int r = 0; if (!_parse_mirror_status(mirror_status_str, &images_health, &log_dev, &log_health)) - goto_out; + return_0; for (i = 0; images_health[i]; i++) if (images_health[i] != 'A') { @@ -254,7 +271,7 @@ static int _ignore_blocked_mirror_devices(struct device *dev, (int)MINOR(log_dev)) < 0) goto_out; - if (!(tmp_dev = dev_create_file(buf, NULL, NULL, 1))) + if (!(tmp_dev = dev_create_file(buf, NULL, NULL, 0))) goto_out; tmp_dev->dev = log_dev; @@ -263,8 +280,10 @@ static int _ignore_blocked_mirror_devices(struct device *dev, } } - if (!check_for_blocking) - return 0; + if (!check_for_blocking) { + r = 1; + goto out; + } /* * We avoid another system call if we can, but if a device is @@ -293,16 +312,19 @@ static int _ignore_blocked_mirror_devices(struct device *dev, strstr(params, "handle_errors")) { log_debug("%s: I/O blocked to mirror device", dev_name(dev)); - return 1; + goto out; } } } while (next); - dm_task_destroy(dmt); - - return 0; + r = 1; out: - return 1; + if (dmt) + dm_task_destroy(dmt); + dm_free(log_health); + dm_free(images_health); + + return r; } int device_is_usable(struct device *dev) @@ -356,7 +378,7 @@ int device_is_usable(struct device *dev) &target_type, ¶ms); if (target_type && !strcmp(target_type, "mirror") && - _ignore_blocked_mirror_devices(dev, start, length, params)) { + !_ignore_blocked_mirror_devices(dev, start, length, params)) { log_debug("%s: Mirror device %s not usable.", dev_name(dev), name); goto out;