svn commit: r342699 - head/sbin/savecore

[Alan Somers]

On Sun, Jun 28, 2020 at 6:55 PM Mark Johnston <markj at freebsd.org> wrote:

> 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 at 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.
>

Using that new valgrind-devel did not find the leaks.  However, it _did_
print a helpful error message explaining why Capsicum interferes.
Commenting out caph_enter_casper allowed valgrind to find the leaks.  It
also found two more than I didn't know about.  Thanks for the tip.

https://www.valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.clientreq

-Alan