From 3696674bc62b0c1250027dbeedafdd7ebafdcf8b Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Sat, 11 Apr 2009 11:31:31 -0700 Subject: [PATCH] More backup improvements: - Changed get_backup_name() to verify the backup path, and make any missing directories. This avoids accidental use of a symlink as a dir in a backup path, and gets rid of any other non-dirs that are in the way. It also avoids the need for various operations to retry after calling make_bak_dir(), simplifying several pices of code. - Changed create_directory_path() to make_path(), giving it flags that lets the caller decide if it should skip a leading slash or drop the trailing filename. - Mention when we create the backup directory, so the user is not caught unaware when rsync uses a directory they didn't expect. - Got rid of some dir-moving backup code that is not used. - Added a little more backup-debug output. --- backup.c | 267 ++++++++++++++++++++++------------------------------ generator.c | 29 ++---- main.c | 14 +++ options.c | 10 +- receiver.c | 2 +- rsync.h | 4 + t_stub.c | 5 - util.c | 98 +++++++++++++------ 8 files changed, 216 insertions(+), 213 deletions(-) diff --git a/backup.c b/backup.c index 5f860f78..8512e00c 100644 --- a/backup.c +++ b/backup.c @@ -34,111 +34,119 @@ extern char backup_dir_buf[MAXPATHLEN]; extern char *backup_suffix; extern char *backup_dir; -/* make a complete pathname for backup file */ -char *get_backup_name(const char *fname) -{ - if (backup_dir) { - if (stringjoin(backup_dir_buf + backup_dir_len, backup_dir_remainder, - fname, backup_suffix, NULL) < backup_dir_remainder) - return backup_dir_buf; - } else { - if (stringjoin(backup_dir_buf, MAXPATHLEN, - fname, backup_suffix, NULL) < MAXPATHLEN) - return backup_dir_buf; - } - - rprintf(FERROR, "backup filename too long\n"); - return NULL; -} - -/**************************************************************************** -Create a directory given an absolute path, perms based upon another directory -path -****************************************************************************/ -int make_bak_dir(const char *fullpath) +/* Create a backup path from the given fname, putting the result into + * backup_dir_buf. Any new directories (compared to the prior backup + * path) are ensured to exist as directories, replacing anything else + * that may be in the way (e.g. a symlink). */ +static BOOL copy_valid_path(const char *fname) { - char fbuf[MAXPATHLEN], *rel, *end, *p; - struct file_struct *file; - int len = backup_dir_len; + const char *f; + int flags; + BOOL ret = True; stat_x sx; + char *b, *rel = backup_dir_buf + backup_dir_len, *name = rel; - while (*fullpath == '.' && fullpath[1] == '/') { - fullpath += 2; - len -= 2; + for (f = fname, b = rel; *f && *f == *b; f++, b++) { + if (*b == '/') + name = b + 1; } - if (strlcpy(fbuf, fullpath, sizeof fbuf) >= sizeof fbuf) - return -1; + if (stringjoin(rel, backup_dir_remainder, fname, backup_suffix, NULL) >= backup_dir_remainder) { + rprintf(FERROR, "backup filename too long\n"); + *name = '\0'; + return False; + } - rel = fbuf + len; - end = p = rel + strlen(rel); + for ( ; ; name = b + 1) { + if ((b = strchr(name, '/')) == NULL) + return True; + *b = '\0'; - /* Try to find an existing dir, starting from the deepest dir. */ - while (1) { - if (--p == fbuf) - return -1; - if (*p == '/') { - *p = '\0'; - if (mkdir_defmode(fbuf) == 0) + if (do_lstat(backup_dir_buf, &sx.st) < 0) { + if (errno == ENOENT) break; - if (errno != ENOENT) { - rsyserr(FERROR, errno, - "make_bak_dir mkdir %s failed", - full_fname(fbuf)); - return -1; - } + rsyserr(FERROR, errno, "backup lstat %s failed", backup_dir_buf); + *name = '\0'; + return False; + } + if (!S_ISDIR(sx.st.st_mode)) { + flags = get_del_for_flag(sx.st.st_mode) | DEL_FOR_BACKUP | DEL_RECURSE; + if (delete_item(backup_dir_buf, sx.st.st_mode, flags) == 0) + break; + *name = '\0'; + return False; } + + *b = '/'; } - /* Make all the dirs that we didn't find on the way here. */ - while (1) { - if (p >= rel) { - /* Try to transfer the directory settings of the - * actual dir that the files are coming from. */ - init_stat_x(&sx); - if (x_stat(rel, &sx.st, NULL) < 0) { - rsyserr(FERROR, errno, - "make_bak_dir stat %s failed", - full_fname(rel)); - } else { - if (!(file = make_file(rel, NULL, NULL, 0, NO_FILTERS))) - continue; + init_stat_x(&sx); + + for ( ; b; name = b + 1, b = strchr(name, '/')) { + *b = '\0'; + + if (mkdir_defmode(backup_dir_buf) < 0) { + rsyserr(FERROR, errno, "backup mkdir %s failed", backup_dir_buf); + *name = '\0'; + ret = False; + break; + } + + /* Try to transfer the directory settings of the actual dir + * that the files are coming from. */ + if (x_stat(rel, &sx.st, NULL) < 0) + rsyserr(FERROR, errno, "backup stat %s failed", full_fname(rel)); + else { + struct file_struct *file; + if (!(file = make_file(rel, NULL, NULL, 0, NO_FILTERS))) + continue; #ifdef SUPPORT_ACLS - if (preserve_acls && !S_ISLNK(file->mode)) { - get_acl(rel, &sx); - cache_tmp_acl(file, &sx); - free_acl(&sx); - } + if (preserve_acls && !S_ISLNK(file->mode)) { + get_acl(rel, &sx); + cache_tmp_acl(file, &sx); + free_acl(&sx); + } #endif #ifdef SUPPORT_XATTRS - if (preserve_xattrs) { - get_xattr(rel, &sx); - cache_tmp_xattr(file, &sx); - free_xattr(&sx); - } + if (preserve_xattrs) { + get_xattr(rel, &sx); + cache_tmp_xattr(file, &sx); + free_xattr(&sx); + } #endif - set_file_attrs(fbuf, file, NULL, NULL, 0); - unmake_file(file); + set_file_attrs(backup_dir_buf, file, NULL, NULL, 0); + unmake_file(file); + } + + *b = '/'; + } + #ifdef SUPPORT_ACLS - uncache_tmp_acls(); + uncache_tmp_acls(); #endif #ifdef SUPPORT_XATTRS - uncache_tmp_xattrs(); + uncache_tmp_xattrs(); #endif - } - } - *p = '/'; - p += strlen(p); - if (p == end) - break; - if (mkdir_defmode(fbuf) < 0) { - rsyserr(FERROR, errno, "make_bak_dir mkdir %s failed", - full_fname(fbuf)); - return -1; - } + + return ret; +} + +/* Make a complete pathname for backup file and verify any new path elements. */ +char *get_backup_name(const char *fname) +{ + if (backup_dir) { + /* copy fname into backup_dir_buf while validating the dirs. */ + if (copy_valid_path(fname)) + return backup_dir_buf; + return NULL; + } else { + if (stringjoin(backup_dir_buf, MAXPATHLEN, + fname, backup_suffix, NULL) < MAXPATHLEN) + return backup_dir_buf; } - return 0; + rprintf(FERROR, "backup filename too long\n"); + return NULL; } /* Has same return codes as make_backup(). */ @@ -155,8 +163,11 @@ static inline int link_or_rename(const char *from, const char *to, if (IS_SPECIAL(stp->st_mode) || IS_DEVICE(stp->st_mode)) return 0; /* Use copy code. */ #endif - if (do_link(from, to) == 0) + if (do_link(from, to) == 0) { + if (DEBUG_GTE(BACKUP, 1)) + rprintf(FINFO, "make_backup: HLINK %s successful.\n", from); return 2; + } /* We prefer to rename a regular file rather than copy it. */ if (!S_ISREG(stp->st_mode) || errno == EEXIST || errno == EISDIR) return 0; @@ -168,6 +179,8 @@ static inline int link_or_rename(const char *from, const char *to, * dir, rename() might return success but do nothing! */ robust_unlink(from); /* Just in case... */ } + if (DEBUG_GTE(BACKUP, 1)) + rprintf(FINFO, "make_backup: RENAME %s successful.\n", from); return 1; } return 0; @@ -205,12 +218,6 @@ int make_backup(const char *fname, BOOL prefer_rename) } if ((ret = link_or_rename(fname, buf, prefer_rename, &sx.st)) != 0) goto success; - } else if (backup_dir && errno == ENOENT) { - /* If the backup dir is missing, try again after making it. */ - if (make_bak_dir(buf) != 0) - return 0; - if ((ret = link_or_rename(fname, buf, prefer_rename, &sx.st)) != 0) - goto success; } /* Fall back to making a copy. */ @@ -235,50 +242,10 @@ int make_backup(const char *fname, BOOL prefer_rename) /* Check to see if this is a device file, or link */ if ((am_root && preserve_devices && IS_DEVICE(file->mode)) || (preserve_specials && IS_SPECIAL(file->mode))) { - int save_errno; - if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0) { - save_errno = errno ? errno : EINVAL; /* 0 paranoia */ - if (errno == ENOENT && make_bak_dir(buf) == 0) { - if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0) - save_errno = errno ? errno : save_errno; - else - save_errno = 0; - } - if (save_errno) { - rsyserr(FERROR, save_errno, "mknod %s failed", - full_fname(buf)); - } - } else - save_errno = 0; - if (DEBUG_GTE(BACKUP, 1) && save_errno == 0) { - rprintf(FINFO, "make_backup: DEVICE %s successful.\n", - fname); - } - ret = 2; - } - - if (!ret && S_ISDIR(file->mode)) { - int ret_code; - /* make an empty directory */ - if (do_mkdir(buf, file->mode) < 0) { - int save_errno = errno ? errno : EINVAL; /* 0 paranoia */ - if (errno == ENOENT && make_bak_dir(buf) == 0) { - if (do_mkdir(buf, file->mode) < 0) - save_errno = errno ? errno : save_errno; - else - save_errno = 0; - } - if (save_errno) { - rsyserr(FINFO, save_errno, "mkdir %s failed", - full_fname(buf)); - } - } - - ret_code = do_rmdir(fname); - if (DEBUG_GTE(BACKUP, 1)) { - rprintf(FINFO, "make_backup: RMDIR %s returns %i\n", - full_fname(fname), ret_code); - } + if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0) + rsyserr(FERROR, errno, "mknod %s failed", full_fname(buf)); + else if (DEBUG_GTE(BACKUP, 1)) + rprintf(FINFO, "make_backup: DEVICE %s successful.\n", fname); ret = 2; } @@ -292,27 +259,17 @@ int make_backup(const char *fname, BOOL prefer_rename) } ret = 2; } else { - if (do_symlink(sl, buf) < 0) { - int save_errno = errno ? errno : EINVAL; /* 0 paranoia */ - if (errno == ENOENT && make_bak_dir(buf) == 0) { - if (do_symlink(sl, buf) < 0) - save_errno = errno ? errno : save_errno; - else - save_errno = 0; - } - if (save_errno) { - rsyserr(FERROR, save_errno, "link %s -> \"%s\"", - full_fname(buf), sl); - } - } + if (do_symlink(sl, buf) < 0) + rsyserr(FERROR, errno, "link %s -> \"%s\"", full_fname(buf), sl); + else if (DEBUG_GTE(BACKUP, 1)) + rprintf(FINFO, "make_backup: SYMLINK %s successful.\n", fname); ret = 2; } } #endif if (!ret && !S_ISREG(file->mode)) { - rprintf(FINFO, "make_bak: skipping non-regular file %s\n", - fname); + rprintf(FINFO, "make_bak: skipping non-regular file %s\n", fname); unmake_file(file); #ifdef SUPPORT_ACLS uncache_tmp_acls(); @@ -325,7 +282,7 @@ int make_backup(const char *fname, BOOL prefer_rename) /* Copy to backup tree if a file. */ if (!ret) { - if (copy_file(fname, buf, -1, file->mode, 1) < 0) { + if (copy_file(fname, buf, -1, file->mode) < 0) { rsyserr(FERROR, errno, "keep_backup failed: %s -> \"%s\"", full_fname(fname), buf); unmake_file(file); @@ -337,6 +294,8 @@ int make_backup(const char *fname, BOOL prefer_rename) #endif return 0; } + if (DEBUG_GTE(BACKUP, 1)) + rprintf(FINFO, "make_backup: COPY %s successful.\n", fname); ret = 2; } @@ -354,9 +313,7 @@ int make_backup(const char *fname, BOOL prefer_rename) #endif success: - if (INFO_GTE(BACKUP, 1)) { - rprintf(FINFO, "backed up %s to %s\n", - fname, buf); - } + if (INFO_GTE(BACKUP, 1)) + rprintf(FINFO, "backed up %s to %s\n", fname, buf); return ret; } diff --git a/generator.c b/generator.c index 12007a18..53ba16f6 100644 --- a/generator.c +++ b/generator.c @@ -756,7 +756,7 @@ static int copy_altdest_file(const char *src, const char *dest, struct file_stru copy_to = buf; } cleanup_set(copy_to, NULL, NULL, -1, -1); - if (copy_file(src, copy_to, fd_w, file->mode, 0) < 0) { + if (copy_file(src, copy_to, fd_w, file->mode) < 0) { if (INFO_GTE(COPY, 1)) { rsyserr(FINFO, errno, "copy_file %s => %s", full_fname(src), copy_to); @@ -1148,7 +1148,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, && do_stat(dn, &sx.st) < 0) { if (dry_run) goto parent_is_dry_missing; - if (create_directory_path(fname) < 0) { + if (make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0) { rsyserr(FERROR_XFER, errno, "recv_generator: mkdir %s failed", full_fname(dn)); @@ -1274,8 +1274,8 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, } if (real_ret != 0 && do_mkdir(fname,file->mode) < 0 && errno != EEXIST) { if (!relative_paths || errno != ENOENT - || create_directory_path(fname) < 0 - || (do_mkdir(fname, file->mode) < 0 && errno != EEXIST)) { + || make_path(fname, MKP_DROP_NAME | MKP_SKIP_SLASH) < 0 + || (do_mkdir(fname, file->mode) < 0 && errno != EEXIST)) { rsyserr(FERROR_XFER, errno, "recv_generator: mkdir %s failed", full_fname(fname)); @@ -1657,7 +1657,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, goto cleanup; if (!(back_file = make_file(fname, NULL, NULL, 0, NO_FILTERS))) goto pretend_missing; - if (copy_file(fname, backupptr, -1, back_file->mode, 1) < 0) { + if (copy_file(fname, backupptr, -1, back_file->mode) < 0) { unmake_file(back_file); back_file = NULL; goto cleanup; @@ -1706,20 +1706,11 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx, goto cleanup; } if ((f_copy = do_open(backupptr, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600)) < 0) { - int save_errno = errno ? errno : EINVAL; /* 0 paranoia */ - if (errno == ENOENT && make_bak_dir(backupptr) == 0) { - if ((f_copy = do_open(backupptr, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600)) < 0) - save_errno = errno ? errno : save_errno; - else - save_errno = 0; - } - if (save_errno) { - rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(backupptr)); - unmake_file(back_file); - back_file = NULL; - close(fd); - goto cleanup; - } + rsyserr(FERROR_XFER, errno, "open %s", full_fname(backupptr)); + unmake_file(back_file); + back_file = NULL; + close(fd); + goto cleanup; } fnamecmp_type = FNAMECMP_BACKUP; } diff --git a/main.c b/main.c index 2ef2f479..75e9a2fe 100644 --- a/main.c +++ b/main.c @@ -77,7 +77,9 @@ extern char *rsync_path; extern char *shell_cmd; extern char *batch_name; extern char *password_file; +extern char *backup_dir; extern char curr_dir[MAXPATHLEN]; +extern char backup_dir_buf[MAXPATHLEN]; extern char *basis_dir[MAX_BASIS_DIRS+1]; extern struct file_list *first_flist; extern struct filter_list_struct daemon_filter_list; @@ -817,6 +819,18 @@ static int do_recv(int f_in, int f_out, char *local_name) exit_cleanup(RERR_IPC); } + if (backup_dir) { + int ret = make_path(backup_dir_buf, MKP_DROP_NAME); /* drops trailing slash */ + if (ret < 0) + exit_cleanup(RERR_SYNTAX); + if (ret) + rprintf(FINFO, "Created backup_dir %s\n", backup_dir_buf); + else if (INFO_GTE(BACKUP, 1)) { + char *dir = *backup_dir_buf ? backup_dir_buf : "."; + rprintf(FINFO, "backup_dir is %s\n", dir); + } + } + io_flush(NORMAL_FLUSH); if ((pid = do_fork()) == -1) { diff --git a/options.c b/options.c index e7c6c612..5af0a319 100644 --- a/options.c +++ b/options.c @@ -1997,19 +1997,21 @@ int parse_arguments(int *argc_p, const char ***argv_p) return 0; } if (backup_dir) { + while (*backup_dir == '.' && backup_dir[1] == '/') + backup_dir += 2; + if (*backup_dir == '.' && backup_dir[1] == '\0') + backup_dir++; backup_dir_len = strlcpy(backup_dir_buf, backup_dir, sizeof backup_dir_buf); backup_dir_remainder = sizeof backup_dir_buf - backup_dir_len; - if (backup_dir_remainder < 32) { + if (backup_dir_remainder < 128) { snprintf(err_buf, sizeof err_buf, "the --backup-dir path is WAY too long.\n"); return 0; } - if (backup_dir_buf[backup_dir_len - 1] != '/') { + if (backup_dir_len && backup_dir_buf[backup_dir_len - 1] != '/') { backup_dir_buf[backup_dir_len++] = '/'; backup_dir_buf[backup_dir_len] = '\0'; } - if (INFO_GTE(BACKUP, 1) && !am_sender) - rprintf(FINFO, "backup_dir is %s\n", backup_dir_buf); } else if (!backup_suffix_len && (!am_server || !am_sender)) { snprintf(err_buf, sizeof err_buf, "--suffix cannot be a null string without --backup-dir\n"); diff --git a/receiver.c b/receiver.c index ee8e015a..245ba444 100644 --- a/receiver.c +++ b/receiver.c @@ -147,7 +147,7 @@ int open_tmpfile(char *fnametmp, const char *fname, struct file_struct *file) * information should have been previously transferred, but that may * not be the case with -R */ if (fd == -1 && relative_paths && errno == ENOENT - && create_directory_path(fnametmp) == 0) { + && make_path(fnametmp, MKP_SKIP_SLASH | MKP_DROP_NAME) == 0) { /* Get back to name with XXXXXX in it. */ get_tmpname(fnametmp, fname); fd = do_mkstemp(fnametmp, file->mode & INITACCESSPERMS); diff --git a/rsync.h b/rsync.h index be7cf8a5..1b0397e1 100644 --- a/rsync.h +++ b/rsync.h @@ -259,6 +259,10 @@ enum delret { DR_SUCCESS = 0, DR_FAILURE, DR_AT_LIMIT, DR_NOT_EMPTY }; +/* Defines for make_path() */ +#define MKP_DROP_NAME (1<<0) /* drop trailing filename or trailing slash */ +#define MKP_SKIP_SLASH (1<<1) /* skip one or more leading slashes */ + #include "errcode.h" #include "config.h" diff --git a/t_stub.c b/t_stub.c index 02cfa69c..db6d1fe3 100644 --- a/t_stub.c +++ b/t_stub.c @@ -65,11 +65,6 @@ struct filter_list_struct daemon_filter_list; return 0; } - int make_bak_dir(UNUSED(const char *fullpath)) -{ - return -1; -} - int copy_xattrs(UNUSED(const char *source), UNUSED(const char *dest)) { return -1; diff --git a/util.c b/util.c index 0cafed69..bad2faed 100644 --- a/util.c +++ b/util.c @@ -185,27 +185,76 @@ int mkdir_defmode(char *fname) } /* Create any necessary directories in fname. Any missing directories are - * created with default permissions. */ -int create_directory_path(char *fname) + * created with default permissions. Returns < 0 on error, or the number + * of directories created. */ +int make_path(char *fname, int flags) { - char *p; + char *end, *p; int ret = 0; - while (*fname == '/') - fname++; - while (strncmp(fname, "./", 2) == 0) + if (flags & MKP_SKIP_SLASH) { + while (*fname == '/') + fname++; + } + + while (*fname == '.' && fname[1] == '/') fname += 2; - umask(orig_umask); - p = fname; - while ((p = strchr(p,'/')) != NULL) { - *p = '\0'; - if (do_mkdir(fname, ACCESSPERMS) < 0 && errno != EEXIST) - ret = -1; - *p++ = '/'; + if (flags & MKP_DROP_NAME) { + end = strrchr(fname, '/'); + if (!end) + return 0; + *end = '\0'; + } else + end = fname + strlen(fname); + + umask(orig_umask); /* NOTE: don't return before setting this back to 0! */ + + /* Try to find an existing dir, starting from the deepest dir. */ + for (p = end; ; ) { + if (do_mkdir(fname, ACCESSPERMS) == 0) { + ret++; + break; + } + if (errno != ENOENT) { + if (errno != EEXIST) + ret = -ret - 1; + break; + } + while (1) { + if (p == fname) { + ret = -ret - 1; + goto double_break; + } + if (*--p == '/') { + if (p == fname) { + ret = -ret - 1; /* impossible... */ + goto double_break; + } + *p = '\0'; + break; + } + } + } + double_break: + + /* Make all the dirs that we didn't find on the way here. */ + while (p != end) { + *p = '/'; + p += strlen(p); + if (ret < 0) /* Skip mkdir on error, but keep restoring the path. */ + continue; + if (do_mkdir(fname, ACCESSPERMS) < 0) + ret = -ret - 1; + else + ret++; } + umask(0); + if (flags & MKP_DROP_NAME) + *end = '/'; + return ret; } @@ -270,8 +319,7 @@ static int safe_read(int desc, char *ptr, size_t len) * * This is used in conjunction with the --temp-dir, --backup, and * --copy-dest options. */ -int copy_file(const char *source, const char *dest, int ofd, - mode_t mode, int create_bak_dir) +int copy_file(const char *source, const char *dest, int ofd, mode_t mode) { int ifd; char buf[1024 * 8]; @@ -293,19 +341,11 @@ int copy_file(const char *source, const char *dest, int ofd, } if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) { - int save_errno = errno ? errno : EINVAL; /* 0 paranoia */ - if (create_bak_dir && errno == ENOENT && make_bak_dir(dest) == 0) { - if ((ofd = do_open(dest, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode)) < 0) - save_errno = errno ? errno : save_errno; - else - save_errno = 0; - } - if (save_errno) { - rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest)); - close(ifd); - errno = save_errno; - return -1; - } + int save_errno = errno; + rsyserr(FERROR_XFER, save_errno, "open %s", full_fname(dest)); + close(ifd); + errno = save_errno; + return -1; } } @@ -440,7 +480,7 @@ int robust_rename(const char *from, const char *to, const char *partialptr, return -2; to = partialptr; } - if (copy_file(from, to, -1, mode, 0) != 0) + if (copy_file(from, to, -1, mode) != 0) return -2; do_unlink(from); return 1; -- 2.34.1