]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
generator: don't read an unstat'd sx.st when creating a device/special
authorAndrew Tridgell <andrew@tridgell.net>
Sat, 13 Jun 2026 20:44:21 +0000 (06:44 +1000)
committerAndrew Tridgell <andrew@tridgell.net>
Sat, 13 Jun 2026 22:33:03 +0000 (08:33 +1000)
this fixes a valgrind error where we could read an uninitialised sx.st
field when we don't fill the stat data.

Also drop the now-obsolete testsuite/valgrind.supp stanzas for these
reads (atomic_create/delete_item, plus the rwrite strlcpy over-read that
master already fixed) -- they are no longer needed now the reads are gone.

Thanks to report from Michael Mess <michael@michaelmess.de>

generator.c
testsuite/valgrind.supp

index 09e276d1fbd7820f20f3da66b37ca1d4877471e7..107c925a35832384eb8086084fb3295be75a1ac3 100644 (file)
@@ -1230,7 +1230,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
        static int need_fuzzy_dirlist = 0;
        struct file_struct *fuzzy_file = NULL;
        int fd = -1, f_copy = -1;
-       stat_x sx, real_sx;
+       stat_x sx = {0}, real_sx;
        STRUCT_STAT partial_st;
        struct file_struct *back_file = NULL;
        int statret, real_ret, stat_errno;
@@ -1628,6 +1628,10 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
         || (preserve_specials && ftype == FT_SPECIAL)) {
                dev_t rdev;
                int del_for_flag;
+               /* Whether the dest existed, captured before the type-mismatch
+                * flip below clears statret -- so atomic_create() gets a delete
+                * flag (and reads sx.st) only when sx.st was actually stat'd. */
+               int dest_existed = (statret == 0);
                if (ftype == FT_DEVICE) {
                        uint32 *devp = F_RDEV_P(file);
                        rdev = MAKEDEV(DEV_MAJOR(devp), DEV_MINOR(devp));
@@ -1674,7 +1678,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
                                fname, (int)file->mode,
                                (long)major(rdev), (long)minor(rdev));
                }
-               if (atomic_create(file, fname, NULL, NULL, rdev, &sx, del_for_flag)) {
+               if (atomic_create(file, fname, NULL, NULL, rdev, &sx, dest_existed ? del_for_flag : 0)) {
                        set_file_attrs(fname, file, NULL, NULL, 0);
                        if (itemizing) {
                                itemize(fnamecmp, file, ndx, statret, &sx,
index b0cf91eace1afcdf3483b82781d695657dfbd8dc..ddaefd086a72f15a9f5ce3d583c5273e421c73b5 100644 (file)
    fun:main
 }
 
-# rwrite() strlcpy()s a peer log message.  read_a_msg() hands it exactly
-# msg_bytes valid bytes with no terminating NUL; the copy is bounded to len,
-# but strlcpy over-reads past the message to compute its (discarded) return
-# length.  Harmless read of one+ uninitialised stack byte.
-{
-   rsync-rwrite-strlcpy-peer-msg
-   Memcheck:Cond
-   fun:strlcpy
-   fun:strlcpy
-   fun:rwrite
-}
-
-# atomic_create()/delete_item() test sxp->st.st_mode when replacing an
-# existing item.  Under fakeroot the faked stat overlay leaves mode bits that
-# valgrind treats as uninitialised.  Pre-existing hard-link code; fakeroot
-# artifact, not an rsync read of uninitialised memory.
-{
-   rsync-atomic-create-stmode
-   Memcheck:Cond
-   fun:atomic_create
-   fun:recv_generator
-}
-{
-   rsync-delete-item-stmode
-   Memcheck:Cond
-   fun:delete_item
-   fun:atomic_create
-}
-
 # libxxhash returns an internally-aligned XXH3 state whose pointer is offset
 # from the malloc base, so valgrind sees only an interior pointer and reports
 # the one-time checksum state as "possibly lost".  Alignment artifact.