[patch] openrsync blk_flush() fd leak in blk_match()

[Hiltjo Posthuma]

Hi,

I think the fd can leak when blk_flush() fails in blocks.c blk_match(), because
ERR and ERRX1 does not to terminate. Am I correct?

I also (hopefully) simplified the logic a bit (set nfd = -1 and goto out).

Thanks for working on (open)rsync!

Patch below:


diff --git a/usr.bin/rsync/blocks.c b/usr.bin/rsync/blocks.c
index 6f1d8d2fb6f..984e048a730 100644
--- a/usr.bin/rsync/blocks.c
+++ b/usr.bin/rsync/blocks.c
@@ -238,7 +238,7 @@ int
 blk_match(struct sess *sess, int fd,
        const struct blkset *blks, const char *path)
 {
-       int              nfd, rc = 0, c;
+       int              nfd = -1, rc = 0, c;
        struct stat      st;
        void            *map = MAP_FAILED;
        size_t           mapsz;
@@ -248,11 +248,10 @@ blk_match(struct sess *sess, int fd,
 
        if (-1 == (nfd = open(path, O_RDONLY, 0))) {
                ERR(sess, "%s: open", path);
-               return 0;
+               goto out;
        } else if (-1 == fstat(nfd, &st)) {
                ERR(sess, "%s: fstat", path);
-               close(nfd);
-               return 0;
+               goto out;
        }
 
        /*
@@ -264,8 +263,7 @@ blk_match(struct sess *sess, int fd,
                map = mmap(NULL, mapsz, PROT_READ, MAP_SHARED, nfd, 0);
                if (MAP_FAILED == map) {
                        ERR(sess, "%s: mmap", path);
-                       close(nfd);
-                       return 0;
+                       goto out;
                }
        }
 
@@ -287,7 +285,7 @@ blk_match(struct sess *sess, int fd,
        } else {
                if ( ! blk_flush(sess, fd, map, st.st_size, 0)) {
                        ERRX1(sess, "blk_flush");
-                       return 0;
+                       goto out;
                }
                LOG3(sess, "%s: flushed (un-chunked) %jd B, 100%% "
                        "upload ratio", path, (intmax_t)st.st_size);
@@ -310,7 +308,8 @@ blk_match(struct sess *sess, int fd,
 out:
        if (MAP_FAILED != map)
                munmap(map, mapsz);
-       close(nfd);
+       if (-1 != nfd)
+               close(nfd);
        return rc;
 }
 

-- 
Kind regards,
Hiltjo