From 3df63052544981f769bae7457df6a85e1020ca8c Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 20 Jan 2021 14:14:29 +0100 Subject: [PATCH 1/8] Change strnlen to sizeof in set_device_path() Signed-off-by: Slawomir Jankowski --- casadm/cas_lib.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 03bb047f8..5e1d89498 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -653,8 +653,7 @@ int set_device_path(char *dest_path, size_t dest_len, const char *src_path, size /* check if given dev_path is whitelisted and then pass it as path or not */ if (is_dev_link_whitelisted(abs_dev_path)){ - result = strncpy_s(dest_path, dest_len, abs_dev_path, - strnlen_s(abs_dev_path, sizeof(abs_dev_path))); + result = strncpy_s(dest_path, dest_len, abs_dev_path, sizeof(abs_dev_path)); return result ?: SUCCESS; } From 035ed3e7f9a03205ac9fe4ea2369b919739d1f5a Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 20 Jan 2021 14:19:20 +0100 Subject: [PATCH 2/8] Fix listing caches with path outside /dev/disk/by-id Signed-off-by: Slawomir Jankowski Signed-off-by: Adam Rutkowski --- casadm/cas_lib.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 5e1d89498..a712116f3 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -819,11 +819,14 @@ struct cache_device *get_cache_device(const struct kcas_cache_info *info, bool b cache->expected_core_count = info->info.core_count; cache->id = cache_id; cache->state = info->info.state; - if (set_device_path(cache->device, sizeof(cache->device), info->cache_path_name, - sizeof(info->cache_path_name)) != SUCCESS) { + + if (strncpy_s(cache->device, sizeof(cache->device), + info->cache_path_name, + sizeof(info->cache_path_name))) { free(cache); return NULL; } + cache->mode = info->info.cache_mode; cache->dirty = info->info.dirty; cache->flushed = info->info.flushed; @@ -2752,8 +2755,11 @@ int list_caches(unsigned int list_format, bool by_id_path) float core_flush_prog; if (!by_id_path) { - get_dev_path(curr_cache->device, curr_cache->device, - sizeof(curr_cache->device)); + if (get_dev_path(curr_cache->device, curr_cache->device, + sizeof(curr_cache->device))) { + cas_printf(LOG_WARNING, "WARNING: Cannot resolve path " + "to cache. By-id path will be shown for that cache.\n"); + } } cache_flush_prog = calculate_flush_progress(curr_cache->dirty, curr_cache->flushed); From 636e083e22d8f67723ee075ded502e8b32f22378 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 20 Jan 2021 14:24:38 +0100 Subject: [PATCH 3/8] Remove unneccesary variable Remove unused `buff` from `print_slow_atomic_cache_start_info()` Signed-off-by: Slawomir Jankowski --- casadm/cas_lib.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index a712116f3..ad9a7fa80 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -540,11 +540,9 @@ const char *get_core_state_name(int core_state) /* check if device is atomic and print information about potential slow start */ void print_slow_atomic_cache_start_info(const char *device_path) { - char buff[MAX_STR_LEN]; struct kcas_cache_check_device cmd_info; int ret; - get_dev_path(device_path, buff, sizeof(buff)); ret = _check_cache_device(device_path, &cmd_info); if (!ret && cmd_info.format_atomic) { From 5f5c150c30e000b11cd2a5cb62fd122a028462f8 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 20 Jan 2021 15:13:17 +0100 Subject: [PATCH 4/8] Add comment about calling only after normalizing path Signed-off-by: Slawomir Jankowski --- casadm/cas_lib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index ad9a7fa80..8c80a4a38 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -630,6 +630,7 @@ static bool is_dev_link_whitelisted(const char* path) return false; } +/* Call this only AFTER normalizing path */ static int _is_by_id_path(const char* dev_path) { static const char dev_by_id_dir[] = "/dev/disk/by-id"; From 1fb5fd9662f69bfa9962d056b10c5a87609e3fc1 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 20 Jan 2021 16:53:30 +0100 Subject: [PATCH 5/8] Verify whether input device path is actually a block device This applies to add/momove core, start/stop cache, zero superblock commands. Signed-off-by: Adam Rutkowski --- casadm/cas_lib.c | 14 ++++++++++++++ casadm/cas_main.c | 9 +++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 8c80a4a38..345058398 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -159,10 +159,24 @@ cas_printf_t cas_printf = std_printf; int validate_dev(const char *dev_path) { struct fstab *fstab_entry; + struct stat status; + fstab_entry = getfsspec(dev_path); if (fstab_entry != NULL) { + printf("Device entry present in fstab, please remove it.\n"); + return FAILURE; + } + + if (stat(dev_path, &status) == -1) { + printf("Failed to query device status.\n"); return FAILURE; } + + if (!S_ISBLK(status.st_mode)) { + printf("Path does not describe a block device\n"); + return FAILURE; + } + return SUCCESS; } diff --git a/casadm/cas_main.c b/casadm/cas_main.c index ecd9ba80b..c9e26324c 100644 --- a/casadm/cas_main.c +++ b/casadm/cas_main.c @@ -92,16 +92,13 @@ static struct command_args command_args_values = { }; int validate_device_name(const char *dev_name) { - if (validate_dev(dev_name)) { - cas_printf(LOG_ERR, "Cache creation aborted, %s entry exists in /etc/fstab. Please remove it!\n", - dev_name); + if (strnlen(dev_name, MAX_STR_LEN) >= MAX_STR_LEN) { + cas_printf(LOG_ERR, "Illegal device name\n"); return FAILURE; } - if (strnlen(dev_name, MAX_STR_LEN) >= MAX_STR_LEN) { - cas_printf(LOG_ERR, "Illegal device %s\n", dev_name); + if (validate_dev(dev_name)) return FAILURE; - } return SUCCESS; } From 1fcb175fee67f8d3e92f93b9958e983c01ac4b4a Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Jan 2021 12:45:17 -0600 Subject: [PATCH 6/8] Fix probing cache device in casadm Signed-off-by: Adam Rutkowski --- casadm/cas_lib.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 345058398..d1bb0250f 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -2849,6 +2849,11 @@ int _check_cache_device(const char *device_path, { int result, fd; + if (strncpy_s(cmd_info->path_name, sizeof(cmd_info->path_name), + device_path, MAX_STR_LEN)) { + return FAILURE; + } + fd = open_ctrl_device(); if (fd == -1) return FAILURE; @@ -2857,7 +2862,7 @@ int _check_cache_device(const char *device_path, close(fd); - return result; + return result ? FAILURE : SUCCESS; } int check_cache_device(const char *device_path) From 5f65d93a3608de8bb5d9a2353c241eb6e223216d Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 20 Jan 2021 18:39:09 +0100 Subject: [PATCH 7/8] Replace cache started check with exclusive open in zero superblock. This check covers the previous case (cache started) as well as other potential incorrect usages of the command. Also now the check is made exactly at the moment of opening the device to write to. Signed-off-by: Adam Rutkowski --- casadm/cas_lib.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index d1bb0250f..6d57640b5 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -2923,13 +2923,6 @@ int zero_md(const char *cache_device){ } close(fd); - /* don't delete metadata if cache is in use */ - if (check_cache_already_added(cache_device) == FAILURE) { - cas_printf(LOG_ERR, "Cache device '%s' is already used as cache. " - "Please stop cache to clear metadata.\n", cache_device); - return FAILURE; - } - result = _check_cache_device(cache_device, &cmd_info); if (result == FAILURE) { cas_printf(LOG_ERR, "Failed to retrieve device's information.\n"); @@ -2942,9 +2935,11 @@ int zero_md(const char *cache_device){ return FAILURE; } - fd = open(cache_device, O_WRONLY | O_SYNC); + fd = open(cache_device, O_WRONLY | O_SYNC | O_EXCL); if (fd < 0) { - cas_printf(LOG_ERR, "Error while opening '%s' to purge metadata.\n", cache_device); + cas_printf(LOG_ERR, "Error while opening '%s'exclusively. This can be due to\n" + "cache instance running on this device. In such case please\n" + "stop the cache and try again.\n", cache_device); return FAILURE; } From 2b739d116527514c39e18d99787016cd1afbf2c2 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 21 Jan 2021 13:07:41 -0600 Subject: [PATCH 8/8] Check exclusive device access before probing metadata Signed-off-by: Adam Rutkowski --- casadm/cas_lib.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 6d57640b5..452ce0623 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -2915,10 +2915,12 @@ int zero_md(const char *cache_device){ int fd = 0; int result; - /* check if given cache device exists */ - fd = open(cache_device, O_RDONLY); + /* check if device is available */ + fd = open(cache_device, O_WRONLY | O_SYNC | O_EXCL); if (fd < 0) { - cas_printf(LOG_ERR, "Device '%s' not found.\n", cache_device); + cas_printf(LOG_ERR, "Error while opening '%s'exclusively. This can be due to\n" + "cache instance running on this device. In such case please " + "stop the cache and try again.\n", cache_device); return FAILURE; } close(fd);