svn commit: r342699 - head/sbin/savecore

Alan Somers asomers at freebsd.org
Mon Jun 29 00:49:50 UTC 2020


On Sun, Jun 28, 2020 at 6:46 PM Warner Losh <imp at bsdimp.com> wrote:

>
>
> On Sun, Jun 28, 2020, 6:41 PM Alan Somers <asomers at freebsd.org> 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?
>>
>
> So the other spots adjusted large, but this one sets one of its elements.
> Help me understand how that is a leak? I'm sure I'm just confused.
>

Because strdup itself allocates new memory.  strdup's return value is
always supposed to be freed.
-Alan


More information about the svn-src-all mailing list