CVS commit: src

["Maxime Villard" <maxv%netbsd.org@localhost>]

Module Name:    src
Committed By:   maxv
Date:           Sat May  2 11:37:17 UTC 2020

Modified Files:
        src/external/cddl/osnet/dev/fbt/x86: fbt_isa.c
        src/sys/arch/amd64/amd64: cpufunc.S
        src/sys/arch/i386/i386: cpufunc.S
        src/sys/arch/x86/include: cpufunc.h
        src/sys/arch/x86/x86: patch.c spectre.c svs.c

Log Message:
Modify the hotpatch mechanism, in order to make it much less ROP-friendly.

Currently x86_patch_window_open is a big problem, because it is a perfect
function to inject/modify executable code with ROP.

 - Remove x86_patch_window_open(), along with its x86_patch_window_close()
   counterpart.
 - Introduce a read-only link-set of hotpatch descriptor structures,
   which reference a maximum of two read-only hotpatch sources.
 - Modify x86_hotpatch() to open a window and call the new
   x86_hotpatch_apply() function in a hard-coded manner.
 - Modify x86_hotpatch() to take a name and a selector, and have
   x86_hotpatch_apply() resolve the descriptor from the name and the
   source from the selector, before hotpatching.
 - Move the error handling in a separate x86_hotpatch_cleanup() function,
   that gets called after we closed the window.

The resulting implementation is a bit complex and non-obvious. But it
gains the following properties: the code executed in the hotpatch window
is strictly hard-coded (no callback and no possibility to execute your own
code in the window) and the pointers this code accesses are strictly
read-only (no possibility to forge pointers to hotpatch an area that was
not designated as hotpatchable at compile-time, and no possibility to
choose what bytes to write other than the maximum of two read-only
templates that were designated as valid for the given destination at
compile-time).

With current CPUs this slightly improves a situation that is already
pretty bad by definition on x86. Assuming CET however, this change closes
a big hole and is kinda great.

The only ~problem there is, is that dtrace-fbt tries to hotpatch random
places with random bytes, and there is just no way to make it safe.
However dtrace is only in a module, that is rarely used and never compiled
into the kernel, so it's not a big problem; add a shitty & vulnerable
independent hotpatch window in it, and leave big XXXs. It looks like fbt
is going to collapse soon anyway.


To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/external/cddl/osnet/dev/fbt/x86/fbt_isa.c
cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/amd64/cpufunc.S
cvs rdiff -u -r1.38 -r1.39 src/sys/arch/i386/i386/cpufunc.S
cvs rdiff -u -r1.38 -r1.39 src/sys/arch/x86/include/cpufunc.h
cvs rdiff -u -r1.46 -r1.47 src/sys/arch/x86/x86/patch.c
cvs rdiff -u -r1.34 -r1.35 src/sys/arch/x86/x86/spectre.c \
    src/sys/arch/x86/x86/svs.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.