Bug 192671 - [cam] cam_close_device/cam_close_spec_device can trash valid file descriptors if called more than once on a device
Summary: [cam] cam_close_device/cam_close_spec_device can trash valid file descriptors...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Enji Cooper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-15 04:26 UTC by Enji Cooper
Modified: 2015-10-26 00:10 UTC (History)
0 users

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 Enji Cooper freebsd_committer 2014-08-15 04:26:46 UTC
cam_close_spec_device doesn't modify dev->fd after calling cam_close_spec_device, so if cam_close_spec_device is called multiple times on a dev object, it can trash valid file descriptors associated with cam or other pieces of code. See http://svnweb.freebsd.org/base/head/lib/libcam/camlib.c?annotate=257388#l680 for more details.

Reported by: Scott Ferris <sferris@isilon.com>
Comment 1 commit-hook freebsd_committer 2015-10-17 09:08:53 UTC
A commit references this bug:

Author: ngie
Date: Sat Oct 17 09:07:54 UTC 2015
New revision: 289450
URL: https://svnweb.freebsd.org/changeset/base/289450

Log:
  Set dev->fd to -1 when calling cam_close_spec_device with a valid dev->fd
  descriptor to avoid trashing valid file descriptors that access dev->fd at a
  later point in time

  PR: 192671
  Submitted by: Scott Ferris <scott.ferris@isilon.com>
  MFC after: 1 week
  Sponsored by: EMC / Isilon Storage Division

Changes:
  head/lib/libcam/camlib.c
Comment 2 Enji Cooper freebsd_committer 2015-10-17 09:14:25 UTC
(In reply to commit-hook from comment #1)

As I replied to the commit email, the commit description I wrote up isn't helpful.

On Oct 17, 2015, at 02:07, Garrett Cooper <ngie@FreeBSD.org> wrote:

Author: ngie
Date: Sat Oct 17 09:07:53 2015
New Revision: 289450
URL: https://svnweb.freebsd.org/changeset/base/289450

Log:
Set dev->fd to -1 when calling cam_close_spec_device with a valid dev->fd
descriptor to avoid trashing valid file descriptors that access dev->fd at a
later point in time

That wasn’t a good description — sorry :(.

The cam_close_spec_device API doesn’t free `struct cam_device *dev` by design, so something could reuse the structure, or call close on dev->fd (out of band), which could trash valid file descriptor(s) by accident.

Thanks!
-NGie
Comment 3 commit-hook freebsd_committer 2015-10-26 00:08:48 UTC
A commit references this bug:

Author: ngie
Date: Mon Oct 26 00:08:41 UTC 2015
New revision: 289972
URL: https://svnweb.freebsd.org/changeset/base/289972

Log:
  MFC r289450:

  Set dev->fd to -1 when calling cam_close_spec_device with a valid dev->fd
  descriptor to avoid trashing valid file descriptors that access dev->fd at a
  later point in time

  PR: 192671
  Submitted by: Scott Ferris <scott.ferris@isilon.com>
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/10/
  stable/10/lib/libcam/camlib.c
Comment 4 commit-hook freebsd_committer 2015-10-26 00:09:49 UTC
A commit references this bug:

Author: ngie
Date: Mon Oct 26 00:09:31 UTC 2015
New revision: 289973
URL: https://svnweb.freebsd.org/changeset/base/289973

Log:
  MFstable/10 r289972:

  MFC r289450:

  Set dev->fd to -1 when calling cam_close_spec_device with a valid dev->fd
  descriptor to avoid trashing valid file descriptors that access dev->fd at a
  later point in time

  PR: 192671
  Submitted by: Scott Ferris <scott.ferris@isilon.com>
  Sponsored by: EMC / Isilon Storage Division

Changes:
_U  stable/9/
_U  stable/9/lib/
_U  stable/9/lib/libcam/
  stable/9/lib/libcam/camlib.c