RFC SPSLR-v3 cleanups
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest3 patches 1 replyIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | gcc-plugins: pinpoint: use quoted includes for internal headersscripts/ scripts/ scripts/ +29 more | +80 | 0 replies | |
| 2/3 | gcc-plugins: sanemaker: make the no-op trap macros statement-safeinclude/ | +8 | 0 replies | |
| 3/3 | gcc-plugins: pinpoint: move to subdirectoryscripts/ scripts/ scripts/ +33 more | +16 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [RFC][PATCH 0/3] 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