Claudio Jeker([email protected]) on 2021.05.06 17:59:32 +0200:
> 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.
reads ok
>
> --
> :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;
> }
>
>
