>From 6d96a8532358721fa6fbe4ae2f140b2117d5ae07 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Mon, 30 Nov 2009 14:13:24 +0000 Subject: [PATCH 5/5] daemon error handling: CHROOT_IN and CHROOT_OUT no longer preserve errno. These macros no longer preserve errno. Now you have to save errno yourself if you need it, ie: int r; int err; CHROOT_IN; r = some_system_call (); err = errno; /* preserve errno */ CHROOT_OUT; if (r == -1) { reply_with_perror_errno (err, "failed"); return -1; } --- daemon/daemon.h | 6 +----- daemon/dir.c | 25 ++++++++++++++++++------- daemon/fallocate.c | 4 +++- daemon/file.c | 46 ++++++++++++++++++++++++++++++++++------------ daemon/glob.c | 6 ++++-- daemon/link.c | 8 ++++++-- daemon/ls.c | 4 +++- daemon/mknod.c | 4 +++- daemon/mount.c | 8 ++++++-- daemon/readdir.c | 4 +++- daemon/realpath.c | 5 ++++- daemon/stat.c | 12 +++++++++--- daemon/statvfs.c | 4 +++- daemon/truncate.c | 4 +++- daemon/upload.c | 6 ++++-- daemon/utimens.c | 4 +++- daemon/xattr.c | 28 ++++++++++++++++++++-------- 17 files changed, 127 insertions(+), 51 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index 88d0306..f4e8009 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -210,21 +210,17 @@ extern void reply (xdrproc_t xdrp, char *ret); * (2) You must not change directory! cwd must always be "/", otherwise * we can't escape our own chroot. * (3) All paths specified must be absolute. - * (4) Neither macro affects errno. + * (4) Caution: these macros might modify errno. */ #define CHROOT_IN \ do { \ - int __old_errno = errno; \ if (chroot (sysroot) == -1) \ perror ("CHROOT_IN: sysroot"); \ - errno = __old_errno; \ } while (0) #define CHROOT_OUT \ do { \ - int __old_errno = errno; \ if (chroot (".") == -1) \ perror ("CHROOT_OUT: ."); \ - errno = __old_errno; \ } while (0) /* Marks functions which are not implemented. diff --git a/daemon/dir.c b/daemon/dir.c index 300bb4c..f23e116 100644 --- a/daemon/dir.c +++ b/daemon/dir.c @@ -33,13 +33,15 @@ int do_rmdir (const char *path) { int r; + int err; CHROOT_IN; r = rmdir (path); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("rmdir: %s", path); + reply_with_perror_errno (err, "rmdir: %s", path); return -1; } @@ -86,13 +88,15 @@ int do_mkdir (const char *path) { int r; + int err; CHROOT_IN; r = mkdir (path, 0777); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("mkdir: %s", path); + reply_with_perror_errno (err, "mkdir: %s", path); return -1; } @@ -103,13 +107,15 @@ int do_mkdir_mode (const char *path, int mode) { int r; + int err; CHROOT_IN; r = mkdir (path, mode); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("mkdir_mode: %s", path); + reply_with_perror_errno (err, "mkdir_mode: %s", path); return -1; } @@ -168,13 +174,15 @@ int do_mkdir_p (const char *path) { int r; + int err; CHROOT_IN; r = recursive_mkdir (path); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("mkdir -p: %s", path); + reply_with_perror_errno (err, "mkdir -p: %s", path); return -1; } if (r == -2) { @@ -190,14 +198,16 @@ do_is_dir (const char *path) { int r; struct stat buf; + int err; CHROOT_IN; r = lstat (path, &buf); + err = errno; CHROOT_OUT; if (r == -1) { - if (errno != ENOENT && errno != ENOTDIR) { - reply_with_perror ("stat: %s", path); + if (err != ENOENT && err != ENOTDIR) { + reply_with_perror_errno (err, "stat: %s", path); return -1; } else @@ -218,10 +228,11 @@ do_mkdtemp (const char *template) CHROOT_IN; char *r = mkdtemp (writable); + int err = errno; CHROOT_OUT; if (r == NULL) { - reply_with_perror ("mkdtemp: %s", template); + reply_with_perror_errno (err, "mkdtemp: %s", template); free (writable); } diff --git a/daemon/fallocate.c b/daemon/fallocate.c index 1800292..0628c32 100644 --- a/daemon/fallocate.c +++ b/daemon/fallocate.c @@ -31,12 +31,14 @@ int do_fallocate (const char *path, int len) { int fd; + int err; CHROOT_IN; fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("failed to open %s", path); + reply_with_perror_errno (err, "failed to open %s", path); return -1; } diff --git a/daemon/file.c b/daemon/file.c index 0b50eeb..a7aa2be 100644 --- a/daemon/file.c +++ b/daemon/file.c @@ -34,13 +34,15 @@ do_touch (const char *path) { int fd; int r; + int err; CHROOT_IN; fd = open (path, O_WRONLY | O_CREAT | O_NOCTTY, 0666); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return -1; } @@ -65,13 +67,15 @@ do_cat (const char *path) int fd; int alloc, size, r, max; char *buf, *buf2; + int err; CHROOT_IN; fd = open (path, O_RDONLY); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return NULL; } @@ -136,13 +140,15 @@ do_read_lines (const char *path) char *line = NULL; size_t len = 0; ssize_t n; + int err; CHROOT_IN; fp = fopen (path, "r"); + err = errno; CHROOT_OUT; if (!fp) { - reply_with_perror ("fopen: %s", path); + reply_with_perror_errno (err, "fopen: %s", path); return NULL; } @@ -180,13 +186,15 @@ int do_rm (const char *path) { int r; + int err; CHROOT_IN; r = unlink (path); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("unlink: %s", path); + reply_with_perror_errno (err, "unlink: %s", path); return -1; } @@ -197,13 +205,15 @@ int do_chmod (int mode, const char *path) { int r; + int err; CHROOT_IN; r = chmod (path, mode); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("chmod: %s: 0%o", path, mode); + reply_with_perror_errno (err, "chmod: %s: 0%o", path, mode); return -1; } @@ -214,13 +224,15 @@ int do_chown (int owner, int group, const char *path) { int r; + int err; CHROOT_IN; r = chown (path, owner, group); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("chown: %s: %d.%d", path, owner, group); + reply_with_perror_errno (err, "chown: %s: %d.%d", path, owner, group); return -1; } @@ -231,13 +243,15 @@ int do_lchown (int owner, int group, const char *path) { int r; + int err; CHROOT_IN; r = lchown (path, owner, group); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("lchown: %s: %d.%d", path, owner, group); + reply_with_perror_errno (err, "lchown: %s: %d.%d", path, owner, group); return -1; } @@ -261,14 +275,16 @@ do_is_file (const char *path) { int r; struct stat buf; + int err; CHROOT_IN; r = lstat (path, &buf); + err = errno; CHROOT_OUT; if (r == -1) { - if (errno != ENOENT && errno != ENOTDIR) { - reply_with_perror ("stat: %s", path); + if (err != ENOENT && err != ENOTDIR) { + reply_with_perror_errno (err, "stat: %s", path); return -1; } else @@ -282,16 +298,18 @@ int do_write_file (const char *path, const char *content, int size) { int fd; + int err; if (size == 0) size = strlen (content); CHROOT_IN; fd = open (path, O_WRONLY | O_TRUNC | O_CREAT | O_NOCTTY, 0666); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return -1; } @@ -315,13 +333,15 @@ do_read_file (const char *path, size_t *size_r) int fd; struct stat statbuf; char *r; + int err; CHROOT_IN; fd = open (path, O_RDONLY); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return NULL; } @@ -371,6 +391,7 @@ do_pread (const char *path, int count, int64_t offset, size_t *size_r) int fd; ssize_t r; char *buf; + int err; /* The actual limit on messages is smaller than this. This check * just limits the amount of memory we'll try and allocate in the @@ -384,10 +405,11 @@ do_pread (const char *path, int count, int64_t offset, size_t *size_r) CHROOT_IN; fd = open (path, O_RDONLY); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return NULL; } diff --git a/daemon/glob.c b/daemon/glob.c index 4fe76f3..fe1bd92 100644 --- a/daemon/glob.c +++ b/daemon/glob.c @@ -30,10 +30,12 @@ do_glob_expand (const char *pattern) { int r; glob_t buf; + int err; /* glob(3) in glibc never calls chdir, so this seems to be safe: */ CHROOT_IN; r = glob (pattern, GLOB_MARK|GLOB_BRACE, NULL, &buf); + err = errno; CHROOT_OUT; if (r == GLOB_NOMATCH) { /* Return an empty list instead of an error. */ @@ -45,8 +47,8 @@ do_glob_expand (const char *pattern) } if (r != 0) { - if (errno != 0) - reply_with_perror ("glob: %s", pattern); + if (err != 0) + reply_with_perror_errno (err, "glob: %s", pattern); else reply_with_error ("glob failed: %s", pattern); return NULL; diff --git a/daemon/link.c b/daemon/link.c index 5ea0d39..c16babd 100644 --- a/daemon/link.c +++ b/daemon/link.c @@ -34,12 +34,14 @@ do_readlink (const char *path) ssize_t r; char *ret; char link[PATH_MAX]; + int err; CHROOT_IN; r = readlink (path, link, sizeof link); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("readlink"); + reply_with_perror_errno (err, "readlink"); return NULL; } @@ -62,13 +64,15 @@ do_readlinklist (const char *path, char *const *names) const char *str; char **ret = NULL; int size = 0, alloc = 0; + int err; CHROOT_IN; fd_cwd = open (path, O_RDONLY | O_DIRECTORY); + err = errno; CHROOT_OUT; if (fd_cwd == -1) { - reply_with_perror ("readlinklist: %s", path); + reply_with_perror_errno (err, "readlinklist: %s", path); return NULL; } diff --git a/daemon/ls.c b/daemon/ls.c index 0af2356..c730cc2 100644 --- a/daemon/ls.c +++ b/daemon/ls.c @@ -36,13 +36,15 @@ do_ls (const char *path) int size = 0, alloc = 0; DIR *dir; struct dirent *d; + int err; CHROOT_IN; dir = opendir (path); + err = errno; CHROOT_OUT; if (!dir) { - reply_with_perror ("opendir: %s", path); + reply_with_perror_errno (err, "opendir: %s", path); return NULL; } diff --git a/daemon/mknod.c b/daemon/mknod.c index 46a1839..c8f0e2f 100644 --- a/daemon/mknod.c +++ b/daemon/mknod.c @@ -50,13 +50,15 @@ do_mknod (int mode, int devmajor, int devminor, const char *path) { #ifdef HAVE_MKNOD int r; + int err; CHROOT_IN; r = mknod (path, mode, makedev (devmajor, devminor)); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("mknod: %s", path); + reply_with_perror_errno (err, "mknod: %s", path); return -1; } diff --git a/daemon/mount.c b/daemon/mount.c index 49a0eab..81fa50a 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -354,16 +354,18 @@ int do_mkmountpoint (const char *path) { int r; + int err; /* NEED_ROOT (return -1); - we don't want this test for this call. */ ABS_PATH (path, return -1); CHROOT_IN; r = mkdir (path, 0777); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("mkmountpoint: %s", path); + reply_with_perror_errno (err, "mkmountpoint: %s", path); return -1; } @@ -379,16 +381,18 @@ int do_rmmountpoint (const char *path) { int r; + int err; /* NEED_ROOT (return -1); - we don't want this test for this call. */ ABS_PATH (path, return -1); CHROOT_IN; r = rmdir (path); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("rmmountpoint: %s", path); + reply_with_perror_errno (err, "rmmountpoint: %s", path); return -1; } diff --git a/daemon/readdir.c b/daemon/readdir.c index 876041e..58b5cd3 100644 --- a/daemon/readdir.c +++ b/daemon/readdir.c @@ -35,6 +35,7 @@ do_readdir (const char *path) DIR *dir; struct dirent *d; int i; + int err; ret = malloc (sizeof *ret); if (ret == NULL) { @@ -47,10 +48,11 @@ do_readdir (const char *path) CHROOT_IN; dir = opendir (path); + err = errno; CHROOT_OUT; if (dir == NULL) { - reply_with_perror ("opendir: %s", path); + reply_with_perror_errno (err, "opendir: %s", path); free (ret); return NULL; } diff --git a/daemon/realpath.c b/daemon/realpath.c index 0edb1d0..35b5dd0 100644 --- a/daemon/realpath.c +++ b/daemon/realpath.c @@ -51,12 +51,15 @@ do_realpath (const char *path) { #ifdef HAVE_REALPATH char *ret; + int err; CHROOT_IN; ret = realpath (path, NULL); + err = errno; CHROOT_OUT; + if (ret == NULL) { - reply_with_perror ("realpath"); + reply_with_perror_errno (err, "realpath"); return NULL; } diff --git a/daemon/stat.c b/daemon/stat.c index 20f4b70..ef93409 100644 --- a/daemon/stat.c +++ b/daemon/stat.c @@ -36,13 +36,15 @@ do_stat (const char *path) int r; guestfs_int_stat *ret; struct stat statbuf; + int err; CHROOT_IN; r = stat (path, &statbuf); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("stat"); + reply_with_perror_errno (err, "stat"); return NULL; } @@ -83,13 +85,15 @@ do_lstat (const char *path) int r; guestfs_int_stat *ret; struct stat statbuf; + int err; CHROOT_IN; r = lstat (path, &statbuf); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("stat"); + reply_with_perror_errno (err, "stat"); return NULL; } @@ -130,6 +134,7 @@ do_lstatlist (const char *path, char *const *names) int path_fd; guestfs_int_stat_list *ret; size_t i, nr_names; + int err; nr_names = count_strings (names); @@ -148,10 +153,11 @@ do_lstatlist (const char *path, char *const *names) CHROOT_IN; path_fd = open (path, O_RDONLY | O_DIRECTORY); + err = errno; CHROOT_OUT; if (path_fd == -1) { - reply_with_perror ("lstatlist: %s", path); + reply_with_perror_errno (err, "lstatlist: %s", path); free (ret->guestfs_int_stat_list_val); free (ret); return NULL; diff --git a/daemon/statvfs.c b/daemon/statvfs.c index e71b19a..a4b092c 100644 --- a/daemon/statvfs.c +++ b/daemon/statvfs.c @@ -46,13 +46,15 @@ do_statvfs (const char *path) int r; guestfs_int_statvfs *ret; struct statvfs statbuf; + int err; CHROOT_IN; r = statvfs (path, &statbuf); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("statvfs"); + reply_with_perror_errno (err, "statvfs"); return NULL; } diff --git a/daemon/truncate.c b/daemon/truncate.c index c2da382..fffb5eb 100644 --- a/daemon/truncate.c +++ b/daemon/truncate.c @@ -33,13 +33,15 @@ do_truncate_size (const char *path, int64_t size) { int fd; int r; + int err; CHROOT_IN; fd = open (path, O_WRONLY | O_NOCTTY); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return -1; } diff --git a/daemon/upload.c b/daemon/upload.c index fdb8654..40a256b 100644 --- a/daemon/upload.c +++ b/daemon/upload.c @@ -51,9 +51,9 @@ do_upload (const char *filename) if (!is_dev) CHROOT_IN; fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666); + err = errno; if (!is_dev) CHROOT_OUT; if (fd == -1) { - err = errno; cancel_receive (); errno = err; reply_with_perror ("%s", filename); @@ -92,14 +92,16 @@ do_download (const char *filename) { int fd, r, is_dev; char buf[GUESTFS_MAX_CHUNK_SIZE]; + int err; is_dev = STRPREFIX (filename, "/dev/"); if (!is_dev) CHROOT_IN; fd = open (filename, O_RDONLY); + err = errno; if (!is_dev) CHROOT_OUT; if (fd == -1) { - reply_with_perror ("%s", filename); + reply_with_perror_errno (err, "%s", filename); return -1; } diff --git a/daemon/utimens.c b/daemon/utimens.c index e836b4e..e0239e7 100644 --- a/daemon/utimens.c +++ b/daemon/utimens.c @@ -35,13 +35,15 @@ do_utimens (const char *path, { int fd; int r; + int err; CHROOT_IN; fd = open (path, O_WRONLY | O_NOCTTY); + err = errno; CHROOT_OUT; if (fd == -1) { - reply_with_perror ("open: %s", path); + reply_with_perror_errno (err, "open: %s", path); return -1; } diff --git a/daemon/xattr.c b/daemon/xattr.c index e58dc7e..4fa720e 100644 --- a/daemon/xattr.c +++ b/daemon/xattr.c @@ -122,12 +122,14 @@ getxattrs (const char *path, char *buf = NULL; int i, j; guestfs_int_xattr_list *r = NULL; + int err; CHROOT_IN; len = listxattr (path, NULL, 0); + err = errno; CHROOT_OUT; if (len == -1) { - reply_with_perror ("listxattr"); + reply_with_perror_errno (err, "listxattr"); goto error; } @@ -139,9 +141,10 @@ getxattrs (const char *path, CHROOT_IN; len = listxattr (path, buf, len); + err = errno; CHROOT_OUT; if (len == -1) { - reply_with_perror ("listxattr"); + reply_with_perror_errno (err, "listxattr"); goto error; } @@ -168,9 +171,10 @@ getxattrs (const char *path, for (i = 0, j = 0; i < len; i += strlen (&buf[i]) + 1, ++j) { CHROOT_IN; vlen = getxattr (path, &buf[i], NULL, 0); + err = errno; CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror_errno (err, "getxattr"); goto error; } @@ -188,9 +192,10 @@ getxattrs (const char *path, vlen = getxattr (path, &buf[i], r->guestfs_int_xattr_list_val[j].attrval.attrval_val, vlen); + err = errno; CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror_errno (err, "getxattr"); goto error; } } @@ -221,12 +226,14 @@ _setxattr (const char *xattr, const char *val, int vallen, const char *path, const void *value, size_t size, int flags)) { int r; + int err; CHROOT_IN; r = setxattr (path, xattr, val, vallen, 0); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("setxattr"); + reply_with_perror_errno (err, "setxattr"); return -1; } @@ -238,12 +245,14 @@ _removexattr (const char *xattr, const char *path, int (*removexattr) (const char *path, const char *name)) { int r; + int err; CHROOT_IN; r = removexattr (path, xattr); + err = errno; CHROOT_OUT; if (r == -1) { - reply_with_perror ("removexattr"); + reply_with_perror_errno (err, "removexattr"); return -1; } @@ -264,6 +273,7 @@ do_lxattrlist (const char *path, char *const *names) size_t k, m, nr_attrs; ssize_t len, vlen; char *buf = NULL; + int err; if (path_len >= PATH_MAX) { reply_with_perror ("lxattrlist: path longer than PATH_MAX"); @@ -366,9 +376,10 @@ do_lxattrlist (const char *path, char *const *names) for (i = 0, j = 0; i < len; i += strlen (&buf[i]) + 1, ++j) { CHROOT_IN; vlen = lgetxattr (pathname, &buf[i], NULL, 0); + err = errno; CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror_errno (err, "getxattr"); goto error; } @@ -385,9 +396,10 @@ do_lxattrlist (const char *path, char *const *names) CHROOT_IN; vlen = lgetxattr (pathname, &buf[i], entry[j+1].attrval.attrval_val, vlen); + err = errno; CHROOT_OUT; if (vlen == -1) { - reply_with_perror ("getxattr"); + reply_with_perror_errno (err, "getxattr"); goto error; } } -- 1.6.5.2