From: Matthias Gerstner Date: Mon, 17 Jul 2017 11:35:25 +0200 Subject: removed all check_config callback implementations to avoid security issues Git-commit: 8cf8208775022301adaa59c240bb7f93742d1329 References: bsc#1049491 see github issue #194 qcow.c contained an information leak, could test for existance of any file in the system file_example.c and file_optical.c allow also to test for existance of any file, plus to temporarily create empty new files anywhere in the file system. This also involves a race condition, if a file didn't exist in the first place, but would be created in-between by some other process, then the file would be deleted by the check_config implementation. Acked-by: Lee Duncan --- consumer.c | 7 ------ file_example.c | 31 ---------------------------- file_optical.c | 57 ----------------------------------------------------- qcow.c | 23 --------------------- tcmu-synthesizer.c | 11 ---------- 5 files changed, 129 deletions(-) --- a/consumer.c +++ b/consumer.c @@ -54,11 +54,6 @@ static int set_medium_error(uint8_t *sen return tcmu_set_sense_data(sense, MEDIUM_ERROR, ASC_READ_ERROR, NULL); } -static bool foo_check_config(const char *cfgstring, char **reason) -{ - return true; -} - static int foo_open(struct tcmu_device *dev) { /* open the backing file */ @@ -137,8 +132,6 @@ static struct tcmulib_handler foo_handle .subtype = "foo", .cfg_desc = "a description goes here", - .check_config = foo_check_config, - .added = foo_open, .removed = foo_close, }; --- a/file_example.c +++ b/file_example.c @@ -143,35 +143,6 @@ file_handler_destroy(struct file_handler } #endif /* ASYNC_FILE_HANDLER */ -static bool file_check_config(const char *cfgstring, char **reason) -{ - char *path; - int fd; - - path = strchr(cfgstring, '/'); - if (!path) { - if (asprintf(reason, "No path found") == -1) - *reason = NULL; - return false; - } - path += 1; /* get past '/' */ - - if (access(path, W_OK) != -1) - return true; /* File exists and is writable */ - - /* We also support creating the file, so see if we can create it */ - fd = creat(path, S_IRUSR | S_IWUSR); - if (fd == -1) { - if (asprintf(reason, "Could not create file") == -1) - *reason = NULL; - return false; - } - - unlink(path); - - return true; -} - static int file_open(struct tcmu_device *dev) { struct file_state *state; @@ -401,8 +372,6 @@ static const char file_cfg_desc[] = static struct tcmur_handler file_handler = { .cfg_desc = file_cfg_desc, - .check_config = file_check_config, - .open = file_open, .close = file_close, #ifdef ASYNC_FILE_HANDLER --- a/file_optical.c +++ b/file_optical.c @@ -176,61 +176,6 @@ static void fbo_handler_destroy(struct f pthread_mutex_destroy(&h->mtx); } -static bool fbo_check_config(const char *cfgstring, char **reason) -{ - char *options; - char *path; - int fd; - - tcmu_dbg("check: cfgstring %s\n", cfgstring); - options = strchr(cfgstring, '/'); - if (!options) { - if (asprintf(reason, "Invalid cfgstring") == -1) - *reason = NULL; - return false; - } - options += 1; /* get past '/' */ - while (options[0] != '/') { - if (strncasecmp(options, "ro/", 3)) { - if (asprintf(reason, "Unknown option %s\n", - options) == -1) - *reason = NULL; - return false; - } - - options = strchr(options, '/'); - if (!options) { - if (asprintf(reason, "Invalid cfgstring") == -1) - *reason = NULL; - return false; - } - options += 1; - } - - path = options; - if (!path) { - if (asprintf(reason, "No path found") == -1) - *reason = NULL; - return false; - } - - if (access(path, R_OK) != -1) - return true; /* File exists */ - - /* We also support creating the file, so see if we can create it */ - /* NB: If we're creating it, then we'll need write permission */ - fd = creat(path, S_IRUSR | S_IWUSR); - if (fd == -1) { - if (asprintf(reason, "Could not create file") == -1) - *reason = NULL; - return false; - } - - unlink(path); - - return true; -} - /* Note: this is called per lun, not per mapping */ static int fbo_open(struct tcmu_device *dev) { @@ -1764,8 +1709,6 @@ static const char fbo_cfg_desc[] = static struct tcmur_handler fbo_handler = { .cfg_desc = fbo_cfg_desc, - .check_config = fbo_check_config, - .open = fbo_open, .close = fbo_close, .name = "File-backed optical Handler", --- a/qcow.c +++ b/qcow.c @@ -1389,27 +1389,6 @@ static struct bdev_ops raw_ops = { /* TCMU QCOW Handler */ -static bool qcow_check_config(const char *cfgstring, char **reason) -{ - char *path; - - path = strchr(cfgstring, '/'); - if (!path) { - if (asprintf(reason, "No path found") == -1) - *reason = NULL; - return false; - } - path += 1; /* get past '/' */ - - if (access(path, R_OK|W_OK) == -1) { - if (asprintf(reason, "File not present, or not writable") == -1) - *reason = NULL; - return false; - } - - return true; /* File exists and is writable */ -} - static int qcow_open(struct tcmu_device *dev) { struct bdev *bdev; @@ -1574,8 +1553,6 @@ static struct tcmur_handler qcow_handler .subtype = "qcow", .cfg_desc = qcow_cfg_desc, - .check_config = qcow_check_config, - .open = qcow_open, .close = qcow_close, .handle_cmd = qcow_handle_cmd, --- a/tcmu-synthesizer.c +++ b/tcmu-synthesizer.c @@ -35,16 +35,6 @@ typedef struct { int watcher_id; } syn_dev_t; -static bool syn_check_config(const char *cfgstring, char **reason) -{ - tcmu_dbg("syn check config\n"); - if (strcmp(cfgstring, "syn/null")) { - asprintf(reason, "invalid option"); - return false; - } - return true; -} - static int syn_handle_cmd(struct tcmu_device *dev, uint8_t *cdb, struct iovec *iovec, size_t iov_cnt, uint8_t *sense) @@ -147,7 +137,6 @@ struct tcmulib_handler syn_handler = { .cfg_desc = "valid options:\n" "null: a nop storage where R/W requests are completed " "immediately, like the null_blk device.", - .check_config = syn_check_config, .added = syn_added, .removed = syn_removed, };