Bug 203647 - makefs: Coverity CID 978431: No free() after malloc().
Summary: makefs: Coverity CID 978431: No free() after malloc().
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Enji Cooper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-08 19:04 UTC by scdbackup
Modified: 2017-01-02 03:36 UTC (History)
2 users (show)

See Also:
ngie: mfc-stable10+
ngie: mfc-stable9+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description scdbackup 2015-10-08 19:04:28 UTC
usr.sbin/makefs/cd9660/cd9660_write.c

CID 978431: Resource leak (RESOURCE_LEAK)
   9. leaked_storage: Variable buffer going out of scope leaks the
   storage it points to.

216        return cd9660_write_filedata(fd, sector, buffer_head,
217            path_table_sectors);

--------------- Source analysis:

There are two return statements in the function.
The first one (return 0) is ok, because malloc() returned NULL.
The second one uses the allocated buffer and does not free it.

--------------- Remedy proposal:

Untangle the return statement.

-        return cd9660_write_filedata(fd, sector, buffer_head,
+        ret = cd9660_write_filedata(fd, sector, buffer_head,
              path_table_sectors);
+        free(buffer);
+        return ret;
Comment 1 Ed Maste freebsd_committer freebsd_triage 2015-10-19 20:10:41 UTC
We need to check if this is fixed in NetBSD's makefs already, and pass the fix there too if not.
Comment 2 scdbackup 2015-10-19 21:01:31 UTC
It looks like it is still in lines 214+215.
The variable buffer_head (=buffer) is used in the return statement
and thus cannot be freed before the context gets dropped.

http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.sbin/makefs/cd9660/cd9660_write.c?annotate=1.15.6.2
Comment 3 commit-hook freebsd_committer freebsd_triage 2015-10-21 11:39:50 UTC
A commit references this bug:

Author: ngie
Date: Wed Oct 21 11:38:48 UTC 2015
New revision: 289687
URL: https://svnweb.freebsd.org/changeset/base/289687

Log:
  Free buffer before returning from cd9660_write_path_table to avoid
  leaking it after returning from the function

  MFC after: 1 week
  PR: 203647
  Submitted by: Thomas Schmitt <scdbackup@gmx.net>
  Coverity CID: 978431
  Sponsored by: EMC / Isilon Storage Division

Changes:
  head/usr.sbin/makefs/cd9660/cd9660_write.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2015-10-21 12:54:55 UTC
A commit references this bug:

Author: ngie
Date: Wed Oct 21 12:54:16 UTC 2015
New revision: 289693
URL: https://svnweb.freebsd.org/changeset/base/289693

Log:
  Unbreak makefs -t cd9660 after r289687

  buffer_head needs to be freed -- not buffer

  Detected by jemalloc, i.e. running makefs failed the arena assert
  because my copy of malloc on CURRENT is compiled with the default
  !MALLOC_PRODUCTION asserts on

  Pointyhat to: ngie
  PR: 203647
  X-MFC with: r289687
  Sponsored by: EMC / Isilon Storage Division

Changes:
  head/usr.sbin/makefs/cd9660/cd9660_write.c
Comment 5 scdbackup 2015-10-21 14:02:24 UTC
Oops. Sorry for not checking carefully enough. Congrats to your malloc settings.
Comment 6 commit-hook freebsd_committer freebsd_triage 2015-11-09 08:03:58 UTC
A commit references this bug:

Author: ngie
Date: Mon Nov  9 08:03:15 UTC 2015
New revision: 290587
URL: https://svnweb.freebsd.org/changeset/base/290587

Log:
  MFC r289687,r289693:

  r289687:

  Free buffer before returning from cd9660_write_path_table to avoid
  leaking it after returning from the function

  PR: 203647
  Submitted by: Thomas Schmitt <scdbackup@gmx.net>
  Coverity CID: 978431
  Sponsored by: EMC / Isilon Storage Division

  r289693:

  Unbreak makefs -t cd9660 after r289687

  buffer_head needs to be freed -- not buffer

  Detected by jemalloc, i.e. running makefs failed the arena assert
  because my copy of malloc on CURRENT is compiled with the default
  !MALLOC_PRODUCTION asserts on

  Pointyhat to: ngie
  PR: 203647
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/10/
  stable/10/usr.sbin/makefs/cd9660/cd9660_write.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2015-11-09 08:03:59 UTC
A commit references this bug:

Author: ngie
Date: Mon Nov  9 08:03:15 UTC 2015
New revision: 290587
URL: https://svnweb.freebsd.org/changeset/base/290587

Log:
  MFC r289687,r289693:

  r289687:

  Free buffer before returning from cd9660_write_path_table to avoid
  leaking it after returning from the function

  PR: 203647
  Submitted by: Thomas Schmitt <scdbackup@gmx.net>
  Coverity CID: 978431
  Sponsored by: EMC / Isilon Storage Division

  r289693:

  Unbreak makefs -t cd9660 after r289687

  buffer_head needs to be freed -- not buffer

  Detected by jemalloc, i.e. running makefs failed the arena assert
  because my copy of malloc on CURRENT is compiled with the default
  !MALLOC_PRODUCTION asserts on

  Pointyhat to: ngie
  PR: 203647
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/10/
  stable/10/usr.sbin/makefs/cd9660/cd9660_write.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2015-11-09 08:06:01 UTC
A commit references this bug:

Author: ngie
Date: Mon Nov  9 08:05:15 UTC 2015
New revision: 290588
URL: https://svnweb.freebsd.org/changeset/base/290588

Log:
  MFstable/10 r290587:

  MFC r289687,r289693:

  r289687:

  Free buffer before returning from cd9660_write_path_table to avoid
  leaking it after returning from the function

  PR: 203647
  Submitted by: Thomas Schmitt <scdbackup@gmx.net>
  Coverity CID: 978431
  Sponsored by: EMC / Isilon Storage Division

  r289693:

  Unbreak makefs -t cd9660 after r289687

  buffer_head needs to be freed -- not buffer

  Detected by jemalloc, i.e. running makefs failed the arena assert
  because my copy of malloc on CURRENT is compiled with the default
  !MALLOC_PRODUCTION asserts on

  Pointyhat to: ngie
  PR: 203647
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/9/
_U  stable/9/usr.sbin/
_U  stable/9/usr.sbin/makefs/
  stable/9/usr.sbin/makefs/cd9660/cd9660_write.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-11-09 08:06:02 UTC
A commit references this bug:

Author: ngie
Date: Mon Nov  9 08:05:15 UTC 2015
New revision: 290588
URL: https://svnweb.freebsd.org/changeset/base/290588

Log:
  MFstable/10 r290587:

  MFC r289687,r289693:

  r289687:

  Free buffer before returning from cd9660_write_path_table to avoid
  leaking it after returning from the function

  PR: 203647
  Submitted by: Thomas Schmitt <scdbackup@gmx.net>
  Coverity CID: 978431
  Sponsored by: EMC / Isilon Storage Division

  r289693:

  Unbreak makefs -t cd9660 after r289687

  buffer_head needs to be freed -- not buffer

  Detected by jemalloc, i.e. running makefs failed the arena assert
  because my copy of malloc on CURRENT is compiled with the default
  !MALLOC_PRODUCTION asserts on

  Pointyhat to: ngie
  PR: 203647
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/9/
_U  stable/9/usr.sbin/
_U  stable/9/usr.sbin/makefs/
  stable/9/usr.sbin/makefs/cd9660/cd9660_write.c