From 96c1351d8a9d749fbcd2aa756789bf0380e2c83a Mon Sep 17 00:00:00 2001 From: Bui Quang Minh Date: Sat, 13 Mar 2021 10:22:24 +0700 Subject: [PATCH] criu: Throw error when parent path is provided but invalid In image dump directory, there are 2 parent symlink error cases: - Parent symlink does not exist - Parent symlink exists but points to invalid target At the moment, 2 cases are handled exactly the same (do full dump). However, while the first case happen when parent path is not provided, the second one is likely user's mistake when provides invalid parent path. So we throw an error in the latter case instead of performing the full dump. Signed-off-by: Bui Quang Minh --- criu/image.c | 31 +++++++++++++++++++++++++++---- criu/include/image.h | 6 ++++++ criu/irmap.c | 6 ++---- criu/page-xfer.c | 24 +++++++++++++++--------- criu/pagemap.c | 6 ++++-- 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/criu/image.c b/criu/image.c index 6ba2ffc7d..06f770626 100644 --- a/criu/image.c +++ b/criu/image.c @@ -149,8 +149,7 @@ InventoryEntry *get_parent_inventory(void) InventoryEntry *ie; int dir; - dir = openat(get_service_fd(IMG_FD_OFF), CR_PARENT_LINK, O_RDONLY); - if (dir == -1) { + if (open_parent(get_service_fd(IMG_FD_OFF), &dir)) { /* * We print the warning below to be notified that we had some * unexpected problem on open. For instance we have a parent @@ -158,10 +157,11 @@ InventoryEntry *get_parent_inventory(void) * when also having no parent directory is an expected case of * first dump iteration. */ - if (errno != ENOENT) - pr_warn("Failed to open parent directory\n"); + pr_warn("Failed to open parent directory\n"); return NULL; } + if (dir < 0) + return NULL; img = open_image_at(dir, CR_FD_INVENTORY, O_RSTR); if (!img) { @@ -561,6 +561,11 @@ int open_image_dir(char *dir, int mode) if (img_streamer_init(dir, mode) < 0) goto err; } else if (opts.img_parent) { + if (faccessat(fd, opts.img_parent, R_OK, 0)) { + pr_perror("Invalid parent image directory provided"); + goto err; + } + ret = symlinkat(opts.img_parent, fd, CR_PARENT_LINK); if (ret < 0 && errno != EEXIST) { pr_perror("Can't link parent snapshot"); @@ -586,6 +591,24 @@ void close_image_dir(void) close_service_fd(IMG_FD_OFF); } +int open_parent(int dfd, int *pfd) +{ + *pfd = -1; + /* Check if the parent symlink exists */ + if (faccessat(dfd, CR_PARENT_LINK, F_OK, AT_SYMLINK_NOFOLLOW) && errno == ENOENT) { + pr_debug("No parent images directory provided\n"); + return 0; + } + + *pfd = openat(dfd, CR_PARENT_LINK, O_RDONLY); + if (*pfd < 0) { + pr_perror("Can't open parent path"); + return -1; + } + + return 0; +} + static unsigned long page_ids = 1; void up_page_ids_base(void) diff --git a/criu/include/image.h b/criu/include/image.h index 62c8d7ba0..7c45381db 100644 --- a/criu/include/image.h +++ b/criu/include/image.h @@ -147,6 +147,12 @@ extern off_t img_raw_size(struct cr_img *img); extern int open_image_dir(char *dir, int mode); extern void close_image_dir(void); +/* + * Return -1 -- parent symlink points to invalid target + * Return 0 && pfd < 0 -- parent symlink does not exist + * Return 0 && pfd >= 0 -- opened + */ +extern int open_parent(int dfd, int *pfd); extern struct cr_img *open_image_at(int dfd, int type, unsigned long flags, ...); #define open_image(typ, flags, ...) open_image_at(-1, typ, flags, ##__VA_ARGS__) diff --git a/criu/irmap.c b/criu/irmap.c index 58bc055d5..c9bfdbe22 100644 --- a/criu/irmap.c +++ b/criu/irmap.c @@ -425,12 +425,10 @@ in: close_image(*img); if (dir == AT_FDCWD) { pr_info("Searching irmap cache in parent\n"); - dir = openat(get_service_fd(IMG_FD_OFF), - CR_PARENT_LINK, O_RDONLY); + if (open_parent(get_service_fd(IMG_FD_OFF), &dir)) + return -1; if (dir >= 0) goto in; - if (errno != ENOENT) - return -1; } pr_info("No irmap cache\n"); diff --git a/criu/page-xfer.c b/criu/page-xfer.c index 98e9c6b6e..34f6a0967 100644 --- a/criu/page-xfer.c +++ b/criu/page-xfer.c @@ -364,10 +364,8 @@ static int open_page_local_xfer(struct page_xfer *xfer, int fd_type, unsigned lo return -1; xfer->pi = open_pages_image(O_DUMP, xfer->pmi, &pages_id); - if (!xfer->pi) { - close_image(xfer->pmi); - return -1; - } + if (!xfer->pi) + goto err_pmi; /* * Open page-read for parent images (if it exists). It will @@ -386,14 +384,15 @@ static int open_page_local_xfer(struct page_xfer *xfer, int fd_type, unsigned lo if (opts.stream) goto out; - pfd = openat(get_service_fd(IMG_FD_OFF), CR_PARENT_LINK, O_RDONLY); - if (pfd < 0 && errno == ENOENT) + if (open_parent(get_service_fd(IMG_FD_OFF), &pfd)) + goto err_pi; + if (pfd < 0) goto out; xfer->parent = xmalloc(sizeof(*xfer->parent)); if (!xfer->parent) { close(pfd); - return -1; + goto err_pi; } ret = open_page_read_at(pfd, img_id, xfer->parent, pr_flags); @@ -412,6 +411,12 @@ out: xfer->write_pages = write_pages_loc; xfer->close = close_page_xfer; return 0; + +err_pi: + close_image(xfer->pi); +err_pmi: + close_image(xfer->pmi); + return -1; } int open_page_xfer(struct page_xfer *xfer, int fd_type, unsigned long img_id) @@ -936,8 +941,9 @@ int check_parent_local_xfer(int fd_type, unsigned long img_id) if (opts.stream) return 0; - pfd = openat(get_service_fd(IMG_FD_OFF), CR_PARENT_LINK, O_RDONLY); - if (pfd < 0 && errno == ENOENT) + if (open_parent(get_service_fd(IMG_FD_OFF), &pfd)) + return -1; + if (pfd < 0) return 0; snprintf(path, sizeof(path), imgset_template[fd_type].fmt, img_id); diff --git a/criu/pagemap.c b/criu/pagemap.c index abcdcdfd5..1d849a1b9 100644 --- a/criu/pagemap.c +++ b/criu/pagemap.c @@ -661,8 +661,9 @@ static int try_open_parent(int dfd, unsigned long id, struct page_read *pr, int if (opts.stream) goto out; - pfd = openat(dfd, CR_PARENT_LINK, O_RDONLY); - if (pfd < 0 && errno == ENOENT) + if (open_parent(dfd, &pfd)) + goto err; + if (pfd < 0) goto out; parent = xmalloc(sizeof(*parent)); @@ -687,6 +688,7 @@ err_free: xfree(parent); err_cl: close(pfd); +err: return -1; }