On Thu, Jun 06, 2019 at 12:37:13AM +0200, Christian Weisgerber wrote:
> 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.
> >
Hi,
I noticed the realloc issue also and wanted to send a separate patch later for
it. Nice it is addressed also :)
> > 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) {
Also could the first malloc and realloc maybe get simplified to
reallocarray(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
>
--
Kind regards,
Hiltjo
