All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] SPSLR-v3 cleanups
@ 2026-09-02 23:33 Kees Cook
  2026-09-02 23:33 ` [PATCH 1/3] gcc-plugins: pinpoint: use quoted includes for internal headers Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Kees Cook @ 2026-09-02 23:33 UTC (permalink / raw)
  To: York Jasper Niebuhr; +Cc: Kees Cook, Ard Biesheuvel, franzen, linux-hardening

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%)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-07 16:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 23:33 [RFC][PATCH 0/3] SPSLR-v3 cleanups Kees Cook
2026-09-02 23:33 ` [PATCH 1/3] gcc-plugins: pinpoint: use quoted includes for internal headers Kees Cook
2026-09-02 23:33 ` [PATCH 2/3] gcc-plugins: sanemaker: make the no-op trap macros statement-safe Kees Cook
2026-09-02 23:33 ` [PATCH 3/3] gcc-plugins: pinpoint: move to subdirectory Kees Cook
2026-09-07 16:44 ` [RFC][PATCH 0/3] SPSLR-v3 cleanups Jasper Niebuhr

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.