Christian Weisgerber:

> > To reproduce:
> > 
> >     mkdir -p /tmp/test /tmp/plop
> >     openrsync -rx /tmp/test/ /tmp/plop/
> > 
> > Result:
> > 
> >     openrsync(3470) in free(): bogus pointer (double free?) 0x7f7ffffdcdc8
> >     Abort trap (core dumped)
> > 
> > The patch below fixes it and simplifies the logic:
> 
> I agree.  However, the (re)allocation of xdev also looks bogus.
> 
> How about this?
> 
> (Also, the realloc idiom is exactly the one the man page warns
> against.  Do we care here?)

Well, let's not set a bad example.

Index: flist.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/flist.c,v
retrieving revision 1.27
diff -u -p -r1.27 flist.c
--- flist.c     2 Jun 2019 14:29:58 -0000       1.27
+++ flist.c     5 Jun 2019 22:36:28 -0000
@@ -808,7 +808,7 @@ flist_gen_dirent(struct sess *sess, char
        FTSENT          *ent;
        struct flist    *f;
        size_t           flsz = 0, stripdir;
-       dev_t           *xdev;
+       dev_t           *newxdev, *xdev = NULL;
        struct stat      st;
 
        cargv[0] = root;
@@ -931,7 +931,8 @@ flist_gen_dirent(struct sess *sess, char
                            !S_ISDIR(ent->fts_statp->st_mode))
                                continue;
 
-                       if ((xdev = malloc(sizeof(dev_t))) == NULL) {
+                       if (xdev == NULL &&
+                           (xdev = malloc(sizeof(dev_t))) == NULL) {
                                ERRX1("malloc");
                                goto out;
                        }
@@ -945,12 +946,14 @@ flist_gen_dirent(struct sess *sess, char
                        if (flag)
                                continue;
 
-                       if (nxdev)
-                               if ((xdev = realloc(xdev, sizeof(dev_t))) ==
-                                   NULL) {
+                       if (nxdev) {
+                               if ((newxdev = reallocarray(xdev, nxdev + 1,
+                                   sizeof(dev_t))) == NULL) {
                                        ERRX1("realloc");
                                        goto out;
                                }
+                               xdev = newxdev;
+                       }
                        xdev[nxdev] = ent->fts_statp->st_dev;
                        nxdev++;
                }
@@ -1008,8 +1011,7 @@ flist_gen_dirent(struct sess *sess, char
        rc = 1;
 out:
        fts_close(fts);
-       if (sess->opts->one_file_system)
-               free(xdev);
+       free(xdev);
        return rc;
 }
 
-- 
Christian "naddy" Weisgerber                          na...@mips.inka.de

Reply via email to