Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 28 Jun 2020 20:55:38 -0400
From:      Mark Johnston <markj@freebsd.org>
To:        Alan Somers <asomers@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r342699 - head/sbin/savecore
Message-ID:  <20200629005538.GB7202@raichu>
In-Reply-To: <CAOtMX2jJfCdiVAuVPL_9QmUR3EhwjC-DpAk5r=J2=tK9WQKh6w@mail.gmail.com>
References:  <201901021709.x02H9ZPM004185@repo.freebsd.org> <CAOtMX2jJfCdiVAuVPL_9QmUR3EhwjC-DpAk5r=J2=tK9WQKh6w@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jun 28, 2020 at 06:40:59PM -0600, Alan Somers wrote:
> On Wed, Jan 2, 2019 at 10:09 AM Mark Johnston <markj@freebsd.org> wrote:
> 
> > Author: markj
> > Date: Wed Jan  2 17:09:35 2019
> > New Revision: 342699
> > URL: https://svnweb.freebsd.org/changeset/base/342699
> >
> > Log:
> >   Capsicumize savecore(8).
> >
> >   - Use cap_fileargs(3) to open dump devices after entering capability
> >     mode, and use cap_syslog(3) to log messages.
> >   - Use a relative directory fd to open output files.
> >   - Use zdopen(3) to compress kernel dumps in capability mode.
> >
> >   Reviewed by:  cem, oshogbo
> >   MFC after:    2 months
> >   Sponsored by: The FreeBSD Foundation
> >   Differential Revision:        https://reviews.freebsd.org/D18458
> >
> > Modified:
> >   head/sbin/savecore/Makefile
> >   head/sbin/savecore/savecore.c
> >
> > Modified: head/sbin/savecore/savecore.c
> >
> > ==============================================================================
> > --- head/sbin/savecore/savecore.c       Wed Jan  2 16:42:07 2019
> > (r342698)
> > +++ head/sbin/savecore/savecore.c       Wed Jan  2 17:09:35 2019
> > (r342699)
> >
> > +static char **
> > +enum_dumpdevs(int *argcp)
> > +{
> > +       struct fstab *fsp;
> > +       char **argv;
> > +       int argc, n;
> > +
> > +       /*
> > +        * We cannot use getfsent(3) in capability mode, so we must
> > +        * scan /etc/fstab and build up a list of candidate devices
> > +        * before proceeding.
> > +        */
> > +       argc = 0;
> > +       n = 8;
> > +       argv = malloc(n * sizeof(*argv));
> >
> 
> It looks like the memory allocated here
> 
> 
> > +       if (argv == NULL) {
> > +               logmsg(LOG_ERR, "malloc(): %m");
> > +               exit(1);
> > +       }
> > +       for (;;) {
> > +               fsp = getfsent();
> > +               if (fsp == NULL)
> > +                       break;
> > +               if (strcmp(fsp->fs_vfstype, "swap") != 0 &&
> > +                   strcmp(fsp->fs_vfstype, "dump") != 0)
> > +                       continue;
> > +               if (argc >= n) {
> > +                       n *= 2;
> > +                       argv = realloc(argv, n * sizeof(*argv));
> >
> 
> and here
> 
> 
> > +                       if (argv == NULL) {
> > +                               logmsg(LOG_ERR, "realloc(): %m");
> > +                               exit(1);
> > +                       }
> > +               }
> > +               argv[argc] = strdup(fsp->fs_spec);
> >
> 
> and here is leaked.  I can't find any corresponding free.  However, neither
> Valgrind nor Coverity complains.  What am I missing?  Does this memory
> sneakily get freed by a subroutine somewhere, or does Capsicum confuse our
> tools?

I'm not sure why Capsicum would change anything.  It would be worth
testing devel/valgrind-devel with https://reviews.freebsd.org/D25452
applied, to see if it's able to detect that bug.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200629005538.GB7202>