Linux Hardening
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: York Jasper Niebuhr <yjn@yjn-systems.com>
Cc: Kees Cook <kees@kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
	franzen@sec.in.tum.de, linux-hardening@vger.kernel.org
Subject: [RFC][PATCH 0/3] SPSLR-v3 cleanups
Date: Wed,  2 Sep 2026 16:33:40 -0700	[thread overview]
Message-ID: <20260902233338.i.375-kees@kernel.org> (raw)

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


             reply	other threads:[~2026-09-02 23:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02 23:33 Kees Cook [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260902233338.i.375-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=ardb@kernel.org \
    --cc=franzen@sec.in.tum.de \
    --cc=linux-hardening@vger.kernel.org \
    --cc=yjn@yjn-systems.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox