pipes get stated all thet time and this avoidably contributed to contention.
The pipe lock is only held to accomodate MAC and to check the type.
Since normally there is no probe for pipe stat depessimize this by having the
flag.
The pipe_state field gets modified with locks held all the time and it's not
feasible to convert them to use atomic store. Move the type flag away to a
separate variable as a simple cleanup and to provide stable field to read.
Use short for both fields to avoid growing the struct.
While here short-circuit MAC for pipe_poll as well.
I have such a custom kernel configuration and its build failed with:
linking kernel.full
ld: error: undefined symbol: mac_vnode_assert_locked
>>> referenced by mac_framework.h:556 (/usr/devel/git/apu2c4/sys/security/mac/mac_framework.h:556)
>>> tmpfs_vnops.o:(mac_vnode_check_stat)
>>> referenced by mac_framework.h:556 (/usr/devel/git/apu2c4/sys/security/mac/mac_framework.h:556)
>>> vfs_default.o:(mac_vnode_check_stat)
>>> referenced by mac_framework.h:556 (/usr/devel/git/apu2c4/sys/security/mac/mac_framework.h:556)
>>> ufs_vnops.o:(mac_vnode_check_stat)
The current scheme of calling VOP_GETATTR adds avoidable overhead.
An example with tmpfs doing fstat (ops/s):
before: 7488958
after: 7913833
Reviewed by: kib (previous version)
Differential Revision: https://reviews.freebsd.org/D25910
The code would unconditionally lock the vnode to audit or call the
mac hoook, even if neither want to do anything. Pre-check the state
to avoid locking in the common case of nothing to do.
Note this code should not be normally executed anyway as vnodes are
always return ready. However, poll1/2 from will-it-scale use regular
files for benchmarking, presumably to focus on the interface itself
as the vnode handler is not supposed to do almost anything.
This in particular fixes poll2 which passes 128 fds.
$ ./poll2_processes -s 10
before: 134411
after: 271572
Comparing fsid_t objects requires internal knowledge of the fsid structure
and yet this is duplicated across a number of places in the code.
Simplify by creating a fsidcmp function (macro).
Reviewed by: mjg, rmacklem
Approved by: mav (mentor)
MFC after: 1 week
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D24749
- thr_kill(2) and thr_exit(2) generally (no argument auditing here.
- A set of syscalls for the process descriptor family, specifically:
pdfork(2), pdgetpid(2) and pdkill(2)
For these syscalls, audit the file descriptor. In the case of pdfork(2)
a pointer to an integer (file descriptor) is passed in as an argument.
We audit the post initialized file descriptor (not the random garbage
that would have been passed in). We will also audit the child process
which was created from the fork operation (similar to what is done for
the fork(2) syscall).
pdkill(2) we audit the signal value and fd, and finally pdgetpid(2)
just the file descriptor:
- Following is a sample of the produced audit trails:
header,111,11,pdfork(2),0,Sat May 16 03:07:50 2020, + 394 msec
argument,0,0x39d,child PID
argument,2,0x2,flags
argument,1,0x8,fd
subject,root,root,0,root,0,924,0,0,0.0.0.0
return,success,925
header,79,11,pdgetpid(2),0,Sat May 16 03:07:50 2020, + 394 msec
argument,1,0x8,fd
subject,root,root,0,root,0,924,0,0,0.0.0.0
return,success,0
trailer,79
header,135,11,pdkill(2),0,Sat May 16 03:07:50 2020, + 395 msec
argument,1,0x8,fd
argument,2,0xf,signal
process_ex,root,root,0,root,0,925,0,0,0.0.0.0
subject,root,root,0,root,0,924,0,0,0.0.0.0
return,success,0
trailer,135
MFC after: 1 week
cdir may have simply failed to resolve (e.g. fget_cap failure in namei
leading to NULL dp passed to AUDIT_ARG_UPATH*_VP); restore the pre-rS358191
behavior of setting cpath[0] = '\0' and bailing out instead of panicking.
This was found by inadvertently running the libc/c063 tests with auditing
enabled, resulting in a panic.
Reviewed by: mjg (committed version actually his)
Differential Revision: https://reviews.freebsd.org/D24445
This lock was made unnecessary by the addition of mac_policy_rms in r356120.
Reviewed by: mjg, kib
Differential Revision: https://reviews.freebsd.org/D24283
to the path related tokens not being processed. Restore this behavior and
and move AUE_JAIL_SET in this block, as it may conditionally contain a
path token.
Discovered by: kevans
PR: 244537
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D23929
The new structure is copy-on-write. With the assumption that path lookups are
significantly more frequent than chdirs and chrooting this is a win.
This provides stable root and jail root vnodes without the need to reference
them on lookup, which in turn means less work on globally shared structures.
Note this also happens to fix a bug where jail vnode was never referenced,
meaning subsequent access on lookup could run into use-after-free.
Reviewed by: kib
Differential Revision: https://reviews.freebsd.org/D23884
jail_remove(2) and finally setloginclass(2) are not being converted and
committed into userspace. Add the cases for these syscalls and make sure
they are being converted properly.
Reviewed by: bz, kevans
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D23882
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.
This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.
Mark all obvious cases as MPSAFE. All entries that haven't been marked
as MPSAFE before are by default marked as NEEDGIANT
Approved by: kib (mentor, blanket)
Commented by: kib, gallatin, melifaro
Differential Revision: https://reviews.freebsd.org/D23718
realpath(3) is used a lot e.g., by clang and is a major source of getcwd
and fstatat calls. This can be done more efficiently in the kernel.
This works by performing a regular lookup while saving the name and found
parent directory. If the terminal vnode is a directory we can resolve it using
usual means. Otherwise we can use the name saved by lookup and resolve the
parent.
See the review for sample syscall counts.
Reviewed by: kib
Differential Revision: https://reviews.freebsd.org/D23574
This in particular significantly shortens amd64_syscall, which otherwise
keeps jumping forward over 2KB of code in total.
Note some of these branches should be either eliminated altogether or
coalesced.
All checking routines walk a linked list of all modules in order to determine
if given hook is installed. This became a significant problem after mac_ntpd
started being loaded by default.
Implement a way perform checks for select hooks by testing a boolean.
Use it for priv_check and priv_grant, which are constantly called from priv_check.
The real fix would use hotpatching, but the above provides a way to know when
to do it.
There was only one consumer and it was using it incorrectly.
It is given an equivalent hack.
Reviewed by: jeff
Differential Revision: https://reviews.freebsd.org/D23037
Filesystems which want to use it in limited capacity can employ the
VOP_UNLOCK_FLAGS macro.
Reviewed by: kib (previous version)
Differential Revision: https://reviews.freebsd.org/D21427
If any non-static modules are loaded (and mac_ntpd tends to be), the lock is
taken all the time al over the kernel. On platforms like arm64 this results in
an avoidable significant performance degradation. Since write-locking is almost
never needed, use a primitive optimized towards read-locking.
Sample result of building the kernel on tmpfs 11 times:
stock 11142.80s user 6704.44s system 4924% cpu 6:02.42 total
patched 11118.95s user 2374.94s system 4547% cpu 4:56.71 total
entry, when that entry has been seen already, keep the
already-looked-up value in a variable and use that instead of looking
it up again.
Approved by: alc, markj (earlier version), kib (earlier version)
Differential Revision: https://reviews.freebsd.org/D22348
Co-mingling two things here:
* Addressing some feedback from Konstantin and Kyle re: jail,
capability mode, and a few other things
* Adding audit support as promised.
The audit support change includes a partial refresh of OpenBSM from
upstream, where the change to add shm_rename has already been
accepted. Matthew doesn't plan to work on refreshing anything else to
support audit for those new event types.
Submitted by: Matthew Bryan <matthew.bryan@isilon.com>
Reviewed by: kib
Relnotes: Yes
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D22083
around entry->{next,prev} when those are used for ordered list
traversal, and use those wrapper functions everywhere. Where the next
field is used for maintaining a stack of deferred operations, #define
defer_next to make that different usage clearer, and then use the
'right' pointer instead of 'next' for that purpose.
Approved by: markj
Tested by: pho (as part of a larger patch)
Differential Revision: https://reviews.freebsd.org/D22347
In case the implementation ever changes from using a chain of next pointers,
then changing the macro definition will be necessary, but changing all the
files that iterate over vm_map entries will not.
Drop a counter in vm_object.c that would have an effect only if the
vm_map entry count was wrong.
Discussed with: alc
Reviewed by: markj
Tested by: pho (earlier version)
Differential Revision: https://reviews.freebsd.org/D21882
neighbors, and is used in a way so that if entries a and b cannot be
merged, we consider them twice, first not-merging a with its successor
b, and then not-merging b with its predecessor a. This change replaces
vm_map_simplify_entry with vm_map_try_merge_entries, which compares
two adjacent entries only, and uses it to avoid duplicated
merge-checks.
Tested by: pho
Reviewed by: alc
Approved by: markj (implicit)
Differential Revision: https://reviews.freebsd.org/D20814
This allows replacing "sys/eventfilter.h" includes with "sys/_eventfilter.h"
in other header files (e.g., sys/{bus,conf,cpu}.h) and reduces header
pollution substantially.
EVENTHANDLER_DECLARE and EVENTHANDLER_LIST_DECLAREs were moved out of .c
files into appropriate headers (e.g., sys/proc.h, powernv/opal.h).
As a side effect of reduced header pollution, many .c files and headers no
longer contain needed definitions. The remainder of the patch addresses
adding appropriate includes to fix those files.
LOCK_DEBUG and LOCK_FILE_LINE_ARG are moved to sys/_lock.h, as required by
sys/mutex.h since r326106 (but silently protected by header pollution prior
to this change).
No functional change (intended). Of course, any out of tree modules that
relied on header pollution for sys/eventhandler.h, sys/lock.h, or
sys/mutex.h inclusion need to be fixed. __FreeBSD_version has been bumped.
We need to make the find_veriexec_file() function available publicly, so
rename it to mac_veriexec_metadata_find_file_info() and make it non-static.
Bump the version of the veriexec device interface so user space will know
the labelized version of fingerprint loading is available.
Approved by: sjg
Obtained from: Juniper Networks, Inc.
Differential Revision: https://reviews.freebsd.org/D20295
MAC_VERIEXEC_CHECK_PATH_SYSCALL per-MAC policy system call.
When we are checking the status of the fingerprint on a vnode using the
per-MAC-policy syscall, we do not need an exclusive lock on the vnode.
Even if there is more than one thread requesting the status at the same time,
the worst we can end up doing is processing the file more than once.
This can potentially be improved in the future with offloading the fingerprint
evaluation to a separate thread and blocking until the update completes. But
for now the race is acceptable.
Obtained from: Juniper Networks, Inc.
MFC after: 1 week
be restricted when veriexec is enforced.
Add mpo_system_check_sysctl method to mac_veriexec which does this.
Obtained from: Juniper Networks, Inc.
MFC after: 1 week
are different types across architectures by using %ju and typecasting to
uintmax_t, where appropriate.
Obtained from: Juniper Networks, Inc.
MFC after: 1 week
mac_veriexec_get_executable_flags(). Only try locking/unlocking if the caller
has not already acquired the process lock.
Obtained from: Juniper Networks, Inc.
MFC after: 1 week
lock mac_ifnet_mtx, which protects labels on struct ifnet, unless at least
one policy is actively using labels on ifnets. This avoids a global mutex
acquire in certain fast paths -- most noticeably ifnet transmit. This was
previously invisible by default, as no MAC policies were loaded by default,
but recently became visible due to mac_ntpd being enabled by default.
gallatin@ reports a reduction in PPS overhead from 300% to 2.2% with this
change. We will want to explore further MAC Framework optimisation to
reduce overhead further, but this brings things more back into the world
of the sane.
MFC after: 3 days
The current approach of injecting manifest into mac_veriexec is to
verify the integrity of it in userspace (veriexec (8)) and pass its
entries into kernel using a char device (/dev/veriexec).
This requires verifying root partition integrity in loader,
for example by using memory disk and checking its hash.
Otherwise if rootfs is compromised an attacker could inject their own data.
This patch introduces an option to parse manifest in kernel based on envs.
The loader sets manifest path and digest.
EVENTHANDLER is used to launch the module right after the rootfs is mounted.
It has to be done this way, since one might want to verify integrity of the init file.
This means that manifest is required to be present on the root partition.
Note that the envs have to be set right before boot to make sure that no one can spoof them.
Submitted by: Kornel Duleba <mindal@semihalf.com>
Reviewed by: sjg
Obtained from: Semihalf
Sponsored by: Stormshield
Differential Revision: https://reviews.freebsd.org/D19281
An integrity check such as a check-hash or a cross-correlation failed.
The integrity error falls between EINVAL that identifies errors in
parameters to a system call and EIO that identifies errors with the
underlying storage media. EINTEGRITY is typically raised by intermediate
kernel layers such as a filesystem or an in-kernel GEOM subsystem when
they detect inconsistencies. Uses include allowing the mount(8) command
to return a different exit value to automate the running of fsck(8)
during a system boot.
These changes make no use of the new error, they just add it. Later
commits will be made for the use of the new error number and it will
be added to additional manual pages as appropriate.
Reviewed by: gnn, dim, brueffer, imp
Discussed with: kib, cem, emaste, ed, jilles
Differential Revision: https://reviews.freebsd.org/D18765
Prior to the change the code would branch on return value and then check
if probes are enabled. Since vast majority of the time they are not, this
is clearly wasteful. Check probes first.
Sponsored by: The FreeBSD Foundation
