As noticed by benno@ the blk.blks buffer is leaked in some cases.
Fix those and cleanup up the pre_* functions a bit more.
I increased the diff context a bit to make the diff easier to read.
--
:wq Claudio
Index: uploader.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
retrieving revision 1.25
diff -u -p -U6 -r1.25 uploader.c
--- uploader.c 6 May 2021 07:35:22 -0000 1.25
+++ uploader.c 6 May 2021 15:34:36 -0000
@@ -191,22 +191,24 @@ pre_link(struct upload *p, struct sess *
* to overwriting with a symbolic link.
* If it's a non-directory, we just overwrite it.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
+
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISLNK(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
/*
* If the symbolic link already exists, then make sure that it
* points to the correct place.
*/
@@ -294,22 +296,23 @@ pre_dev(struct upload *p, struct sess *s
* If it replaces a directory, remove the directory first.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !(S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode))) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
/* Make sure existing device is of the correct type. */
if (rc != -1) {
if ((f->st.mode & (S_IFCHR|S_IFBLK)) !=
@@ -382,22 +385,23 @@ pre_fifo(struct upload *p, struct sess *
* mark it from replacement.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISFIFO(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
if (rc == -1) {
newfifo = 1;
if (mktemplate(&temp, f->path, sess->opts->recursive) == -1) {
ERRX1("mktemplate");
@@ -458,22 +462,23 @@ pre_sock(struct upload *p, struct sess *
* mark it from replacement.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISSOCK(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
if (rc == -1) {
newsock = 1;
if (mktemplate(&temp, f->path, sess->opts->recursive) == -1) {
ERRX1("mktemplate");
@@ -530,13 +535,14 @@ pre_dir(const struct upload *p, struct s
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
if (rc == -1 && errno != ENOENT) {
ERR("%s: fstatat", f->path);
return -1;
- } else if (rc != -1 && !S_ISDIR(st.st_mode)) {
+ }
+ if (rc != -1 && !S_ISDIR(st.st_mode)) {
ERRX("%s: not a directory", f->path);
return -1;
} else if (rc != -1) {
/*
* FIXME: we should fchmod the permissions here as well,
* as we may locally have shut down writing into the
@@ -682,19 +688,21 @@ pre_file(const struct upload *p, int *fi
if (fstat(*filefd, st) == -1) {
ERR("%s: fstat", f->path);
close(*filefd);
*filefd = -1;
return -1;
- } else if (!S_ISREG(st->st_mode)) {
+ }
+ if (!S_ISREG(st->st_mode)) {
ERRX("%s: not regular", f->path);
close(*filefd);
*filefd = -1;
return -1;
}
+ /* quick check if file is the same */
if (st->st_size == f->st.size &&
st->st_mtime == f->st.mtime) {
LOG3("%s: skipping: up to date", f->path);
if (!rsync_set_metadata(sess, 0, *filefd, f, f->path)) {
ERRX1("rsync_set_metadata");
close(*filefd);
@@ -910,24 +918,26 @@ rsync_uploader(struct upload *u, int *fi
}
if ((mbuf = malloc(blk.len)) == NULL) {
ERR("malloc");
close(*fileinfd);
*fileinfd = -1;
+ free(blk.blks);
return -1;
}
offs = 0;
i = 0;
do {
msz = pread(*fileinfd, mbuf, blk.len, offs);
if ((size_t)msz != blk.len && (size_t)msz != blk.rem) {
ERR("pread");
close(*fileinfd);
*fileinfd = -1;
free(mbuf);
+ free(blk.blks);
return -1;
}
init_blk(&blk.blks[i], &blk, offs, i, mbuf, sess);
offs += blk.len;
LOG3(
"i=%ld, offs=%lld, msz=%ld, blk.len=%lu,
blk.rem=%lu",
@@ -964,12 +974,13 @@ rsync_uploader(struct upload *u, int *fi
(sizeof(int32_t) + /* short checksum */
blk.csum); /* long checksum */
if (u->bufsz > u->bufmax) {
if ((bufp = realloc(u->buf, u->bufsz)) == NULL) {
ERR("realloc");
+ free(blk.blks);
return -1;
}
u->buf = bufp;
u->bufmax = u->bufsz;
}
