8ce99bb405
fget_cap() tries to do a cheaper snapshot of a file descriptor without holding the file descriptor lock. This snapshot does not do a deep copy of the ioctls capability array, but instead uses a different return value to inform the caller to retry the copy with the lock held. However, filecaps_copy() was returning 1 to indicate that a retry was required, and fget_cap() was checking for 0 (actually '!filecaps_copy()'). As a result, fget_cap() did not do a deep copy of the ioctls array and just reused the original pointer. This cause multiple file descriptor entries to think they owned the same pointer and eventually resulted in duplicate frees. The only code path that I'm aware of that triggers this is to create a listen socket that has a restricted list of ioctls and then call accept() which calls fget_cap() with a valid filecaps structure from getsock_cap(). To fix, change the return value of filecaps_copy() to return true if it succeeds in copying the caps and false if it fails because the lock is required. I find this more intuitive than fixing the caller in this case. While here, change the return type from 'int' to 'bool'. Finally, make filecaps_copy() more robust in the failure case by not copying any of the source filecaps structure over. This avoids the possibility of leaking a pointer into a structure if a similar future caller doesn't properly handle the return value from filecaps_copy() at the expense of one more branch. I also added a test case that panics before this change and now passes. Reviewed by: kib Discussed with: mjg (not a fan of the extra branch) MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D15047 |
||
|---|---|---|
| .. | ||
| BSD.debug.dist | ||
| BSD.include.dist | ||
| BSD.lib32.dist | ||
| BSD.libsoft.dist | ||
| BSD.release.dist | ||
| BSD.root.dist | ||
| BSD.sendmail.dist | ||
| BSD.tests.dist | ||
| BSD.usr.dist | ||
| BSD.var.dist | ||
| Makefile | ||
| README | ||
$FreeBSD$
Note: If you modify these files, please keep hier(7) updated!
These files are used to create empty file hierarchies for building the
system into. Some notes about working with them are placed here to try
and keep them in good working order.
a) The files use 4 space indentation, and other than in the header
comments, should not contain any tabs. An indentation of 4 is
preferable to the standard indentation of 8 because the indentation
of levels in these files can become quite deep causing the line to
overflow 80 characters.
This also matches with the files generated when using the
mtree -c option, which was implemented that way for the same reason.
b) Only directories should be listed here.
c) The listing should be kept in filename sorted order.
d) Sanity checking changes to these files can be done by following
this procedure (the sed -e is ugly, but fixing mtree -c to
not emit the trailing white space would be even uglier):
mkdir /tmp/MTREE
mtree -deU -f BSD.X.dist -p /tmp/MTREE
mtree -cdin -k uname,gname,mode -p /tmp/MTREE | \
sed -e 's/ *$//' >BSD.X.new
diff -u BSD.X.dist BSD.X.new
rm -r /tmp/MTREE
Note that you will get some differences about /set lines,
and uname= gname= on certain directory areas, mainly man page
sections. This is caused by mtree not having a look ahead
mechanism for making better selections for these as it
traverses the hierarchy.
The BSD.X.new file should NOT be committed, as it will be missing
the correct header, and important keywords like ``nochange''.
Simply use the diff for a sanity check to make sure things are in
the correct order and correctly indented.
e) Further sanity checking of the system builds with DESTDIR=/someplace
are more complicated, but can often catch missing entries in these
files. I tend to run this more complete sanity check shortly after
the target date for a new release is announced.
If you want details on it bug me about it via email to
rgrimes@FreeBSD.org.