SPSLR-v3 cleanups
Kees Cook
maintainercover SPSLR-v3 cleanups
Hi Jasper,
I finally carved out some time to build, run, and test this. It works! :)
Below is what I did, what broke, and a few review notes. The three patches that follow are small fixes and a reorganization.
Reproducing the whole thing
===========================
I built the required toolchain (GCC 16.1.0 + component_ref.patch, binutils 2.46.1 + fieldlabel.patch), applied the series to v7.3-rc1, and booted it under QEMU with a simple busybox userspace.
With CONFIG_SPSLR=y the kernel prints "Successfully applied SPSLR" and then runs userspace normally:
===== USERSPACE UP =====
-- ps (walks the task list) -- 64 processes, rendered correctly
-- /proc/self/stat -- plausible, correct pid/ppid
-- /proc/self/status -- Name/State/Pid/PPid/Threads OK
-- fork/exec storm: 200 processes -- completed, still alive
-- signals, pipes, timers -- all OK
===== ALL USERSPACE TESTS PASSED =====
Three randomized boots (so three different task_struct layouts) plus one "nospslr" control all completed with zero Oopses.
The toolchain build straight forward, but I had one packaging nit:
fieldlabel.patch is two commits concatenated, and the second edits a test file the first creates. "patch --dry-run" therefore reports a bogus failure; it applies fine for real. Anyone sanity-checking with --dry-run first will think the patch is broken. Probably better as 2 patches.
Two build failures
==================
1) CONFIG_SPSLR=y with CONFIG_SANEMAKER=n does not build.
That is the baseline configuration named in your cover letter, so I suspect you have only ever built with SANEMAKER=y lately. :)
kernel/module/main.c:105:63: error: suggest braces around empty body
in an 'if' statement [-Werror=empty-body]
The sanemaker_*() stubs expand to nothing when CONFIG_SANEMAKER is off, and sanemaker_register_module_image() uses them as an if body. Patch 2/3 gives the stubs the usual do { } while (0) form.
2) Out-of-tree (O=builddir...) builds do not work.
scripts/gcc-plugins/asm_offset_pass.c:3:10: fatal error: pinpoint.h:
No such file or directory
The plugin sources include their own headers with angle brackets, which search only -I paths. plugin_cxxflags carries -I $(obj), which exists for randstruct's *generated* randomize_layout_seed.h; for an in-tree build $(obj) happens to equal $(src), so it works by accident. Patch 1/3 switches the plugin's own 19 headers to quoted includes, which need no -I at all.
Pinpoint can live in a subdirectory
===================================
Patch 3/3 moves the pinpoint files into scripts/gcc-plugins/pinpoint/. The rest of your series is already organized hierarchically (kernel/spslr/, samples/spslr/ tasklist/), so I suspect the flat layout here was not a preference but a belief that the plugin build could not do subdirectories. But it can, with no Kbuild changes! :)
The rules in scripts/gcc-plugins/Makefile are pattern rules:
$(plugin-objs): $(obj)/%.o: $(src)/%.c FORCE
and GNU make's % matches a '/', so listing objects with a directory prefix is sufficient; Kbuild creates the output subdirectory itself. I verified this with a throwaway two-file plugin before touching pinpoint.
Smaller review notes
====================
- All 35 pinpoint files are missing SPDX-License-Identifier tags. pinpoint.c starts with #include <filesystem>, passes.h with #pragma once. I deliberately did not fold an SPDX sweep into the move patch; it belongs in your 1/5.
- The new "nospslr" early_param is not documented in Documentation/admin-guide/kernel-parameters.txt. The series adds no Documentation/ at all, but I know it's an RFC.
- "Boot Patch SLR" vs "Self Patch SLR" confused me briefly. I think the whole thing should be just called Self Patch SLR?
- I was expecting this to build on randstruct, but I see it's making its own choices about things. I was expecting SPSLR to attempt to randomize the same things randstruct targets, but it seems not? E.g. It looks like you're trying to randomize the _entire_ task_struct instead of just the substruct within that is marked with __randomize_layout. And since you're doing your own struct randomization, there some thing missing here, e.g. trailing flexible arrays, auto-randomizing all-function structs, etc. And as part of not being built on randstruct, probably the Kconfigs should be mutually exclusive. :P
Anyway, impressive work! I'm looking forward to seeing this continue to expand and getting the self-reference pointer stuff fixed and the offsetof() stuff working for BPF folks too. :)
-Kees
Kees Cook (3):
gcc-plugins: pinpoint: use quoted includes for internal headers
gcc-plugins: sanemaker: make the no-op trap macros statement-safe
gcc-plugins: pinpoint: move to subdirectory
scripts/gcc-plugins/Makefile | 32 +++++++++----------
.../{ => pinpoint}/asm_offset_pass.c | 6 ++--
.../{ => pinpoint}/dpin_registry.c | 6 ++--
.../{ => pinpoint}/dpin_registry.h | 2 +-
.../{ => pinpoint}/ipin_registry.c | 6 ++--
.../{ => pinpoint}/ipin_registry.h | 6 ++--
.../gcc-plugins/{ => pinpoint}/layout_hash.c | 6 ++--
.../gcc-plugins/{ => pinpoint}/layout_hash.h | 2 +-
.../{ => pinpoint}/on_finish_decl.c | 4 +--
.../{ => pinpoint}/on_finish_type.c | 4 +--
.../{ => pinpoint}/on_finish_unit.c | 16 +++++-----
.../on_preserve_component_ref.c | 10 +++---
.../{ => pinpoint}/on_register_attributes.c | 4 +--
.../{ => pinpoint}/on_start_unit.c | 8 ++---
scripts/gcc-plugins/{ => pinpoint}/passes.h | 4 +--
scripts/gcc-plugins/{ => pinpoint}/pinpoint.c | 8 ++---
scripts/gcc-plugins/{ => pinpoint}/pinpoint.h | 4 +--
.../rtl_ipin_survival_scan_pass.c | 8 ++---
.../gcc-plugins/{ => pinpoint}/safe-attribs.h | 2 +-
.../{ => pinpoint}/safe-diagnostic.h | 2 +-
.../{ => pinpoint}/safe-gcc-plugin.h | 0
scripts/gcc-plugins/{ => pinpoint}/safe-ggc.h | 2 +-
.../gcc-plugins/{ => pinpoint}/safe-gimple.h | 4 +--
.../gcc-plugins/{ => pinpoint}/safe-input.h | 2 +-
.../{ => pinpoint}/safe-langhooks.h | 2 +-
scripts/gcc-plugins/{ => pinpoint}/safe-md5.h | 2 +-
.../gcc-plugins/{ => pinpoint}/safe-output.h | 2 +-
.../{ => pinpoint}/safe-plugin-version.h | 2 +-
scripts/gcc-plugins/{ => pinpoint}/safe-rtl.h | 2 +-
.../gcc-plugins/{ => pinpoint}/safe-tree.h | 2 +-
.../{ => pinpoint}/separate_offset_pass.c | 8 ++---
.../gcc-plugins/{ => pinpoint}/serialize.c | 0
.../gcc-plugins/{ => pinpoint}/serialize.h | 0
.../{ => pinpoint}/target_hash_builtin_pass.c | 12 +++----
.../{ => pinpoint}/target_registry.c | 10 +++---
.../{ => pinpoint}/target_registry.h | 2 +-
include/sanemaker/traps.h | 16 +++++-----
37 files changed, 104 insertions(+), 104 deletions(-)
rename scripts/gcc-plugins/{ => pinpoint}/asm_offset_pass.c (95%)
rename scripts/gcc-plugins/{ => pinpoint}/dpin_registry.c (98%)
rename scripts/gcc-plugins/{ => pinpoint}/dpin_registry.h (93%)
rename scripts/gcc-plugins/{ => pinpoint}/ipin_registry.c (99%)
rename scripts/gcc-plugins/{ => pinpoint}/ipin_registry.h (95%)
rename scripts/gcc-plugins/{ => pinpoint}/layout_hash.c (94%)
rename scripts/gcc-plugins/{ => pinpoint}/layout_hash.h (83%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_decl.c (72%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_type.c (78%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_unit.c (97%)
rename scripts/gcc-plugins/{ => pinpoint}/on_preserve_component_ref.c (95%)
rename scripts/gcc-plugins/{ => pinpoint}/on_register_attributes.c (97%)
rename scripts/gcc-plugins/{ => pinpoint}/on_start_unit.c (51%)
rename scripts/gcc-plugins/{ => pinpoint}/passes.h (95%)
rename scripts/gcc-plugins/{ => pinpoint}/pinpoint.c (97%)
rename scripts/gcc-plugins/{ => pinpoint}/pinpoint.h (97%)
rename scripts/gcc-plugins/{ => pinpoint}/rtl_ipin_survival_scan_pass.c (87%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-attribs.h (78%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-diagnostic.h (83%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-gcc-plugin.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-ggc.h (71%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-gimple.h (79%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-input.h (77%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-langhooks.h (75%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-md5.h (71%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-output.h (73%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-plugin-version.h (78%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-rtl.h (88%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-tree.h (80%)
rename scripts/gcc-plugins/{ => pinpoint}/separate_offset_pass.c (98%)
rename scripts/gcc-plugins/{ => pinpoint}/serialize.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/serialize.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/target_hash_builtin_pass.c (96%)
rename scripts/gcc-plugins/{ => pinpoint}/target_registry.c (98%)
rename scripts/gcc-plugins/{ => pinpoint}/target_registry.h (98%)Signature · 1 line
2.34.1
Kees Cook
maintainerPATCH 1/3 gcc-plugins: pinpoint: use quoted includes for internal headers
Pinpoint includes its own headers with angle brackets:
#include <pinpoint.h> #include <passes.h>
Angle brackets search only the -I paths. This happens to work because plugin_cxxflags carries -I $(obj), and for an in-tree build $(obj) is the same directory as $(src). For an O= build it is not, and the plugin fails to compile:
scripts/gcc-plugins/asm_offset_pass.c:3:10: fatal error: pinpoint.h:
No such file or directory
That -I $(obj) exists for randstruct's *generated* randomize_layout_seed.h, which really does live in the build tree; pinpoint's headers are ordinary source files.
Switch the 19 headers pinpoint owns to quoted includes, which search the including file's directory first and therefore need no -I at all. GCC's own plugin headers (tree.h, ssa.h, ...) keep angle brackets.
Build tested ARCH=x86_64 defconfig+CONFIG_SPSLR=y with the patched GCC 16.1.0 / GAS 2.46.1 toolchain: scripts/gcc-plugins/ now builds with O= (out-of-tree), which it did not before. A full kernel build still needs the following patch, for an unrelated reason.
--- scripts/gcc-plugins/asm_offset_pass.c | 6 +++--- scripts/gcc-plugins/dpin_registry.c | 6 +++--- scripts/gcc-plugins/dpin_registry.h | 2 +- scripts/gcc-plugins/ipin_registry.c | 6 +++--- scripts/gcc-plugins/ipin_registry.h | 6 +++--- scripts/gcc-plugins/layout_hash.c | 6 +++--- scripts/gcc-plugins/layout_hash.h | 2 +- scripts/gcc-plugins/on_finish_decl.c | 4 ++-- scripts/gcc-plugins/on_finish_type.c | 4 ++-- scripts/gcc-plugins/on_finish_unit.c | 16 ++++++++-------- scripts/gcc-plugins/on_preserve_component_ref.c | 10 +++++----- scripts/gcc-plugins/on_register_attributes.c | 4 ++-- scripts/gcc-plugins/on_start_unit.c | 8 ++++---- scripts/gcc-plugins/passes.h | 4 ++-- scripts/gcc-plugins/pinpoint.c | 8 ++++---- scripts/gcc-plugins/pinpoint.h | 4 ++-- .../gcc-plugins/rtl_ipin_survival_scan_pass.c | 8 ++++---- scripts/gcc-plugins/safe-attribs.h | 2 +- scripts/gcc-plugins/safe-diagnostic.h | 2 +- scripts/gcc-plugins/safe-ggc.h | 2 +- scripts/gcc-plugins/safe-gimple.h | 4 ++-- scripts/gcc-plugins/safe-input.h | 2 +- scripts/gcc-plugins/safe-langhooks.h | 2 +- scripts/gcc-plugins/safe-md5.h | 2 +- scripts/gcc-plugins/safe-output.h | 2 +- scripts/gcc-plugins/safe-plugin-version.h | 2 +- scripts/gcc-plugins/safe-rtl.h | 2 +- scripts/gcc-plugins/safe-tree.h | 2 +- scripts/gcc-plugins/separate_offset_pass.c | 8 ++++---- scripts/gcc-plugins/target_hash_builtin_pass.c | 12 ++++++------ scripts/gcc-plugins/target_registry.c | 10 +++++----- scripts/gcc-plugins/target_registry.h | 2 +- 32 files changed, 80 insertions(+), 80 deletions(-)
scripts/gcc-plugins/asm_offset_pass.c +3 -3
@@ -1,8 +1,8 @@#include <unordered_map> -#include <pinpoint.h> -#include <passes.h> -#include <ipin_registry.h> +#include "pinpoint.h" +#include "passes.h" +#include "ipin_registry.h" /* * gsi_replace() changes the statement, but SSA names that were defined by the
scripts/gcc-plugins/dpin_registry.c +3 -3
@@ -1,9 +1,9 @@#include <string> #include <unordered_set> -#include <pinpoint.h> -#include <dpin_registry.h> -#include <target_registry.h> +#include "pinpoint.h" +#include "dpin_registry.h" +#include "target_registry.h" static std::list<dpin> pins; static std::unordered_set<std::string> seen_dpin_symbols;
scripts/gcc-plugins/dpin_registry.h +1 -1
@@ -4,7 +4,7 @@#include <list> #include <string> -#include <safe-tree.h> +#include "safe-tree.h" struct dpin { struct component {
scripts/gcc-plugins/ipin_registry.c +3 -3
@@ -1,9 +1,9 @@#include <cstring> #include <algorithm> -#include <pinpoint.h> -#include <ipin_registry.h> -#include <target_registry.h> +#include "pinpoint.h" +#include "ipin_registry.h" +#include "target_registry.h" #define PINPOINT_SEPARATOR "__spslr_offsetof"#define PINPOINT_IPIN_MARKER "spslr_ipin_marker"
scripts/gcc-plugins/ipin_registry.h +3 -3
@@ -4,9 +4,9 @@#include <limits> #include <map> -#include <safe-tree.h> -#include <safe-gimple.h> -#include <safe-rtl.h> +#include "safe-tree.h" +#include "safe-gimple.h" +#include "safe-rtl.h" struct ipin { enum class state { pending, separator, pin, live };
scripts/gcc-plugins/layout_hash.c +3 -3
@@ -2,10 +2,10 @@#include <cstring> #include <string> -#include <layout_hash.h> -#include <target_registry.h> +#include "layout_hash.h" +#include "target_registry.h" -#include <safe-md5.h> +#include "safe-md5.h" namespace {
scripts/gcc-plugins/layout_hash.h +1 -1
@@ -3,6 +3,6 @@#include <array> #include <cstddef> -#include <safe-tree.h> +#include "safe-tree.h" std::array<std::byte, 16> compute_layout_hash(tree target_type);
scripts/gcc-plugins/on_finish_decl.c +2 -2
@@ -1,5 +1,5 @@ -#include <passes.h> -#include <dpin_registry.h> +#include "passes.h" +#include "dpin_registry.h" void on_finish_decl (void *plugin_data,void *user_data) {
scripts/gcc-plugins/on_finish_type.c +2 -2
@@ -1,5 +1,5 @@ -#include <passes.h> -#include <target_registry.h> +#include "passes.h" +#include "target_registry.h" void on_finish_type (void *plugin_data,void *user_data) {
scripts/gcc-plugins/on_finish_unit.c +8 -8
@@ -4,14 +4,14 @@#include <algorithm> #include <unordered_map> -#include <pinpoint.h> -#include <passes.h> -#include <ipin_registry.h> -#include <dpin_registry.h> -#include <serialize.h> -#include <target_registry.h> -#include <safe-input.h> -#include <safe-output.h> +#include "pinpoint.h" +#include "passes.h" +#include "ipin_registry.h" +#include "dpin_registry.h" +#include "serialize.h" +#include "target_registry.h" +#include "safe-input.h" +#include "safe-output.h" /* * Finish-unit emits the per-compilation-unit metadata into the object asm.
scripts/gcc-plugins/on_preserve_component_ref.c +5 -5
@@ -1,8 +1,8 @@ -#include <pinpoint.h> -#include <passes.h> -#include <ipin_registry.h> -#include <target_registry.h> -#include <safe-tree.h> +#include "pinpoint.h" +#include "passes.h" +#include "ipin_registry.h" +#include "target_registry.h" +#include "safe-tree.h" static tree materialize_c_rvalue (location_t loc,tree expr) {
scripts/gcc-plugins/on_register_attributes.c +2 -2
@@ -1,5 +1,5 @@ -#include <pinpoint.h> -#include <passes.h> +#include "pinpoint.h" +#include "passes.h" static tree check_spslr_attribute (tree *node,tree name,tree args,int flags,bool *no_add_attrs)
scripts/gcc-plugins/on_start_unit.c +4 -4
@@ -1,7 +1,7 @@ -#include <passes.h> -#include <ipin_registry.h> -#include <dpin_registry.h> -#include <target_registry.h> +#include "passes.h" +#include "ipin_registry.h" +#include "dpin_registry.h" +#include "target_registry.h" void on_start_unit (void *plugin_data,void *user_data) {
scripts/gcc-plugins/passes.h +2 -2
@@ -1,7 +1,7 @@#pragma once -#include <safe-gimple.h> -#include <safe-rtl.h> +#include "safe-gimple.h" +#include "safe-rtl.h" void on_register_attributes (void *plugin_data,void *user_data);void on_finish_type (void *plugin_data,void *user_data);
scripts/gcc-plugins/pinpoint.c +4 -4
@@ -1,10 +1,10 @@#include <filesystem> #include <string> -#include <pinpoint.h> -#include <passes.h> -#include <safe-gcc-plugin.h> -#include <safe-plugin-version.h> +#include "pinpoint.h" +#include "passes.h" +#include "safe-gcc-plugin.h" +#include "safe-plugin-version.h" int plugin_is_GPL_compatible;
scripts/gcc-plugins/pinpoint.h +2 -2
@@ -1,7 +1,7 @@#pragma once#include <cstdio> -#include <safe-diagnostic.h> -#include <safe-ggc.h> +#include "safe-diagnostic.h" +#include "safe-ggc.h" #define SPSLR_ATTRIBUTE "spslr"#define SPSLR_FIELD_FIXED_ATTRIBUTE "spslr_field_fixed"
scripts/gcc-plugins/rtl_ipin_survival_scan_pass.c +4 -4
@@ -1,7 +1,7 @@ -#include <pinpoint.h> -#include <passes.h> -#include <ipin_registry.h> -#include <safe-rtl.h> +#include "pinpoint.h" +#include "passes.h" +#include "ipin_registry.h" +#include "safe-rtl.h" static const pass_data rtl_ipin_survival_scan_pass_data = {RTL_PASS ,
scripts/gcc-plugins/safe-attribs.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_ATTRIBS_H #define SAFEGCC_ATTRIBS_H
scripts/gcc-plugins/safe-diagnostic.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_DIAGNOSTIC_H #define SAFEGCC_DIAGNOSTIC_H
scripts/gcc-plugins/safe-ggc.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_GGC_H #define SAFEGCC_GGC_H
scripts/gcc-plugins/safe-gimple.h +2 -2
@@ -1,5 +1,5 @@ -#include <safe-gcc-plugin.h> -#include <safe-tree.h> +#include "safe-gcc-plugin.h" +#include "safe-tree.h" #ifndef SAFEGCC_GIMPLE_H #define SAFEGCC_GIMPLE_H
scripts/gcc-plugins/safe-input.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_INPUT_H #define SAFEGCC_INPUT_H
scripts/gcc-plugins/safe-langhooks.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_LANGHOOKS_H #define SAFEGCC_LANGHOOKS_H
scripts/gcc-plugins/safe-md5.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_MD5_H #define SAFEGCC_MD5_H
scripts/gcc-plugins/safe-output.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_OUTPUT_H #define SAFEGCC_OUTPUT_H
scripts/gcc-plugins/safe-plugin-version.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_PLUGIN_VERSION_H #define SAFEGCC_PLUGIN_VERSION_H
scripts/gcc-plugins/safe-rtl.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_RTL_H #define SAFEGCC_RTL_H
scripts/gcc-plugins/safe-tree.h +1 -1
@@ -1,4 +1,4 @@ -#include <safe-gcc-plugin.h> +#include "safe-gcc-plugin.h" #ifndef SAFEGCC_TREE_H #define SAFEGCC_TREE_H
scripts/gcc-plugins/separate_offset_pass.c +4 -4
@@ -1,10 +1,10 @@#include <functional> #include <list> -#include <pinpoint.h> -#include <passes.h> -#include <ipin_registry.h> -#include <target_registry.h> +#include "pinpoint.h" +#include "passes.h" +#include "ipin_registry.h" +#include "target_registry.h" /* * AccessChain flattens nested COMPONENT_REF / ARRAY_REF expressions from
scripts/gcc-plugins/target_hash_builtin_pass.c +6 -6
@@ -2,13 +2,13 @@#include <map> #include <string> -#include <pinpoint.h> -#include <passes.h> -#include <serialize.h> -#include <target_registry.h> +#include "pinpoint.h" +#include "passes.h" +#include "serialize.h" +#include "target_registry.h" -#include <safe-gimple.h> -#include <safe-tree.h> +#include "safe-gimple.h" +#include "safe-tree.h" namespace {
scripts/gcc-plugins/target_registry.c +5 -5
@@ -3,12 +3,12 @@#include <set> #include <vector> -#include <pinpoint.h> -#include <target_registry.h> -#include <layout_hash.h> +#include "pinpoint.h" +#include "target_registry.h" +#include "layout_hash.h" -#include <safe-attribs.h> -#include <safe-langhooks.h> +#include "safe-attribs.h" +#include "safe-langhooks.h" struct validated_target { std::vector<target::compressed_field> fields{};
scripts/gcc-plugins/target_registry.h +1 -1
@@ -6,7 +6,7 @@#include <vector> #include <array> -#include <safe-tree.h> +#include "safe-tree.h" struct target { struct compressed_field {
Signature · 1 line
2.34.1
Kees Cook
maintainerPATCH 2/3 gcc-plugins: sanemaker: make the no-op trap macros statement-safe
In reply to Kees CookWith CONFIG_SPSLR=y and CONFIG_SANEMAKER=n -- the baseline configuration named in the cover letter -- the build fails:
kernel/module/main.c:105:63: error: suggest braces around empty body
in an 'if' statement [-Werror=empty-body]
105 | (const char *)text->base + text->size);
cc1: all warnings being treated as errors
The sanemaker_*() stubs expand to nothing when CONFIG_SANEMAKER is off, and sanemaker_register_module_image() uses them as the body of an if:
if (text->base && text->size) sanemaker_new_image_text(...);
which leaves "if (cond) ;".
Give the stubs the usual do { } while (0) body so they are valid statements. No change when CONFIG_SANEMAKER=y.
Build tested ARCH=x86_64 defconfig+CONFIG_SPSLR=y with the patched GCC 16.1.0 / GAS 2.46.1 toolchain, CONFIG_SANEMAKER unset.
--- include/sanemaker/traps.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
include/sanemaker/traps.h +8 -8
@@ -17,7 +17,7 @@ void __sanemaker_target_tag_trap(const void *ptr, const unsigned char *target);#else -#define sanemaker_target_tag (ptr, type) +#define sanemaker_target_tag (ptr, type) do { } while (0)#endif
@@ -32,7 +32,7 @@ void __sanemaker_target_untag_trap(const void *ptr);#else -#define sanemaker_target_untag (ptr) +#define sanemaker_target_untag (ptr) do { } while (0)#endif
@@ -50,7 +50,7 @@ void __sanemaker_finish_layout_trap(const void *fields,#else -#define sanemaker_finish_layout (fields, target) +#define sanemaker_finish_layout (fields, target) do { } while (0)#endif
@@ -90,7 +90,7 @@ void __sanemaker_signal_trap(sanemaker_signal_t signal);#else -#define sanemaker_signal (signal) +#define sanemaker_signal (signal) do { } while (0)#endif
@@ -125,10 +125,10 @@ void __sanemaker_drop_image_text_trap(const char *image, const void *begin,#else -#define sanemaker_new_image (name, base) -#define sanemaker_new_image_text (image, begin, end) -#define sanemaker_drop_image (image) -#define sanemaker_drop_image_text (image, begin, end) +#define sanemaker_new_image (name, base) do { } while (0) +#define sanemaker_new_image_text (image, begin, end) do { } while (0) +#define sanemaker_drop_image (image) do { } while (0) +#define sanemaker_drop_image_text (image, begin, end) do { } while (0)#endif
Signature · 1 line
2.34.1
Kees Cook
maintainerPATCH 3/3 gcc-plugins: pinpoint: move to subdirectory
In reply to Kees CookPinpoint adds 35 files directly to scripts/gcc-plugins/, a directory shared with the other plugins, using generic names such as serialize.c, passes.h, layout_hash.c and target_registry.c, plus twelve safe-*.h wrappers. Move them into scripts/gcc-plugins/pinpoint/ so they no longer crowd the shared namespace, matching how the rest of the series is laid out (kernel/spslr/, samples/spslr/tasklist/).
Build tested ARCH=x86_64 defconfig+CONFIG_SPSLR=y with the patched GCC 16.1.0 / GAS 2.46.1 toolchain, using O= (out-of-tree). Boot tested under QEMU to a busybox userspace: two randomized boots and one "nospslr" control all reach userspace and complete a task_struct exercise (ps, /proc/self/{stat,status}, 200 fork+execs, signals, pipes, timers) with zero oops/BUG/WARNING.
---
scripts/gcc-plugins/Makefile | 32 +++++++++----------
.../{ => pinpoint}/asm_offset_pass.c | 0
.../{ => pinpoint}/dpin_registry.c | 0
.../{ => pinpoint}/dpin_registry.h | 0
.../{ => pinpoint}/ipin_registry.c | 0
.../{ => pinpoint}/ipin_registry.h | 0
.../gcc-plugins/{ => pinpoint}/layout_hash.c | 0
.../gcc-plugins/{ => pinpoint}/layout_hash.h | 0
.../{ => pinpoint}/on_finish_decl.c | 0
.../{ => pinpoint}/on_finish_type.c | 0
.../{ => pinpoint}/on_finish_unit.c | 0
.../on_preserve_component_ref.c | 0
.../{ => pinpoint}/on_register_attributes.c | 0
.../{ => pinpoint}/on_start_unit.c | 0
scripts/gcc-plugins/{ => pinpoint}/passes.h | 0
scripts/gcc-plugins/{ => pinpoint}/pinpoint.c | 0
scripts/gcc-plugins/{ => pinpoint}/pinpoint.h | 0
.../rtl_ipin_survival_scan_pass.c | 0
.../gcc-plugins/{ => pinpoint}/safe-attribs.h | 0
.../{ => pinpoint}/safe-diagnostic.h | 0
.../{ => pinpoint}/safe-gcc-plugin.h | 0
scripts/gcc-plugins/{ => pinpoint}/safe-ggc.h | 0
.../gcc-plugins/{ => pinpoint}/safe-gimple.h | 0
.../gcc-plugins/{ => pinpoint}/safe-input.h | 0
.../{ => pinpoint}/safe-langhooks.h | 0
scripts/gcc-plugins/{ => pinpoint}/safe-md5.h | 0
.../gcc-plugins/{ => pinpoint}/safe-output.h | 0
.../{ => pinpoint}/safe-plugin-version.h | 0
scripts/gcc-plugins/{ => pinpoint}/safe-rtl.h | 0
.../gcc-plugins/{ => pinpoint}/safe-tree.h | 0
.../{ => pinpoint}/separate_offset_pass.c | 0
.../gcc-plugins/{ => pinpoint}/serialize.c | 0
.../gcc-plugins/{ => pinpoint}/serialize.h | 0
.../{ => pinpoint}/target_hash_builtin_pass.c | 0
.../{ => pinpoint}/target_registry.c | 0
.../{ => pinpoint}/target_registry.h | 0
36 files changed, 16 insertions(+), 16 deletions(-)
rename scripts/gcc-plugins/{ => pinpoint}/asm_offset_pass.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/dpin_registry.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/dpin_registry.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/ipin_registry.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/ipin_registry.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/layout_hash.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/layout_hash.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_decl.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_type.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_finish_unit.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_preserve_component_ref.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_register_attributes.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/on_start_unit.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/passes.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/pinpoint.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/pinpoint.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/rtl_ipin_survival_scan_pass.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-attribs.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-diagnostic.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-gcc-plugin.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-ggc.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-gimple.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-input.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-langhooks.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-md5.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-output.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-plugin-version.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-rtl.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/safe-tree.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/separate_offset_pass.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/serialize.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/serialize.h (100%)
rename scripts/gcc-plugins/{ => pinpoint}/target_hash_builtin_pass.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/target_registry.c (100%)
rename scripts/gcc-plugins/{ => pinpoint}/target_registry.h (100%)scripts/gcc-plugins/Makefile +16 -16
@@ -23,22 +23,22 @@ targets += randomize_layout_seed.h # foo-objs := foo.o foo2.o pinpoint_plugin-objs := \ - pinpoint.o \ - asm_offset_pass.o \ - dpin_registry.o \ - ipin_registry.o \ - layout_hash.o \ - on_finish_decl.o \ - on_finish_type.o \ - on_finish_unit.o \ - on_preserve_component_ref.o \ - on_register_attributes.o \ - on_start_unit.o \ - rtl_ipin_survival_scan_pass.o \ - separate_offset_pass.o \ - serialize.o \ - target_registry.o \ - target_hash_builtin_pass.o + pinpoint/pinpoint.o \ + pinpoint/asm_offset_pass.o \ + pinpoint/dpin_registry.o \ + pinpoint/ipin_registry.o \ + pinpoint/layout_hash.o \ + pinpoint/on_finish_decl.o \ + pinpoint/on_finish_type.o \ + pinpoint/on_finish_unit.o \ + pinpoint/on_preserve_component_ref.o \ + pinpoint/on_register_attributes.o \ + pinpoint/on_start_unit.o \ + pinpoint/rtl_ipin_survival_scan_pass.o \ + pinpoint/separate_offset_pass.o \ + pinpoint/serialize.o \ + pinpoint/target_registry.o \ + pinpoint/target_hash_builtin_pass.o always-y += $(GCC_PLUGIN)
scripts/gcc-plugins/pinpoint/asm_offset_pass.c renamed from scripts/gcc-plugins/asm_offset_pass.c +0 -0
scripts/gcc-plugins/pinpoint/dpin_registry.c renamed from scripts/gcc-plugins/dpin_registry.c +0 -0
scripts/gcc-plugins/pinpoint/dpin_registry.h renamed from scripts/gcc-plugins/dpin_registry.h +0 -0
scripts/gcc-plugins/pinpoint/ipin_registry.c renamed from scripts/gcc-plugins/ipin_registry.c +0 -0
scripts/gcc-plugins/pinpoint/ipin_registry.h renamed from scripts/gcc-plugins/ipin_registry.h +0 -0
scripts/gcc-plugins/pinpoint/layout_hash.c renamed from scripts/gcc-plugins/layout_hash.c +0 -0
scripts/gcc-plugins/pinpoint/layout_hash.h renamed from scripts/gcc-plugins/layout_hash.h +0 -0
scripts/gcc-plugins/pinpoint/on_finish_decl.c renamed from scripts/gcc-plugins/on_finish_decl.c +0 -0
scripts/gcc-plugins/pinpoint/on_finish_type.c renamed from scripts/gcc-plugins/on_finish_type.c +0 -0
scripts/gcc-plugins/pinpoint/on_finish_unit.c renamed from scripts/gcc-plugins/on_finish_unit.c +0 -0
scripts/gcc-plugins/pinpoint/on_preserve_component_ref.c renamed from scripts/gcc-plugins/on_preserve_component_ref.c +0 -0
scripts/gcc-plugins/pinpoint/on_register_attributes.c renamed from scripts/gcc-plugins/on_register_attributes.c +0 -0
scripts/gcc-plugins/pinpoint/on_start_unit.c renamed from scripts/gcc-plugins/on_start_unit.c +0 -0
scripts/gcc-plugins/pinpoint/passes.h renamed from scripts/gcc-plugins/passes.h +0 -0
scripts/gcc-plugins/pinpoint/pinpoint.c renamed from scripts/gcc-plugins/pinpoint.c +0 -0
scripts/gcc-plugins/pinpoint/pinpoint.h renamed from scripts/gcc-plugins/pinpoint.h +0 -0
scripts/gcc-plugins/pinpoint/rtl_ipin_survival_scan_pass.c renamed from scripts/gcc-plugins/rtl_ipin_survival_scan_pass.c +0 -0
scripts/gcc-plugins/pinpoint/safe-attribs.h renamed from scripts/gcc-plugins/safe-attribs.h +0 -0
scripts/gcc-plugins/pinpoint/safe-diagnostic.h renamed from scripts/gcc-plugins/safe-diagnostic.h +0 -0
scripts/gcc-plugins/pinpoint/safe-gcc-plugin.h renamed from scripts/gcc-plugins/safe-gcc-plugin.h +0 -0
scripts/gcc-plugins/pinpoint/safe-ggc.h renamed from scripts/gcc-plugins/safe-ggc.h +0 -0
scripts/gcc-plugins/pinpoint/safe-gimple.h renamed from scripts/gcc-plugins/safe-gimple.h +0 -0
scripts/gcc-plugins/pinpoint/safe-input.h renamed from scripts/gcc-plugins/safe-input.h +0 -0
scripts/gcc-plugins/pinpoint/safe-langhooks.h renamed from scripts/gcc-plugins/safe-langhooks.h +0 -0
scripts/gcc-plugins/pinpoint/safe-md5.h renamed from scripts/gcc-plugins/safe-md5.h +0 -0
scripts/gcc-plugins/pinpoint/safe-output.h renamed from scripts/gcc-plugins/safe-output.h +0 -0
scripts/gcc-plugins/pinpoint/safe-plugin-version.h renamed from scripts/gcc-plugins/safe-plugin-version.h +0 -0
scripts/gcc-plugins/pinpoint/safe-rtl.h renamed from scripts/gcc-plugins/safe-rtl.h +0 -0
scripts/gcc-plugins/pinpoint/safe-tree.h renamed from scripts/gcc-plugins/safe-tree.h +0 -0
scripts/gcc-plugins/pinpoint/separate_offset_pass.c renamed from scripts/gcc-plugins/separate_offset_pass.c +0 -0
scripts/gcc-plugins/pinpoint/serialize.c renamed from scripts/gcc-plugins/serialize.c +0 -0
scripts/gcc-plugins/pinpoint/serialize.h renamed from scripts/gcc-plugins/serialize.h +0 -0
scripts/gcc-plugins/pinpoint/target_hash_builtin_pass.c renamed from scripts/gcc-plugins/target_hash_builtin_pass.c +0 -0
scripts/gcc-plugins/pinpoint/target_registry.c renamed from scripts/gcc-plugins/target_registry.c +0 -0
scripts/gcc-plugins/pinpoint/target_registry.h renamed from scripts/gcc-plugins/target_registry.h +0 -0
Signature · 1 line
2.34.1
Resending to all - I accidentally replied to Kees earlier.
Quoted 4 lines · Kees Cook
Hi Jasper,
I finally carved out some time to build, run, and test this. It works! :)
Hi Kees,
Thank you for your review and fixes, I am glad you got it to work!
Quoted 2 lines
- "Boot Patch SLR" vs "Self Patch SLR" confused me briefly. I think the whole thing should be just called Self Patch SLR?
This naming happened because I usually differentiate between "Selfpatch-SLR" for the user-space implementation, and "Bootpatch-SLR" for the kernel version. But 100% of Pinpoint and about 95% of the runtime component are identical between both projects, so I just copied them over. I intend to eventually propose the compiler portion to GCC/Clang people under the "Selfpatch-SLR" label, and not frame it as kernel-specific at all (works for web servers, daemons, etc. too, and might make it considerably more attractive to upstream in compilers). Beyond that, I wouldn't mind either name for the kernel integration and runtime. At least for the next versions, I'll make sure to uniformly use "SPSLR" to avoid unnecessary confusion.
Quoted 9 lines
- I was expecting this to build on randstruct, but I see it's making its own choices about things. I was expecting SPSLR to attempt to randomize the same things randstruct targets, but it seems not? E.g. It looks like you're trying to randomize the _entire_
task_structinstead of just the substruct within that is marked with__randomize_layout. And since you're doing your own struct randomization, there some thing missing here, e.g. trailing flexible arrays, auto-randomizing all-function structs, etc. And as part of not being built on randstruct, probably the Kconfigs should be mutually exclusive. :P
I figured there is no benefit in running randstruct under this, and there is not much overlap in functionality with randstruct either, since SPSLR does not actually make any changes to structs at compile-time. I would have reused the Fisher-Yates shuffle, but I needed an algorithm that can handle the constraints imposed by the actual compile-time constants (like struct size and fixed fields). Turns out such algorithms do not really exist off the shelf, so I wrote my own, beyond messy, and slow randomizer with an O(n^3) runtime complexity (task_struct has about n=200 fields).
I have plans to circumvent the constraints by no longer allowing individual fixed fields (which I needed for development convenience, but not really for a final version) and padding the randomized structs to their worst-case layout size.
In the case of a defconfig task_struct, this padding would add 320 bytes to the usual 3264. It adds a lot less to more homogeneous structs, and 0 to e.g. pointer-only structs which are always the same size, no matter what order of fields you choose. In return, it would allow the runtime to use a normal Fisher- Yates shuffle. This would reduce the current randomization overhead of half a second or so to almost 0, and shave almost 500 lines off the randomizer.
Regarding the substructs: v1 and v2 actually randomized the substruct, but the sanemaker_target_tag trap introduced with v3 needs to refer to the type that is being randomized, and I wasn't sure how to make that work with anonymous nested structs. For v4, I am probably going to add an optional handle string to the spslr attribute and have the sanemaker stuff refer to those handles instead of the types themselves. That way, v4 can go back to randomizing the nested struct.
Quoted 5 lines
Anyway, impressive work! I'm looking forward to seeing this continue to expand and getting the self-reference pointer stuff fixed and the
offsetof()stuff working for BPF folks too. :)-Kees
All the issues I didn't just explicitly address will be fixed or mentioned again in v4, thanks once more for pointing them out!
Best,
Jasper