Skip to content

RFC SPSLR-v3 cleanups

Kees Cook linux-hardening 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 RFC Latest3 patches 1 replyIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3gcc-plugins: pinpoint: use quoted includes for internal headersscripts/gcc-plugins/asm_offset_pass.c scripts/gcc-plugins/dpin_registry.c scripts/gcc-plugins/dpin_registry.h +29 more+80 -800 replies
2/3gcc-plugins: sanemaker: make the no-op trap macros statement-safeinclude/sanemaker/traps.h+8 -80 replies
3/3gcc-plugins: pinpoint: move to subdirectoryscripts/gcc-plugins/Makefile scripts/gcc-plugins/pinpoint/asm_offset_pass.c scripts/gcc-plugins/pinpoint/dpin_registry.c +33 more+16 -160 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

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