* Re: [RFC 08/11] security: Hornet LSM
From: Alexei Starovoitov @ 2025-12-18 1:22 UTC (permalink / raw)
To: Blaise Boscaccy, Linus Torvalds
Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James Bottomley,
David Howells, LSM List, open list:DOCUMENTATION, LKML, bpf
In-Reply-To: <20251211021257.1208712-9-bboscaccy@linux.microsoft.com>
On Wed, Dec 10, 2025 at 6:14 PM Blaise Boscaccy
<bboscaccy@linux.microsoft.com> wrote:
> +++ b/security/hornet/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config SECURITY_HORNET
> + bool "Hornet support"
> + depends on SECURITY
> + default n
So you're disallowing this new LSM to be a module?
That doesn't smell good.
> +static int hornet_verify_hashes(struct hornet_maps *maps,
> + struct hornet_parse_context *ctx)
> +{
> + int map_fd;
> + u32 i;
> + struct bpf_map *map;
> + int err = 0;
> + unsigned char hash[SHA256_DIGEST_SIZE];
> +
> + for (i = 0; i < ctx->hash_count; i++) {
> + if (ctx->skips[i])
> + continue;
> +
> + err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
> + ctx->indexes[i] * sizeof(map_fd),
> + sizeof(map_fd));
As was pointed out several times earlier this is an obvious TOCTOU bug.
An attacker can change this map_fd between LSM checks and later verifier use.
All the "security" checks further are useless.
> + if (err < 0)
> + return LSM_INT_VERDICT_BADSIG;
> +
> + CLASS(fd, f)(map_fd);
> + if (fd_empty(f))
> + return LSM_INT_VERDICT_BADSIG;
> + if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
Ohh. So this is why this LSM has to be built-in.
bpf_map_fops is bpf internal detail. It's not going to be exported.
You cannot open code __bpf_map_get() and get away with it.
> + return LSM_INT_VERDICT_BADSIG;
> +
> + if (!map->frozen)
> + return LSM_INT_VERDICT_BADSIG;
> +
> + map = fd_file(f)->private_data;
> + map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
This too. It's absolutely not ok for LSM to mess with bpf internal state.
The whole LSM is one awful hack.
The diff stat doesn't touch anything in the kernel/bpf/
yet you're messing with bpf internals.
Clearly, you guys want to merge this garbage through LSM tree.
Make sure to keep my Nack when you send it during the merge window.
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2025-12-18 7:21 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Corbet, Sumit Garg, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Sumit Garg, Ilias Apalodimas,
Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
Hi,
On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello,
>
> the objective of this series is to make tee driver stop using callbacks
> in struct device_driver. These were superseded by bus methods in 2006
> (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> methods.")) but nobody cared to convert all subsystems accordingly.
>
> Here the tee drivers are converted. The first commit is somewhat
> unrelated, but simplifies the conversion (and the drivers). It
> introduces driver registration helpers that care about setting the bus
> and owner. (The latter is missing in all drivers, so by using these
> helpers the drivers become more correct.)
>
> v1 of this series is available at
> https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
>
> Changes since v1:
>
> - rebase to v6.19-rc1 (no conflicts)
> - add tags received so far
> - fix whitespace issues pointed out by Sumit Garg
> - fix shutdown callback to shutdown and not remove
>
> As already noted in v1's cover letter, this series should go in during a
> single merge window as there are runtime warnings when the series is
> only applied partially. Sumit Garg suggested to apply the whole series
> via Jens Wiklander's tree.
> If this is done the dependencies in this series are honored, in case the
> plan changes: Patches #4 - #17 depend on the first two.
>
> Note this series is only build tested.
>
> Uwe Kleine-König (17):
> tee: Add some helpers to reduce boilerplate for tee client drivers
> tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> tee: Adapt documentation to cover recent additions
> hwrng: optee - Make use of module_tee_client_driver()
> hwrng: optee - Make use of tee bus methods
> rtc: optee: Migrate to use tee specific driver registration function
> rtc: optee: Make use of tee bus methods
> efi: stmm: Make use of module_tee_client_driver()
> efi: stmm: Make use of tee bus methods
> firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> firmware: arm_scmi: Make use of tee bus methods
> firmware: tee_bnxt: Make use of module_tee_client_driver()
> firmware: tee_bnxt: Make use of tee bus methods
> KEYS: trusted: Migrate to use tee specific driver registration
> function
> KEYS: trusted: Make use of tee bus methods
> tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> tpm/tpm_ftpm_tee: Make use of tee bus methods
>
> Documentation/driver-api/tee.rst | 18 +----
> drivers/char/hw_random/optee-rng.c | 26 ++----
> drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> drivers/rtc/rtc-optee.c | 27 ++-----
> drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> include/linux/tee_drv.h | 12 +++
> security/keys/trusted-keys/trusted_tee.c | 17 ++--
> 10 files changed, 164 insertions(+), 138 deletions(-)
>
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> --
> 2.47.3
>
Thank you for the nice cleanup, Uwe.
I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
The branch is based on v6.19-rc1, and I'll try to keep it stable for
others to depend on, if needed. Let's see if we can agree on taking
the remaining patches via that branch.
Cheers,
Jens
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Peter Zijlstra @ 2025-12-18 11:23 UTC (permalink / raw)
To: Marco Elver
Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <aUGBff8Oko5O8EsP@elver.google.com>
On Tue, Dec 16, 2025 at 04:57:49PM +0100, Marco Elver wrote:
> Below is the preview of the complete changes to make the lock guards
> work properly.
Right. Not pretty but it works.
I did spend a few hours yesterday trying out various thing that don't
work -- as I'm sure you did too -- but could not come up with something
saner.
So yeah, lets just do this.
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Alexandre Belloni @ 2025-12-18 13:53 UTC (permalink / raw)
To: Jens Wiklander
Cc: Uwe Kleine-König, Jonathan Corbet, Sumit Garg,
Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <CAHUa44FrDZbvRvfN8obf80_k=Eqxe9YxHpjaE5jU7nkxPUwfag@mail.gmail.com>
On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> Hi,
>
> On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> >
> > Hello,
> >
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> >
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> >
> > v1 of this series is available at
> > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> >
> > Changes since v1:
> >
> > - rebase to v6.19-rc1 (no conflicts)
> > - add tags received so far
> > - fix whitespace issues pointed out by Sumit Garg
> > - fix shutdown callback to shutdown and not remove
> >
> > As already noted in v1's cover letter, this series should go in during a
> > single merge window as there are runtime warnings when the series is
> > only applied partially. Sumit Garg suggested to apply the whole series
> > via Jens Wiklander's tree.
> > If this is done the dependencies in this series are honored, in case the
> > plan changes: Patches #4 - #17 depend on the first two.
> >
> > Note this series is only build tested.
> >
> > Uwe Kleine-König (17):
> > tee: Add some helpers to reduce boilerplate for tee client drivers
> > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > tee: Adapt documentation to cover recent additions
> > hwrng: optee - Make use of module_tee_client_driver()
> > hwrng: optee - Make use of tee bus methods
> > rtc: optee: Migrate to use tee specific driver registration function
> > rtc: optee: Make use of tee bus methods
> > efi: stmm: Make use of module_tee_client_driver()
> > efi: stmm: Make use of tee bus methods
> > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > firmware: arm_scmi: Make use of tee bus methods
> > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > firmware: tee_bnxt: Make use of tee bus methods
> > KEYS: trusted: Migrate to use tee specific driver registration
> > function
> > KEYS: trusted: Make use of tee bus methods
> > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > tpm/tpm_ftpm_tee: Make use of tee bus methods
> >
> > Documentation/driver-api/tee.rst | 18 +----
> > drivers/char/hw_random/optee-rng.c | 26 ++----
> > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > drivers/rtc/rtc-optee.c | 27 ++-----
> > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > include/linux/tee_drv.h | 12 +++
> > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > 10 files changed, 164 insertions(+), 138 deletions(-)
> >
> > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > --
> > 2.47.3
> >
>
> Thank you for the nice cleanup, Uwe.
>
> I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
>
> The branch is based on v6.19-rc1, and I'll try to keep it stable for
> others to depend on, if needed. Let's see if we can agree on taking
> the remaining patches via that branch.
6 and 7 can go through your branch.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Jens Wiklander @ 2025-12-18 16:29 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Uwe Kleine-König, Jonathan Corbet, Sumit Garg,
Olivia Mackall, Herbert Xu, Clément Léger,
Ard Biesheuvel, Maxime Coquelin, Alexandre Torgue, Sumit Garg,
Ilias Apalodimas, Jan Kiszka, Sudeep Holla, Christophe JAILLET,
Rafał Miłecki, Michael Chan, Pavan Chebbi,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Peter Huewe, op-tee,
linux-kernel, linux-doc, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi,
linux-mips, netdev, linux-integrity, keyrings,
linux-security-module, Jason Gunthorpe
In-Reply-To: <20251218135332f323fa91@mail.local>
On Thu, Dec 18, 2025 at 2:53 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 18/12/2025 08:21:27+0100, Jens Wiklander wrote:
> > Hi,
> >
> > On Mon, Dec 15, 2025 at 3:17 PM Uwe Kleine-König
> > <u.kleine-koenig@baylibre.com> wrote:
> > >
> > > Hello,
> > >
> > > the objective of this series is to make tee driver stop using callbacks
> > > in struct device_driver. These were superseded by bus methods in 2006
> > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > methods.")) but nobody cared to convert all subsystems accordingly.
> > >
> > > Here the tee drivers are converted. The first commit is somewhat
> > > unrelated, but simplifies the conversion (and the drivers). It
> > > introduces driver registration helpers that care about setting the bus
> > > and owner. (The latter is missing in all drivers, so by using these
> > > helpers the drivers become more correct.)
> > >
> > > v1 of this series is available at
> > > https://lore.kernel.org/all/cover.1765472125.git.u.kleine-koenig@baylibre.com
> > >
> > > Changes since v1:
> > >
> > > - rebase to v6.19-rc1 (no conflicts)
> > > - add tags received so far
> > > - fix whitespace issues pointed out by Sumit Garg
> > > - fix shutdown callback to shutdown and not remove
> > >
> > > As already noted in v1's cover letter, this series should go in during a
> > > single merge window as there are runtime warnings when the series is
> > > only applied partially. Sumit Garg suggested to apply the whole series
> > > via Jens Wiklander's tree.
> > > If this is done the dependencies in this series are honored, in case the
> > > plan changes: Patches #4 - #17 depend on the first two.
> > >
> > > Note this series is only build tested.
> > >
> > > Uwe Kleine-König (17):
> > > tee: Add some helpers to reduce boilerplate for tee client drivers
> > > tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
> > > tee: Adapt documentation to cover recent additions
> > > hwrng: optee - Make use of module_tee_client_driver()
> > > hwrng: optee - Make use of tee bus methods
> > > rtc: optee: Migrate to use tee specific driver registration function
> > > rtc: optee: Make use of tee bus methods
> > > efi: stmm: Make use of module_tee_client_driver()
> > > efi: stmm: Make use of tee bus methods
> > > firmware: arm_scmi: optee: Make use of module_tee_client_driver()
> > > firmware: arm_scmi: Make use of tee bus methods
> > > firmware: tee_bnxt: Make use of module_tee_client_driver()
> > > firmware: tee_bnxt: Make use of tee bus methods
> > > KEYS: trusted: Migrate to use tee specific driver registration
> > > function
> > > KEYS: trusted: Make use of tee bus methods
> > > tpm/tpm_ftpm_tee: Make use of tee specific driver registration
> > > tpm/tpm_ftpm_tee: Make use of tee bus methods
> > >
> > > Documentation/driver-api/tee.rst | 18 +----
> > > drivers/char/hw_random/optee-rng.c | 26 ++----
> > > drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
> > > drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
> > > drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
> > > drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
> > > drivers/rtc/rtc-optee.c | 27 ++-----
> > > drivers/tee/tee_core.c | 84 ++++++++++++++++++++
> > > include/linux/tee_drv.h | 12 +++
> > > security/keys/trusted-keys/trusted_tee.c | 17 ++--
> > > 10 files changed, 164 insertions(+), 138 deletions(-)
> > >
> > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > --
> > > 2.47.3
> > >
> >
> > Thank you for the nice cleanup, Uwe.
> >
> > I've applied patch 1-3 to the branch tee_bus_callback_for_6.20 in my
> > tree at https://git.kernel.org/pub/scm/linux/kernel/git/jenswi/linux-tee.git/
> >
> > The branch is based on v6.19-rc1, and I'll try to keep it stable for
> > others to depend on, if needed. Let's see if we can agree on taking
> > the remaining patches via that branch.
>
> 6 and 7 can go through your branch.
Good, I've added them to my branch now.
Thanks,
Jens
^ permalink raw reply
* Re: [RFC 00/11] Reintroduce Hornet LSM
From: ryan foster @ 2025-12-18 17:40 UTC (permalink / raw)
To: Blaise Boscaccy
Cc: James.Bottomley, akpm, bpf, corbet, dhowells, gnoack, jmorris,
linux-doc, linux-kernel, linux-security-module, linux, mic, paul,
serge
In-Reply-To: <CAHtS329aD5DyYSUkcuZXXjZKywqqTHe_1pA6uyaQH=Zwi9kydw@mail.gmail.com>
Hi Paul, Blaise,
Thanks for clarifying, that matches my understanding.
My read is Hornet's semantic goal is load-time provenance and
integrity: validate the user-generated inputs once the kernel has
constructed the final in-kernel program instance (post-relocation,
with any integrity-relevant referenced inputs accounted for), and
record that result so LSM/policy can make decisions based on a fact
the kernel can truthfully assert.
Attaching and linking are separate runtime policy/capability
questions. It makes sense that SELinux, IPE, or a BPF LSM program
could choose to gate attach/link based on whether Hornet recorded the
program as verified, but that enforcement is orthogonal to Hornet's
core integrity/provenance objective.
Thanks
On Thu, Dec 18, 2025 at 9:39 AM ryan foster <foster.ryan.r@gmail.com> wrote:
>
> Hi Paul, Blaise,
>
> Thanks for clarifying, that matches my understanding.
>
> My read is Hornet's semantic goal is load-time provenance and integrity: validate the user-generated inputs once the kernel has constructed the final in-kernel program instance (post-relocation, with any integrity-relevant referenced inputs accounted for), and record that result so LSM/policy can make decisions based on a fact the kernel can truthfully assert.
>
> Attaching and linking are separate runtime policy/capability questions. It makes sense that SELinux, IPE, or a BPF LSM program could choose to gate attach/link based on whether Hornet recorded the program as verified, but that enforcement is orthogonal to Hornet's core integrity/provenance objective.
>
> Thanks
>
>
> On Wed, Dec 17, 2025 at 11:33 AM Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>>
>> ryan foster <foster.ryan.r@gmail.com> writes:
>>
>> > Hi all,
>> >
>>
>> Hi Ryan,
>>
>> > I want to confirm I understand the current semantics, and specific issues
>> > this series is addressing.
>> >
>> > In the signed BPF two step flow, the LSM makes decisions using what is
>> > known at the time of run hooks. At load time, the only clear fact is "the
>> > loader is signed". However, if we really want integrity for "the final
>> > program that will execute after relocation, and any inputs as part of the
>> > contract, matches what was signed". The fact exists after loader runs, so
>> > the kernel could end up allowing and auditing based on the signed loader,
>> > even though it cannot yet truthfully say the runnable payload has been
>> > verified.
>> >
>>
>> Correct.
>>
>> > If this is the right understanding, perhaps we could consider a design that
>> > moves enforcement to the moment the program becomes effective. E.g. Load
>> > can create a program object, but it is inert by default. The kernel should
>> > only allow attach or link creation if the kernel has already recorded a
>> > verified record of the final relocated instruction stream plus
>> > referenced state for inputs, is included in the "integrity contract".
>> >
>> > If the referenced state is mutable, then either state must be frozen before
>> > the contract is verified, or any mutation must invalidate verified and
>> > force re-verification and a new policy decision. Otherwise the state is
>> > susceptible to TOCTOU issues.
>> >
>> > Is this the semantic goal Hortnet is aiming for, and is attack or link
>> > creation the intended enforcement point for the "cannot become effective
>> > until verified" rule, instead of trying to make a load time hook represent
>> > final payload verification?
>> >
>> > Thanks
>> >
>> > Regard, Ryan
>>
>>
>> The semantic goal for Hornet is to validate the provenance and integrity
>> of all the user-generated inputs when they are loaded into the kernel,
>> in order to allow users to make intelligent security decisions based
>> on that. IMO, attaching and linking are orthogonal run-time policy issues
>> that are seperate from provenance and data integrity concerns.
>>
>> Allowing or disallowing linking and attaching based on the completeness of
>> signature validation does make sense. That kind of decision would
>> probably be handled by selinux, IPE, or a custom BPF LSM program most
>> likely though.
>>
>> -blaise
^ permalink raw reply
* [PATCH v3 sec cap tests] security: Add KUnit tests for kuid_root_in_ns and vfsuid_root_in_currentns
From: Ryan Foster @ 2025-12-18 18:49 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-kernel, paul, serge, Ryan Foster
In-Reply-To: <20251204215610.148342-1-foster.ryan.r@gmail.com>
Add comprehensive KUnit tests for the namespace-related capability
functions that Serge Hallyn refactored in commit 9891d2f79a9f
("Clarify the rootid_owns_currentns").
The tests verify:
- Basic functionality: UID 0 in init namespace, invalid vfsuid, non-zero UIDs
- Actual namespace traversal: Creating user namespaces with different UID
mappings where uid 0 maps to different kuids (e.g., 1000, 2000, 3000)
- Hierarchy traversal: Testing multiple nested namespaces to verify
correct namespace hierarchy traversal
This addresses the feedback to "test the actual functionality" by creating
real user namespaces with different values for the namespace's uid 0, rather
than just basic input validation.
The test file is included at the end of commoncap.c when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, following the standard
kernel pattern (e.g., scsi_lib.c, ext4/mballoc.c). This allows tests to
access static functions in the same compilation unit without modifying
production code based on test configuration.
All 7 tests pass:
- test_vfsuid_root_in_currentns_init_ns
- test_vfsuid_root_in_currentns_invalid
- test_vfsuid_root_in_currentns_nonzero
- test_kuid_root_in_ns_init_ns_uid0
- test_kuid_root_in_ns_init_ns_nonzero
- test_kuid_root_in_ns_with_mapping
- test_kuid_root_in_ns_with_different_mappings
Updated MAINTAINER capabilities to include commoncap test
---
MAINTAINERS | 1 +
security/Kconfig | 17 +++
security/commoncap.c | 4 +
security/commoncap_test.c | 290 ++++++++++++++++++++++++++++++++++++++
4 files changed, 312 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c0030e126fc8..6f162c736dfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5682,6 +5682,7 @@ F: include/trace/events/capability.h
F: include/uapi/linux/capability.h
F: kernel/capability.c
F: security/commoncap.c
+F: security/commoncap_test.c
CAPELLA MICROSYSTEMS LIGHT SENSOR DRIVER
M: Kevin Tsai <ktsai@capellamicro.com>
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..c7b3f42ef875 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..3399535808fe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1521,3 +1521,7 @@ DEFINE_LSM(capability) = {
};
#endif /* CONFIG_SECURITY */
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+#include "commoncap_test.c"
+#endif
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..1088364a54e6
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/cred.h>
+#include <linux/mnt_idmapping.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/refcount.h>
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Functions are static in commoncap.c, but we can call them since we're
+ * included in the same compilation unit when tests are enabled.
+ */
+
+/**
+ * test_vfsuid_root_in_currentns_init_ns - Test vfsuid_root_in_currentns with init ns
+ *
+ * Verifies that UID 0 in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create UID 0 in init namespace */
+ kuid = KUIDT_INIT(0);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* In init namespace, UID 0 should own current namespace */
+ KUNIT_EXPECT_TRUE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_invalid - Test vfsuid_root_in_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_vfsuid_root_in_currentns_nonzero - Test vfsuid_root_in_currentns with non-zero UID
+ *
+ * Verifies that a non-zero UID correctly returns false.
+ *
+ * @test: KUnit test context
+ */
+static void test_vfsuid_root_in_currentns_nonzero(struct kunit *test)
+{
+ vfsuid_t vfsuid;
+ kuid_t kuid;
+
+ /* Create a non-zero UID */
+ kuid = KUIDT_INIT(1000);
+ vfsuid = VFSUIDT_INIT(kuid);
+
+ /* Non-zero UID should return false */
+ KUNIT_EXPECT_FALSE(test, vfsuid_root_in_currentns(vfsuid));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_uid0 - Test kuid_root_in_ns with init namespace and UID 0
+ *
+ * Verifies that kuid_root_in_ns correctly identifies UID 0 in init namespace.
+ * This tests the core namespace traversal logic. In init namespace, UID 0
+ * maps to itself, so it should own the namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_uid0(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(0);
+ init_ns = &init_user_ns;
+
+ /* UID 0 should own init namespace */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_init_ns_nonzero - Test kuid_root_in_ns with init namespace and non-zero UID
+ *
+ * Verifies that kuid_root_in_ns correctly rejects non-zero UIDs in init namespace.
+ * Only UID 0 should own a namespace.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_init_ns_nonzero(struct kunit *test)
+{
+ kuid_t kuid;
+ struct user_namespace *init_ns;
+
+ kuid = KUIDT_INIT(1000);
+ init_ns = &init_user_ns;
+
+ /* Non-zero UID should not own namespace */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(kuid, init_ns));
+}
+
+/**
+ * create_test_user_ns_with_mapping - Create a mock user namespace with UID mapping
+ *
+ * Creates a minimal user namespace structure for testing where uid 0 in the
+ * namespace maps to a specific kuid in the parent namespace.
+ *
+ * @test: KUnit test context
+ * @parent_ns: Parent namespace (typically init_user_ns)
+ * @mapped_kuid: The kuid that uid 0 in this namespace maps to in parent
+ *
+ * Returns: Pointer to allocated namespace, or NULL on failure
+ */
+static struct user_namespace *create_test_user_ns_with_mapping(struct kunit *test,
+ struct user_namespace *parent_ns,
+ kuid_t mapped_kuid)
+{
+ struct user_namespace *ns;
+ struct uid_gid_extent extent;
+
+ /* Allocate a test namespace - use kzalloc to zero all fields */
+ ns = kunit_kzalloc(test, sizeof(*ns), GFP_KERNEL);
+ if (!ns)
+ return NULL;
+
+ /* Initialize basic namespace structure fields */
+ ns->parent = parent_ns;
+ ns->level = parent_ns ? parent_ns->level + 1 : 0;
+ ns->owner = mapped_kuid;
+ ns->group = KGIDT_INIT(0);
+
+ /* Initialize ns_common structure */
+ refcount_set(&ns->ns.__ns_ref, 1);
+ ns->ns.inum = 0; /* Mock inum */
+
+ /* Set up uid mapping: uid 0 in this namespace maps to mapped_kuid in parent
+ * Format: first (uid in ns) : lower_first (kuid in parent) : count
+ * So: uid 0 in ns -> kuid mapped_kuid in parent
+ * This means from_kuid(ns, mapped_kuid) returns 0
+ */
+ extent.first = 0; /* uid 0 in this namespace */
+ extent.lower_first = __kuid_val(mapped_kuid); /* maps to this kuid in parent */
+ extent.count = 1;
+
+ ns->uid_map.extent[0] = extent;
+ ns->uid_map.nr_extents = 1;
+
+ /* Set up gid mapping: gid 0 maps to gid 0 in parent (simplified) */
+ extent.first = 0;
+ extent.lower_first = 0;
+ extent.count = 1;
+
+ ns->gid_map.extent[0] = extent;
+ ns->gid_map.nr_extents = 1;
+
+ return ns;
+}
+
+/**
+ * test_kuid_root_in_ns_with_mapping - Test kuid_root_in_ns with namespace where uid 0
+ * maps to different kuid
+ *
+ * Creates a user namespace where uid 0 maps to kuid 1000 in the parent namespace.
+ * Verifies that kuid_root_in_ns correctly identifies kuid 1000 as owning the namespace.
+ *
+ * Note: kuid_root_in_ns walks up the namespace hierarchy, so it checks the current
+ * namespace first, then parent, then parent's parent, etc. So:
+ * - kuid 1000 owns test_ns because from_kuid(test_ns, 1000) == 0
+ * - kuid 0 also owns test_ns because from_kuid(init_user_ns, 0) == 0
+ * (checked in parent)
+ *
+ * This tests the actual functionality as requested: creating namespaces with
+ * different values for the namespace's uid 0.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_mapping(struct kunit *test)
+{
+ struct user_namespace *test_ns;
+ struct user_namespace *parent_ns;
+ kuid_t mapped_kuid, other_kuid;
+
+ parent_ns = &init_user_ns;
+ mapped_kuid = KUIDT_INIT(1000);
+ other_kuid = KUIDT_INIT(2000);
+
+ test_ns = create_test_user_ns_with_mapping(test, parent_ns, mapped_kuid);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_ns);
+
+ /* kuid 1000 should own test_ns because it maps to uid 0 in test_ns */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(mapped_kuid, test_ns));
+
+ /* kuid 0 should also own test_ns (checked via parent init_user_ns) */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), test_ns));
+
+ /* Other kuids should not own test_ns */
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(other_kuid, test_ns));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(500), test_ns));
+}
+
+/**
+ * test_kuid_root_in_ns_with_different_mappings - Test with multiple namespaces
+ *
+ * Creates multiple user namespaces with different UID mappings to verify
+ * that kuid_root_in_ns correctly handles different namespace hierarchies.
+ *
+ * Since kuid_root_in_ns walks up the hierarchy, kuids that map to 0 in init_user_ns
+ * will own all namespaces, while kuids that only map to 0 in specific namespaces
+ * will only own those namespaces and their children.
+ *
+ * @test: KUnit test context
+ */
+static void test_kuid_root_in_ns_with_different_mappings(struct kunit *test)
+{
+ struct user_namespace *ns1, *ns2, *ns3;
+
+ /* Create ns1 where uid 0 maps to kuid 1000 */
+ ns1 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(1000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns1);
+
+ /* Create ns2 where uid 0 maps to kuid 2000 */
+ ns2 = create_test_user_ns_with_mapping(test, &init_user_ns, KUIDT_INIT(2000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns2);
+
+ /* Create ns3 as a child of ns1 where uid 0 maps to kuid 3000 */
+ ns3 = create_test_user_ns_with_mapping(test, ns1, KUIDT_INIT(3000));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ns3);
+
+ /* Test ns1: kuid 1000 owns it, kuid 0 owns it (via parent), kuid 2000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns1));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns1));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns1));
+
+ /* Test ns2: kuid 2000 owns it, kuid 0 owns it (via parent), kuid 1000 does not */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns2));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns2));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns2));
+
+ /* Test ns3: kuid 3000 owns it, kuid 1000 owns it (via parent ns1),
+ * kuid 0 owns it (via init_user_ns), kuid 2000 does not
+ */
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(3000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(1000), ns3));
+ KUNIT_EXPECT_TRUE(test, kuid_root_in_ns(KUIDT_INIT(0), ns3));
+ KUNIT_EXPECT_FALSE(test, kuid_root_in_ns(KUIDT_INIT(2000), ns3));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_vfsuid_root_in_currentns_init_ns),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_invalid),
+ KUNIT_CASE(test_vfsuid_root_in_currentns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_uid0),
+ KUNIT_CASE(test_kuid_root_in_ns_init_ns_nonzero),
+ KUNIT_CASE(test_kuid_root_in_ns_with_mapping),
+ KUNIT_CASE(test_kuid_root_in_ns_with_different_mappings),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
--
2.52.0
^ permalink raw reply related
* Re: [RFC 08/11] security: Hornet LSM
From: Blaise Boscaccy @ 2025-12-18 21:26 UTC (permalink / raw)
To: Alexei Starovoitov, Linus Torvalds
Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James Bottomley,
David Howells, LSM List, open list:DOCUMENTATION, LKML, bpf
In-Reply-To: <CAADnVQJ1CRvTXBU771KaYzrx-vRaWF+k164DcFOqOsCxmuL+ig@mail.gmail.com>
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> On Wed, Dec 10, 2025 at 6:14 PM Blaise Boscaccy
> <bboscaccy@linux.microsoft.com> wrote:
>> +++ b/security/hornet/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +config SECURITY_HORNET
>> + bool "Hornet support"
>> + depends on SECURITY
>> + default n
>
> So you're disallowing this new LSM to be a module?
> That doesn't smell good.
>
>> +static int hornet_verify_hashes(struct hornet_maps *maps,
>> + struct hornet_parse_context *ctx)
>> +{
>> + int map_fd;
>> + u32 i;
>> + struct bpf_map *map;
>> + int err = 0;
>> + unsigned char hash[SHA256_DIGEST_SIZE];
>> +
>> + for (i = 0; i < ctx->hash_count; i++) {
>> + if (ctx->skips[i])
>> + continue;
>> +
>> + err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
>> + ctx->indexes[i] * sizeof(map_fd),
>> + sizeof(map_fd));
>
> As was pointed out several times earlier this is an obvious TOCTOU bug.
> An attacker can change this map_fd between LSM checks and later verifier use.
> All the "security" checks further are useless.
Thank you, Alexei, for pointing that out. I’ll ensure it’s addressed in
the next iteration.
>
>> + if (err < 0)
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + CLASS(fd, f)(map_fd);
>> + if (fd_empty(f))
>> + return LSM_INT_VERDICT_BADSIG;
>> + if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
>
> Ohh. So this is why this LSM has to be built-in.
> bpf_map_fops is bpf internal detail. It's not going to be exported.
> You cannot open code __bpf_map_get() and get away with it.
>
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + if (!map->frozen)
>> + return LSM_INT_VERDICT_BADSIG;
>> +
>> + map = fd_file(f)->private_data;
>> + map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
>
> This too. It's absolutely not ok for LSM to mess with bpf internal state.
>
> The whole LSM is one awful hack.
> The diff stat doesn't touch anything in the kernel/bpf/
> yet you're messing with bpf internals.
>
> Clearly, you guys want to merge this garbage through LSM tree.
> Make sure to keep my Nack when you send it during the merge window.
Sure thing. I'll include your Nacked-by: in future versions.
-blaise
^ permalink raw reply
* [PATCH v19] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-12-19 8:15 UTC (permalink / raw)
To: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Eric W. Biederman,
Adrian Reber, Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Alexey Dobriyan,
Jens Axboe, Paul Moore, Elena Reshetova, David Windsor,
Mateusz Guzik, YueHaibing, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, tiozhang, Penglei Jiang, Lorenzo Stoakes,
Adrian Ratiu, Ingo Molnar, Peter Zijlstra (Intel),
Cyrill Gorcunov, Eric Dumazet, Ryan Lee, apparmor, selinux
In-Reply-To: <GV2PPF74270EBEEE807D016A79FE7A2F463E4D6A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
This introduces signal->exec_bprm, which is used to
fix the case when at least one of the sibling threads
is traced, and therefore the trace process may be blocked
in ptrace_attach, but de_thread() will need to wait for the
tracer to continue execution.
Furthermore this introduces the new ptrace_may_access flag
PTRACE_MODE_BPRMCREDS, which informs the security engines
that the credentials in child->signal->exec_bprm shall be
checked in the ptrace_access_check callback.
The problem happens when a tracer tries to ptrace_attach
to a multi-threaded process, that does an execve in one of
the threads at the same time, without doing that in a forked
sub-process. That means: There is a race condition, when one
or more of the threads are already ptraced, but the thread
that invoked the execve is not yet traced. Now in this
case the execve locks the cred_guard_mutex and waits for
de_thread() to complete. But that waits for the traced
sibling threads to exit, and those have to wait for the
tracer to receive the exit signal, but the tracer cannot
call wait right now, because it is waiting for the ptrace
call to complete, and this will probably never happen.
The traced process and the tracer are now in a deadlock
situation, and can only be killed when this happens.
The solution is to detect this situation and allow
ptrace_attach to continue by temporarily releasing the
cred_guard_mutex, while de_thread() is still waiting for
traced zombies to be eventually released by the tracer.
In the case of the thread group leader we only have to wait
for the thread to become a zombie, which may also need
co-operation from the tracer due to PTRACE_O_TRACEEXIT.
When a tracer wants to ptrace_attach a task that already
is in execve, we simply retry the ptrace_may_access
check with the new PTRACE_MODE_BPRMCREDS flag to check
that the tracer has permission to trace the new credentials
and dumpability which are about to be used after execve
completes. If the ptrace_attach happens on a thread that
is a sibling-thread of the thread doing execve, it is
sufficient to check against the old credentials, as this
thread will be waited for, before the new credentials are
installed.
Other threads die quickly since the cred_guard_mutex is
released, but a deadly signal is already pending. In case
the mutex_lock_killable misses the signal, the non-zero
current->signal->exec_bprm makes sure they release the
mutex immediately and return with -ERESTARTNOINTR.
This means there is no API change, unlike the previous
version of this patch which was discussed here:
https://lore.kernel.org/lkml/b6537ae6-31b1-5c50-f32b-8b8332ace882@hotmail.de/
The disadvantage of that approach was, that it is a
user-visible API-change, but also that the debugger does
not know when to retry the PTRACE_ATTACH, in worst case
it will go into an endless loop, not knowing that a
waitpid and/or PTRACE_CONT is necessary to unblock the
traced process.
But the main reason why the overlapping lifetime of the
current and the new credentials is an improvement over the
previous version, is that the tracee could have escaped the
PTRACE_ATTACH if it is very short-lived, while now from
the tracer's perspective, everything looks as if the
PTRACE_ATTACH happened immediatly after the new executable
is loaded and just after the new credentials are committed.
See tools/testing/selftests/ptrace/vmaccess.c
for a test case that gets fixed by this change.
Note that since the test case was originally designed to
test the ptrace_attach returning an error in this situation,
the test expectation needed to be adjusted, to allow the
API to succeed at the first attempt.
Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
fs/exec.c | 89 +++++++++++---
fs/proc/base.c | 12 ++
include/linux/cred.h | 1 +
include/linux/ptrace.h | 1 +
include/linux/sched/signal.h | 20 +++-
kernel/cred.c | 30 ++++-
kernel/fork.c | 2 +-
kernel/ptrace.c | 29 ++++-
kernel/seccomp.c | 18 ++-
security/apparmor/lsm.c | 5 +-
security/commoncap.c | 5 +-
security/landlock/task.c | 7 +-
security/selinux/hooks.c | 7 +-
security/smack/smack_lsm.c | 5 +-
security/yama/yama_lsm.c | 11 +-
tools/testing/selftests/ptrace/vmaccess.c | 137 ++++++++++++++++++++--
16 files changed, 330 insertions(+), 49 deletions(-)
v10: Changes to previous version, make the PTRACE_ATTACH
return -EAGAIN, instead of execve return -ERESTARTSYS.
Added some lessions learned to the description.
v11: Check old and new credentials in PTRACE_ATTACH again without
changing the API.
Note: I got actually one response from an automatic checker to the v11 patch,
https://lore.kernel.org/lkml/202107121344.wu68hEPF-lkp@intel.com/
which is complaining about:
>> >> kernel/ptrace.c:425:26: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const *old_cred @@ got struct cred const [noderef] __rcu *real_cred @@
417 struct linux_binprm *bprm = task->signal->exec_bprm;
418 const struct cred *old_cred;
419 struct mm_struct *old_mm;
420
421 retval = down_write_killable(&task->signal->exec_update_lock);
422 if (retval)
423 goto unlock_creds;
424 task_lock(task);
> 425 old_cred = task->real_cred;
v12: Essentially identical to v11.
- Fixed a minor merge conflict in linux v5.17, and fixed the
above mentioned nit by adding __rcu to the declaration.
- re-tested the patch with all linux versions from v5.11 to v6.6
v10 was an alternative approach which did imply an API change.
But I would prefer to avoid such an API change.
The difficult part is getting the right dumpability flags assigned
before de_thread starts, hope you like this version.
If not, the v10 is of course also acceptable.
v13: Fixed duplicated Return section in function header of
is_dumpability_changed which was reported by the kernel test robot
v14: rebased to v6.7, refreshed and retested.
And added a more detailed description of the actual bug.
v15: rebased to v6.8-rc1, addressed some review comments.
Split the test case vmaccess into vmaccess1 and vmaccess2
to improve overall test coverage.
v16: rebased to 6.17-rc2, fixed some minor merge conflicts.
v17: avoid use of task->in_execve in ptrace_attach.
v18: Add some more comments, avoid the temporary impersonation of the
new credentials, and use instead a new option to ptrace_may_access.
All security engines have to handle this option, but the advantage is
that the engines could detect and maybe also deny the unsafe execve.
v19: This mainly improves the commit message and updates some comments.
Rebased and re-tested on v6.19-rc1.
Thanks
Bernd.
diff --git a/fs/exec.c b/fs/exec.c
index 9d5ebc9d15b0..db5c3b10c48a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -905,11 +905,13 @@ static int exec_mmap(struct mm_struct *mm)
return 0;
}
-static int de_thread(struct task_struct *tsk)
+static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
{
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
+ struct task_struct *t;
+ bool unsafe_execve_in_progress = false;
if (thread_group_empty(tsk))
goto no_thread_group;
@@ -932,6 +934,36 @@ static int de_thread(struct task_struct *tsk)
if (!thread_group_leader(tsk))
sig->notify_count--;
+ for_other_threads(tsk, t) {
+ if (unlikely(t->ptrace) &&
+ (t != tsk->group_leader || !t->exit_state)) {
+ unsafe_execve_in_progress = true;
+ break;
+ }
+ }
+
+ if (unlikely(unsafe_execve_in_progress)) {
+ /*
+ * Since the spin lock was acquired while holding the
+ * mutex, both should be unlocked in reverse sequence and
+ * the spin lock re-acquired after releasing the mutex.
+ */
+ spin_unlock_irq(lock);
+ /*
+ * Sibling threads are notified by the non-zero exec_bprm,
+ * that they have just been zapped, and the cred_guard_mutex
+ * is to be released by them immediately.
+ * The caller of ptrace_attach on the other hand is allowed
+ * to ptrace any additional sibling threads that may not yet
+ * have ben ptraced, but if the group_exec_task is being
+ * ptraced, an additional check has to be performed, that the
+ * tracer is allowed to ptrace the new exec credentials.
+ */
+ sig->exec_bprm = bprm;
+ mutex_unlock(&sig->cred_guard_mutex);
+ spin_lock_irq(lock);
+ }
+
while (sig->notify_count) {
__set_current_state(TASK_KILLABLE);
spin_unlock_irq(lock);
@@ -1021,6 +1053,11 @@ static int de_thread(struct task_struct *tsk)
release_task(leader);
}
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
sig->group_exec_task = NULL;
sig->notify_count = 0;
@@ -1032,6 +1069,11 @@ static int de_thread(struct task_struct *tsk)
return 0;
killed:
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
/* protects against exit_notify() and __exit_signal() */
read_lock(&tasklist_lock);
sig->group_exec_task = NULL;
@@ -1114,13 +1156,31 @@ int begin_new_exec(struct linux_binprm * bprm)
*/
trace_sched_prepare_exec(current, bprm);
+ /* If the binary is not readable then enforce mm->dumpable=0 */
+ would_dump(bprm, bprm->file);
+ if (bprm->have_execfd)
+ would_dump(bprm, bprm->executable);
+
+ /*
+ * Figure out dumpability. Note that this checking only of current
+ * is wrong, but userspace depends on it. This should be testing
+ * bprm->secureexec instead.
+ */
+ if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
+ is_dumpability_changed(current_cred(), bprm->cred) ||
+ !(uid_eq(current_euid(), current_uid()) &&
+ gid_eq(current_egid(), current_gid())))
+ set_dumpable(bprm->mm, suid_dumpable);
+ else
+ set_dumpable(bprm->mm, SUID_DUMP_USER);
+
/*
* Ensure all future errors are fatal.
*/
bprm->point_of_no_return = true;
/* Make this the only thread in the thread group */
- retval = de_thread(me);
+ retval = de_thread(me, bprm);
if (retval)
goto out;
/* see the comment in check_unsafe_exec() */
@@ -1144,11 +1204,6 @@ int begin_new_exec(struct linux_binprm * bprm)
if (retval)
goto out;
- /* If the binary is not readable then enforce mm->dumpable=0 */
- would_dump(bprm, bprm->file);
- if (bprm->have_execfd)
- would_dump(bprm, bprm->executable);
-
/*
* Release all of the old mmap stuff
*/
@@ -1210,18 +1265,6 @@ int begin_new_exec(struct linux_binprm * bprm)
me->sas_ss_sp = me->sas_ss_size = 0;
- /*
- * Figure out dumpability. Note that this checking only of current
- * is wrong, but userspace depends on it. This should be testing
- * bprm->secureexec instead.
- */
- if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
- !(uid_eq(current_euid(), current_uid()) &&
- gid_eq(current_egid(), current_gid())))
- set_dumpable(current->mm, suid_dumpable);
- else
- set_dumpable(current->mm, SUID_DUMP_USER);
-
perf_event_exec();
/*
@@ -1275,6 +1318,10 @@ int begin_new_exec(struct linux_binprm * bprm)
* cred_guard_mutex must be held at least to this point to prevent
* ptrace_attach() from altering our determination of the task's
* credentials; any time after this it may be unlocked.
+ * Note that de_thread() may temporarily release the cred_guard_mutex,
+ * but the credentials are pre-determined in that case and the ptrace
+ * access check guarantees, that the access permissions of the tracer
+ * are sufficient to trace the task also with the new credentials.
*/
security_bprm_committed_creds(bprm);
@@ -1360,6 +1407,10 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
return -ERESTARTNOINTR;
+ /*
+ * It is not necessary to check current->signal->exec_bprm here,
+ * because de_thread() has already an equivalent check.
+ */
bprm->cred = prepare_exec_creds();
if (likely(bprm->cred))
return 0;
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 4eec684baca9..84a525ca9e5b 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2838,6 +2838,18 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;
+ /*
+ * A fatal signal is guaranteed to be already pending in the
+ * unlikely event, that current->signal->exec_bprm happens
+ * to be non-zero here, so just release the mutex again
+ * and continue as if mutex_lock_interruptible did fail.
+ */
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ rv = -ERESTARTNOINTR;
+ goto out_free;
+ }
+
rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 343a140a6ba2..b243223cd24b 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -155,6 +155,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
extern struct cred *cred_alloc_blank(void);
extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void);
+extern bool is_dumpability_changed(const struct cred *, const struct cred *);
extern int commit_creds(struct cred *);
extern void abort_creds(struct cred *);
extern struct cred *prepare_kernel_cred(struct task_struct *);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 90507d4afcd6..dbd58a4807bc 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -64,6 +64,7 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
#define PTRACE_MODE_NOAUDIT 0x04
#define PTRACE_MODE_FSCREDS 0x08
#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_BPRMCREDS 0x20
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 7d6449982822..6850e971dfff 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -241,10 +241,28 @@ struct signal_struct {
struct mm_struct *oom_mm; /* recorded mm when the thread group got
* killed by the oom killer */
+ struct linux_binprm *exec_bprm; /* Used to check ptrace_may_access
+ * against new credentials while
+ * de_thread is waiting for other
+ * traced threads to terminate.
+ * Set while de_thread is executing.
+ * The cred_guard_mutex is released
+ * after de_thread() has called
+ * zap_other_threads(), therefore
+ * a fatal signal is guaranteed to be
+ * already pending in the unlikely
+ * event, that
+ * current->signal->exec_bprm happens
+ * to be non-zero after the
+ * cred_guard_mutex was acquired.
+ */
+
struct mutex cred_guard_mutex; /* guard against foreign influences on
* credential calculations
* (notably. ptrace)
- * Deprecated do not use in new code.
+ * Held while execve runs, except when
+ * a sibling thread is being traced.
+ * Not recommended to use in new code.
* Use exec_update_lock instead.
*/
struct rw_semaphore exec_update_lock; /* Held while task_struct is
diff --git a/kernel/cred.c b/kernel/cred.c
index a6f686b30da1..08e73a6fc123 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -351,6 +351,30 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
return false;
}
+/**
+ * is_dumpability_changed - Will changing creds affect dumpability?
+ * @old: The old credentials.
+ * @new: The new credentials.
+ *
+ * If the @new credentials have no elevated privileges compared to the
+ * @old credentials, the task may remain dumpable. Otherwise we have
+ * to mark the task as undumpable to avoid information leaks from higher
+ * to lower privilege domains.
+ *
+ * Return: True if the task will become undumpable.
+ */
+bool is_dumpability_changed(const struct cred *old, const struct cred *new)
+{
+ if (!uid_eq(old->euid, new->euid) ||
+ !gid_eq(old->egid, new->egid) ||
+ !uid_eq(old->fsuid, new->fsuid) ||
+ !gid_eq(old->fsgid, new->fsgid) ||
+ !cred_cap_issubset(old, new))
+ return true;
+
+ return false;
+}
+
/**
* commit_creds - Install new credentials upon the current task
* @new: The credentials to be assigned
@@ -379,11 +403,7 @@ int commit_creds(struct cred *new)
get_cred(new); /* we will require a ref for the subj creds too */
/* dumpability changes */
- if (!uid_eq(old->euid, new->euid) ||
- !gid_eq(old->egid, new->egid) ||
- !uid_eq(old->fsuid, new->fsuid) ||
- !gid_eq(old->fsgid, new->fsgid) ||
- !cred_cap_issubset(old, new)) {
+ if (is_dumpability_changed(old, new)) {
if (task->mm)
set_dumpable(task->mm, suid_dumpable);
task->pdeath_signal = 0;
diff --git a/kernel/fork.c b/kernel/fork.c
index b1f3915d5f8e..aa06aa7051b2 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1393,7 +1393,7 @@ struct mm_struct *mm_access(struct task_struct *task, unsigned int mode)
struct mm_struct *mm;
int err;
- err = down_read_killable(&task->signal->exec_update_lock);
+ err = down_read_killable(&task->signal->exec_update_lock);
if (err)
return ERR_PTR(err);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 392ec2f75f01..be8bdcc71d08 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -20,6 +20,7 @@
#include <linux/pagemap.h>
#include <linux/ptrace.h>
#include <linux/security.h>
+#include <linux/binfmts.h>
#include <linux/signal.h>
#include <linux/uio.h>
#include <linux/audit.h>
@@ -285,6 +286,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
return -EPERM;
}
+ if ((mode & PTRACE_MODE_BPRMCREDS) && !task->signal->exec_bprm) {
+ WARN(1, "denying ptrace access check with PTRACE_MODE_BPRMCREDS\n");
+ return -EPERM;
+ }
+
/* May we inspect the given task?
* This check is used both for attaching with ptrace
* and for allowing access to sensitive information in /proc.
@@ -313,7 +319,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
caller_uid = cred->uid;
caller_gid = cred->gid;
}
- tcred = __task_cred(task);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ tcred = task->signal->exec_bprm->cred;
+ else
+ tcred = __task_cred(task);
if (uid_eq(caller_uid, tcred->euid) &&
uid_eq(caller_uid, tcred->suid) &&
uid_eq(caller_uid, tcred->uid) &&
@@ -337,7 +346,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
* Pairs with a write barrier in commit_creds().
*/
smp_rmb();
- mm = task->mm;
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ mm = task->signal->exec_bprm->mm;
+ else
+ mm = task->mm;
if (mm &&
((get_dumpable(mm) != SUID_DUMP_USER) &&
!ptrace_has_cap(mm->user_ns, mode)))
@@ -451,6 +463,14 @@ static int ptrace_attach(struct task_struct *task, long request,
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
if (retval)
return retval;
+
+ if (unlikely(task == task->signal->group_exec_task)) {
+ retval = __ptrace_may_access(task,
+ PTRACE_MODE_ATTACH_REALCREDS |
+ PTRACE_MODE_BPRMCREDS);
+ if (retval)
+ return retval;
+ }
}
scoped_guard (write_lock_irq, &tasklist_lock) {
@@ -488,6 +508,10 @@ static int ptrace_traceme(void)
{
int ret = -EPERM;
+ if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
+ return -ERESTARTNOINTR;
+
+ /* It is not necessary to check current->signal->exec_bprm here. */
write_lock_irq(&tasklist_lock);
/* Are we already being traced? */
if (!current->ptrace) {
@@ -503,6 +527,7 @@ static int ptrace_traceme(void)
}
}
write_unlock_irq(&tasklist_lock);
+ mutex_unlock(¤t->signal->cred_guard_mutex);
return ret;
}
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 25f62867a16d..6e4ff108faa0 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -2010,9 +2010,21 @@ static long seccomp_set_mode_filter(unsigned int flags,
* Make sure we cannot change seccomp or nnp state via TSYNC
* while another thread is in the middle of calling exec.
*/
- if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
- mutex_lock_killable(¤t->signal->cred_guard_mutex))
- goto out_put_fd;
+ if (flags & SECCOMP_FILTER_FLAG_TSYNC) {
+ if (mutex_lock_killable(¤t->signal->cred_guard_mutex))
+ goto out_put_fd;
+
+ /*
+ * A fatal signal is guaranteed to be already pending in the
+ * unlikely event, that current->signal->exec_bprm happens
+ * to be non-zero here, so just release the mutex again
+ * and continue as if mutex_lock_killable did fail.
+ */
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ goto out_put_fd;
+ }
+ }
spin_lock_irq(¤t->sighand->siglock);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index a87cd60ed206..12da76db0bab 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -130,7 +130,10 @@ static int apparmor_ptrace_access_check(struct task_struct *child,
int error;
bool needput;
- cred = get_task_cred(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ cred = get_cred(child->signal->exec_bprm->cred);
+ else
+ cred = get_task_cred(child);
tracee = cred_label(cred); /* ref count on cred */
tracer = __begin_current_label_crit_section(&needput);
error = aa_may_ptrace(current_cred(), tracer, cred, tracee,
diff --git a/security/commoncap.c b/security/commoncap.c
index 8a23dfab7fac..b97368ba9669 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -169,7 +169,10 @@ int cap_ptrace_access_check(struct task_struct *child, unsigned int mode)
rcu_read_lock();
cred = current_cred();
- child_cred = __task_cred(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ child_cred = child->signal->exec_bprm->cred;
+ else
+ child_cred = __task_cred(child);
if (mode & PTRACE_MODE_FSCREDS)
caller_caps = &cred->cap_effective;
else
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..46ce9b6e4728 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -17,6 +17,7 @@
#include <linux/rcupdate.h>
#include <linux/sched.h>
#include <linux/sched/signal.h>
+#include <linux/binfmts.h>
#include <net/af_unix.h>
#include <net/sock.h>
@@ -96,7 +97,11 @@ static int hook_ptrace_access_check(struct task_struct *const child,
scoped_guard(rcu)
{
- child_dom = landlock_get_task_domain(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ child_dom = landlock_cred(child->signal->
+ exec_bprm->cred)->domain;
+ else
+ child_dom = landlock_get_task_domain(child);
err = domain_ptrace(parent_subject->domain, child_dom);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d053ce562370..fb46c309c31f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2113,7 +2113,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
unsigned int mode)
{
u32 sid = current_sid();
- u32 csid = task_sid_obj(child);
+ u32 csid;
+
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ csid = cred_sid(child->signal->exec_bprm->cred);
+ else
+ csid = task_sid_obj(child);
if (mode & PTRACE_MODE_READ)
return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a0bd4919a9d9..347df3c7445e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -476,7 +476,10 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
{
struct smack_known *skp;
- skp = smk_of_task_struct_obj(ctp);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ skp = smk_of_task(smack_cred(ctp->signal->exec_bprm->cred));
+ else
+ skp = smk_of_task_struct_obj(ctp);
return smk_ptrace_rule_check(current, skp, mode, __func__);
}
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 38b21ee0c560..dddc6eccfc9b 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -11,6 +11,7 @@
#include <linux/lsm_hooks.h>
#include <linux/sysctl.h>
#include <linux/ptrace.h>
+#include <linux/binfmts.h>
#include <linux/prctl.h>
#include <linux/ratelimit.h>
#include <linux/workqueue.h>
@@ -363,13 +364,19 @@ static int yama_ptrace_access_check(struct task_struct *child,
rc = -EPERM;
if (!rc && !task_is_descendant(current, child) &&
!ptracer_exception_found(current, child) &&
- !ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
+ !ns_capable(mode & PTRACE_MODE_BPRMCREDS ?
+ child->signal->exec_bprm->cred->user_ns :
+ __task_cred(child)->user_ns,
+ CAP_SYS_PTRACE))
rc = -EPERM;
rcu_read_unlock();
break;
case YAMA_SCOPE_CAPABILITY:
rcu_read_lock();
- if (!ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
+ if (!ns_capable(mode & PTRACE_MODE_BPRMCREDS ?
+ child->signal->exec_bprm->cred->user_ns :
+ __task_cred(child)->user_ns,
+ CAP_SYS_PTRACE))
rc = -EPERM;
rcu_read_unlock();
break;
diff --git a/tools/testing/selftests/ptrace/vmaccess.c b/tools/testing/selftests/ptrace/vmaccess.c
index 3801b5831527..7fa39be16827 100644
--- a/tools/testing/selftests/ptrace/vmaccess.c
+++ b/tools/testing/selftests/ptrace/vmaccess.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright (c) 2020 Bernd Edlinger <bernd.edlinger@hotmail.de>
+ * Copyright (c) 2020-2025 Bernd Edlinger <bernd.edlinger@hotmail.de>
* All rights reserved.
*
* Check whether /proc/$pid/mem can be accessed without causing deadlocks
@@ -14,6 +14,7 @@
#include <signal.h>
#include <unistd.h>
#include <sys/ptrace.h>
+#include <sys/syscall.h>
static void *thread(void *arg)
{
@@ -23,7 +24,7 @@ static void *thread(void *arg)
TEST(vmaccess)
{
- int f, pid = fork();
+ int s, f, pid = fork();
char mm[64];
if (!pid) {
@@ -31,19 +32,42 @@ TEST(vmaccess)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("true", "true", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
sprintf(mm, "/proc/%d/mem", pid);
+ /* deadlock did happen here */
f = open(mm, O_RDONLY);
ASSERT_GE(f, 0);
close(f);
- f = kill(pid, SIGCONT);
- ASSERT_EQ(f, 0);
+ f = waitpid(-1, &s, WNOHANG);
+ ASSERT_NE(f, -1);
+ ASSERT_NE(f, 0);
+ ASSERT_NE(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 0);
+ f = waitpid(-1, &s, 0);
+ ASSERT_EQ(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ f = waitpid(-1, NULL, 0);
+ ASSERT_EQ(f, -1);
+ ASSERT_EQ(errno, ECHILD);
}
-TEST(attach)
+/*
+ * Same test as previous, except that
+ * we try to ptrace the group leader,
+ * which is about to call execve,
+ * when the other thread is already ptraced.
+ * This exercises the code in de_thread
+ * where it is waiting inside the
+ * while (sig->notify_count) {
+ * loop.
+ */
+TEST(attach1)
{
int s, k, pid = fork();
@@ -52,19 +76,76 @@ TEST(attach)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("sleep", "sleep", "2", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
+ /* deadlock may happen here */
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
- ASSERT_EQ(errno, EAGAIN);
- ASSERT_EQ(k, -1);
+ ASSERT_EQ(k, 0);
k = waitpid(-1, &s, WNOHANG);
ASSERT_NE(k, -1);
ASSERT_NE(k, 0);
ASSERT_NE(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
ASSERT_EQ(WEXITSTATUS(s), 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ k = waitpid(-1, NULL, 0);
+ ASSERT_EQ(k, -1);
+ ASSERT_EQ(errno, ECHILD);
+}
+
+/*
+ * Same test as previous, except that
+ * the group leader is ptraced first,
+ * but this time with PTRACE_O_TRACEEXIT,
+ * and the thread that does execve is
+ * not yet ptraced. This exercises the
+ * code block in de_thread where the
+ * if (!thread_group_leader(tsk)) {
+ * is executed and enters a wait state.
+ */
+static long thread2_tid;
+static void *thread2(void *arg)
+{
+ thread2_tid = syscall(__NR_gettid);
+ sleep(2);
+ execlp("false", "false", NULL);
+ return NULL;
+}
+
+TEST(attach2)
+{
+ int s, k, pid = fork();
+
+ if (!pid) {
+ pthread_t pt;
+
+ pthread_create(&pt, NULL, thread2, NULL);
+ pthread_join(pt, NULL);
+ return;
+ }
+
sleep(1);
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
ASSERT_EQ(k, 0);
@@ -72,12 +153,46 @@ TEST(attach)
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFSTOPPED(s), 1);
ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
- k = ptrace(PTRACE_DETACH, pid, 0L, 0L);
+ k = ptrace(PTRACE_SETOPTIONS, pid, 0L, PTRACE_O_TRACEEXIT);
+ ASSERT_EQ(k, 0);
+ thread2_tid = ptrace(PTRACE_PEEKDATA, pid, &thread2_tid, 0L);
+ ASSERT_NE(thread2_tid, -1);
+ ASSERT_NE(thread2_tid, 0);
+ ASSERT_NE(thread2_tid, pid);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ sleep(2);
+ /* deadlock may happen here */
+ k = ptrace(PTRACE_ATTACH, thread2_tid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
ASSERT_EQ(k, 0);
k = waitpid(-1, &s, 0);
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
- ASSERT_EQ(WEXITSTATUS(s), 0);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
k = waitpid(-1, NULL, 0);
ASSERT_EQ(k, -1);
ASSERT_EQ(errno, ECHILD);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v8 05/12] tpm2-sessions: Define TPM2_NAME_MAX_SIZE
From: Jonathan McDowell @ 2025-12-19 9:32 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216092147.2326606-6-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:39AM +0200, Jarkko Sakkinen wrote:
>Define TPM2_NAME_MAX_SIZE, which describes the maximum size for hashes
>encoded as TPMT_HA, which the prime identifier used for persistent and
>transient keys in TPM2 protocol.
>
>Set its value to 'SHA512_DIGEST_SIZE + 2', as SHA512 has the largest
>digest size of the algorithms in TCG algorithm repository.
>
>In additionl, rename TPM2_NAME_SIZE as TPM2_NULL_NAME_SIZE in order to
>avoid any possible confusion.
One minor capitalisation nit, otherwise:
Reviewed-by: Jonathan McDowell <noodles@meta.com>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>v6:
>- Rewrote the commit message.
>v2:
>- Rename TPM2_NAME_SIZE as TPM2_NULL_NAME_SIZE.
>---
> drivers/char/tpm/tpm-sysfs.c | 2 +-
> drivers/char/tpm/tpm2-sessions.c | 2 +-
> include/linux/tpm.h | 37 +++++++++++++++++++++-----------
> 3 files changed, 27 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>index 94231f052ea7..4a6a27ee295d 100644
>--- a/drivers/char/tpm/tpm-sysfs.c
>+++ b/drivers/char/tpm/tpm-sysfs.c
>@@ -314,7 +314,7 @@ static ssize_t null_name_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> struct tpm_chip *chip = to_tpm_chip(dev);
>- int size = TPM2_NAME_SIZE;
>+ int size = TPM2_NULL_NAME_SIZE;
>
> bin2hex(buf, chip->null_key_name, size);
> size *= 2;
>diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
>index 4149379665c4..525b8622d1c3 100644
>--- a/drivers/char/tpm/tpm2-sessions.c
>+++ b/drivers/char/tpm/tpm2-sessions.c
>@@ -137,7 +137,7 @@ struct tpm2_auth {
> * we must compute and remember
> */
> u32 name_h[AUTH_MAX_NAMES];
>- u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
>+ u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
> };
>
> #ifdef CONFIG_TCG_TPM2_HMAC
>diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>index 202da079d500..e10f2096eae7 100644
>--- a/include/linux/tpm.h
>+++ b/include/linux/tpm.h
>@@ -27,9 +27,33 @@
>
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>
>+/*
>+ * SHA-512 is, as of today, the largest digest in the TCG algorithm repository.
>+ */
> #define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
>+
>+/*
>+ * A TPM name digest i.e., TPMT_HA, is a concatenation of TPM_ALG_ID of the
>+ * name algorithm and hash of TPMT_PUBLIC.
>+ */
>+#define TPM2_MAX_NAME_SIZE (TPM2_MAX_DIGEST_SIZE + 2)
>+
>+/*
>+ * The maximum number of PCR banks.
>+ */
> #define TPM2_MAX_PCR_BANKS 8
>
>+/*
>+ * fixed define for the size of a name. This is actually HASHALG size
"Fixed define".
>+ * plus 2, so 32 for SHA256
>+ */
>+#define TPM2_NULL_NAME_SIZE 34
>+
>+/*
>+ * The maximum size for an object context
>+ */
>+#define TPM2_MAX_CONTEXT_SIZE 4096
>+
> struct tpm_chip;
> struct trusted_key_payload;
> struct trusted_key_options;
>@@ -139,17 +163,6 @@ struct tpm_chip_seqops {
> /* fixed define for the curve we use which is NIST_P256 */
> #define EC_PT_SZ 32
>
>-/*
>- * fixed define for the size of a name. This is actually HASHALG size
>- * plus 2, so 32 for SHA256
>- */
>-#define TPM2_NAME_SIZE 34
>-
>-/*
>- * The maximum size for an object context
>- */
>-#define TPM2_MAX_CONTEXT_SIZE 4096
>-
> struct tpm_chip {
> struct device dev;
> struct device devs;
>@@ -211,7 +224,7 @@ struct tpm_chip {
> /* saved context for NULL seed */
> u8 null_key_context[TPM2_MAX_CONTEXT_SIZE];
> /* name of NULL seed */
>- u8 null_key_name[TPM2_NAME_SIZE];
>+ u8 null_key_name[TPM2_NULL_NAME_SIZE];
> u8 null_ec_key_x[EC_PT_SZ];
> u8 null_ec_key_y[EC_PT_SZ];
> struct tpm2_auth *auth;
>--
>2.39.5
>
>
J.
--
Why do I get the feeling I'm going to regret this?
This .sig brought to you by the letter S and the number 50
Product of the Republic of HuggieTag
^ permalink raw reply
* Re: [PATCH v8 07/12] KEYS: trusted: Remove dead branch from tpm2_unseal_cmd
From: Jonathan McDowell @ 2025-12-19 9:37 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216092147.2326606-8-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:41AM +0200, Jarkko Sakkinen wrote:
>TPM2_Unseal requires TPM2_ST_SESSIONS, and tpm2_unseal_cmd() always does
>set up either password or HMAC session.
>
>Remove the branch in tpm2_unseal_cmd() conditionally setting
>TPM2_ST_NO_SESSIONS. It is faulty but luckily it is never exercised at
>run-time, and thus does not cause regressions.
Reviewed-by: Jonathan McDowell <noodles@meta.com>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
> security/keys/trusted-keys/trusted_tpm2.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
>diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>index d3a5c5f2b926..3666e3e48eab 100644
>--- a/security/keys/trusted-keys/trusted_tpm2.c
>+++ b/security/keys/trusted-keys/trusted_tpm2.c
>@@ -451,10 +451,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> struct trusted_key_options *options,
> u32 blob_handle)
> {
>- struct tpm_header *head;
> struct tpm_buf buf;
> u16 data_len;
>- int offset;
> u8 *data;
> int rc;
>
>@@ -495,14 +493,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> tpm_buf_append_u16(&buf, options->blobauth_len);
> tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
>
>- if (tpm2_chip_auth(chip)) {
>+ if (tpm2_chip_auth(chip))
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
>- } else {
>- offset = buf.handles * 4 + TPM_HEADER_SIZE;
>- head = (struct tpm_header *)buf.data;
>- if (tpm_buf_length(&buf) == offset)
>- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
>- }
> }
>
> rc = tpm_buf_fill_hmac_session(chip, &buf);
>--
>2.39.5
>
>
J.
--
"Never test for an error condition you don't know how to handle."
--Steinbach
^ permalink raw reply
* Re: [PATCH v8 04/12] tpm: Change tpm_get_random() opportunistic
From: Jonathan McDowell @ 2025-12-19 9:42 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, David S . Miller, Herbert Xu, Eric Biggers,
Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216092147.2326606-5-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:38AM +0200, Jarkko Sakkinen wrote:
>hwrng framework does not have a requirement that the all bytes requested
>need to be provided. By enforcing such a requirement internally, TPM driver
>can cause unpredictability in latency, as a single tpm_get_random() call
>can result multiple TPM commands.
>
>Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have
>significant effect to the system latency.
>
>Thus, send TPM command only once and return bytes received instead of
>committing to the number of requested bytes.
Function comment for tpm_get_random needs updated as well, as it
currently says "until all of the @max bytes have been received", which
is no longer true with this patch. With that:
Reviewed-by: Jonathan McDowell <noodles@meta.com>
>Cc: David S. Miller <davem@davemloft.net>
>Cc: Herbert Xu <herbert@gondor.apana.org.au>
>Cc: Eric Biggers <ebiggers@kernel.org>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>v7:
>- Given that hwrng is now only caller for tpm_get_random(), remove the
> wait parameter.
>v4:
>- Fixed grammar mistakes.
>---
> drivers/char/tpm/tpm-interface.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>index d157be738612..677dcef05dfb 100644
>--- a/drivers/char/tpm/tpm-interface.c
>+++ b/drivers/char/tpm/tpm-interface.c
>@@ -626,10 +626,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> */
> int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> {
>- u32 num_bytes = max;
>- u8 *out_ptr = out;
>- int retries = 5;
>- int total = 0;
> int rc;
>
> if (!out || !max || max > TPM_MAX_RNG_DATA)
>@@ -646,28 +642,14 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> rc = tpm2_start_auth_session(chip);
> if (rc)
> return rc;
>- }
>-
>- do {
>- if (chip->flags & TPM_CHIP_FLAG_TPM2)
>- rc = tpm2_get_random(chip, out_ptr, num_bytes);
>- else
>- rc = tpm1_get_random(chip, out_ptr, num_bytes);
>-
>- if (rc < 0)
>- goto err;
>-
>- out_ptr += rc;
>- total += rc;
>- num_bytes -= rc;
>- } while (retries-- && total < max);
>
>- tpm_put_ops(chip);
>- return total ? total : -EIO;
>+ rc = tpm2_get_random(chip, out, max);
>+ } else {
>+ rc = tpm1_get_random(chip, out, max);
>+ }
>
>-err:
> tpm_put_ops(chip);
>- return rc;
>+ return rc != 0 ? rc : -EIO;
> }
> EXPORT_SYMBOL_GPL(tpm_get_random);
>
>--
>2.39.5
>
>
J.
--
... We are talking one charming motherf**king pig.
^ permalink raw reply
* Re: [PATCH v8 02/12] KEYS: trusted: Use get_random_bytes_wait() instead of tpm_get_random()
From: Jonathan McDowell @ 2025-12-19 9:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Eric Biggers, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20251216092147.2326606-3-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:36AM +0200, Jarkko Sakkinen wrote:
>Substitute remaining tpm_get_random() calls in trusted_tpm1.c with
>get_random_bytes_wait() thus aligning random number generation for TPM 1.2
>with the removal of '.get_random' callback.
Had to double check we wouldn't end up not getting all the randomness we
ask for, but get_random_bytes_wait does indeed DTRT.
Reviewed-by: Jonathan McDowell <noodles@meta.com>
>Cc: Eric Biggers <ebiggers@kernel.org>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
> security/keys/trusted-keys/trusted_tpm1.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
>diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
>index 7ce7e31bcdfb..3d75bb6f9689 100644
>--- a/security/keys/trusted-keys/trusted_tpm1.c
>+++ b/security/keys/trusted-keys/trusted_tpm1.c
>@@ -371,13 +371,10 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> unsigned char ononce[TPM_NONCE_SIZE];
> int ret;
>
>- ret = tpm_get_random(chip, ononce, TPM_NONCE_SIZE);
>+ ret = get_random_bytes_wait(ononce, TPM_NONCE_SIZE);
> if (ret < 0)
> return ret;
>
>- if (ret != TPM_NONCE_SIZE)
>- return -EIO;
>-
> tpm_buf_reset(tb, TPM_TAG_RQU_COMMAND, TPM_ORD_OSAP);
> tpm_buf_append_u16(tb, type);
> tpm_buf_append_u32(tb, handle);
>@@ -464,15 +461,10 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> memcpy(td->xorwork + SHA1_DIGEST_SIZE, sess.enonce, SHA1_DIGEST_SIZE);
> sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash);
>
>- ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
>+ ret = get_random_bytes_wait(td->nonceodd, TPM_NONCE_SIZE);
> if (ret < 0)
> goto out;
>
>- if (ret != TPM_NONCE_SIZE) {
>- ret = -EIO;
>- goto out;
>- }
>-
> ordinal = htonl(TPM_ORD_SEAL);
> datsize = htonl(datalen);
> pcrsize = htonl(pcrinfosize);
>@@ -575,14 +567,10 @@ static int tpm_unseal(struct tpm_buf *tb,
> }
>
> ordinal = htonl(TPM_ORD_UNSEAL);
>- ret = tpm_get_random(chip, nonceodd, TPM_NONCE_SIZE);
>+ ret = get_random_bytes_wait(nonceodd, TPM_NONCE_SIZE);
> if (ret < 0)
> return ret;
>
>- if (ret != TPM_NONCE_SIZE) {
>- pr_info("tpm_get_random failed (%d)\n", ret);
>- return -EIO;
>- }
> ret = TSS_authhmac(authdata1, keyauth, TPM_NONCE_SIZE,
> enonce1, nonceodd, cont, sizeof(uint32_t),
> &ordinal, bloblen, blob, 0, 0);
>--
>2.39.5
>
>
J.
--
Web [ Avoid temporary variables and strange women. ]
site: https:// [ ] Made by
www.earth.li/~noodles/ [ ] HuggieTag 0.0.24
^ permalink raw reply
* Re: [PATCH v8 03/12] tpm: Orchestrate TPM commands in tpm_get_random()
From: Jonathan McDowell @ 2025-12-19 9:43 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Eric Biggers, Peter Huewe, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20251216092147.2326606-4-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 11:21:37AM +0200, Jarkko Sakkinen wrote:
>tpm1_get_random() and tpm2_get_random() contain duplicate orchestration
>code. Consolidate orchestration to tpm_get_random().
Does keeping the respective tpm1/tpm2 bits in their -cmd.c files not
make more sense, still allowing for the consolidation of orchestration
in tpm_get_random?
>Cc: Eric Biggers <ebiggers@kernel.org>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
> drivers/char/tpm/tpm-interface.c | 175 +++++++++++++++++++++++++++++--
> drivers/char/tpm/tpm.h | 2 -
> drivers/char/tpm/tpm1-cmd.c | 69 ------------
> drivers/char/tpm/tpm2-cmd.c | 104 ------------------
> 4 files changed, 164 insertions(+), 186 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>index f745a098908b..d157be738612 100644
>--- a/drivers/char/tpm/tpm-interface.c
>+++ b/drivers/char/tpm/tpm-interface.c
>@@ -26,7 +26,7 @@
> #include <linux/suspend.h>
> #include <linux/freezer.h>
> #include <linux/tpm_eventlog.h>
>-
>+#include <linux/tpm_command.h>
> #include "tpm.h"
>
> /*
>@@ -486,19 +486,153 @@ int tpm_pm_resume(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(tpm_pm_resume);
>
>+struct tpm1_get_random_out {
>+ __be32 rng_data_len;
>+ u8 rng_data[TPM_MAX_RNG_DATA];
>+} __packed;
>+
>+static int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>+{
>+ struct tpm1_get_random_out *resp;
>+ struct tpm_buf buf;
>+ u32 recd;
>+ int rc;
>+
>+ if (!out || !max || max > TPM_MAX_RNG_DATA)
>+ return -EINVAL;
>+
>+ rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GETRANDOM);
>+ if (rc)
>+ return rc;
>+
>+ tpm_buf_append_u32(&buf, max);
>+
>+ rc = tpm_transmit_cmd(chip, &buf, sizeof(resp->rng_data_len), "TPM_GetRandom");
>+ if (rc) {
>+ if (rc > 0)
>+ rc = -EIO;
>+ goto err;
>+ }
>+
>+ resp = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
>+
>+ recd = be32_to_cpu(resp->rng_data_len);
>+ if (recd > max) {
>+ rc = -EIO;
>+ goto err;
>+ }
>+
>+ if (buf.length < TPM_HEADER_SIZE + sizeof(resp->rng_data_len) + recd) {
>+ rc = -EIO;
>+ goto err;
>+ }
>+
>+ memcpy(out, resp->rng_data, recd);
>+ tpm_buf_destroy(&buf);
>+ return recd;
>+
>+err:
>+ tpm_buf_destroy(&buf);
>+ return rc;
>+}
>+
>+struct tpm2_get_random_out {
>+ __be16 size;
>+ u8 buffer[TPM_MAX_RNG_DATA];
>+} __packed;
>+
>+static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>+{
>+ struct tpm2_get_random_out *resp;
>+ struct tpm_header *head;
>+ struct tpm_buf buf;
>+ off_t offset;
>+ u32 recd;
>+ int ret;
>+
>+ if (!out || !max || max > TPM_MAX_RNG_DATA)
>+ return -EINVAL;
>+
>+ ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
>+ if (ret)
>+ return ret;
>+
>+ if (tpm2_chip_auth(chip)) {
>+ tpm_buf_append_hmac_session(chip, &buf,
>+ TPM2_SA_ENCRYPT | TPM2_SA_CONTINUE_SESSION,
>+ NULL, 0);
>+ } else {
>+ head = (struct tpm_header *)buf.data;
>+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
>+ }
>+ tpm_buf_append_u16(&buf, max);
>+
>+ ret = tpm_buf_fill_hmac_session(chip, &buf);
>+ if (ret) {
>+ tpm_buf_destroy(&buf);
>+ return ret;
>+ }
>+
>+ ret = tpm_transmit_cmd(chip, &buf, offsetof(struct tpm2_get_random_out, buffer),
>+ "TPM2_GetRandom");
>+
>+ ret = tpm_buf_check_hmac_response(chip, &buf, ret);
>+ if (ret) {
>+ if (ret > 0)
>+ ret = -EIO;
>+
>+ goto out;
>+ }
>+
>+ head = (struct tpm_header *)buf.data;
>+ offset = TPM_HEADER_SIZE;
>+
>+ /* Skip the parameter size field: */
>+ if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
>+ offset += 4;
>+
>+ resp = (struct tpm2_get_random_out *)&buf.data[offset];
>+ recd = min_t(u32, be16_to_cpu(resp->size), max);
>+
>+ if (tpm_buf_length(&buf) <
>+ TPM_HEADER_SIZE + offsetof(struct tpm2_get_random_out, buffer) + recd) {
>+ ret = -EIO;
>+ goto out;
>+ }
>+
>+ memcpy(out, resp->buffer, recd);
>+ return recd;
>+
>+out:
>+ tpm2_end_auth_session(chip);
>+ tpm_buf_destroy(&buf);
>+ return ret;
>+}
>+
> /**
>- * tpm_get_random() - get random bytes from the TPM's RNG
>- * @chip: a &struct tpm_chip instance, %NULL for the default chip
>- * @out: destination buffer for the random bytes
>- * @max: the max number of bytes to write to @out
>+ * tpm_get_random() - Get random bytes from the TPM's RNG
>+ * @chip: A &tpm_chip instance. Whenset to %NULL, the default chip is used.
>+ * @out: Destination buffer for the acquired random bytes.
>+ * @max: The maximum number of bytes to write to @out.
>+ *
>+ * Iterates pulling more bytes from TPM up until all of the @max bytes have been
>+ * received.
> *
>- * Return: number of random bytes read or a negative error value.
>+ * Returns the number of random bytes read on success.
>+ * Returns -EINVAL when @out is NULL, or @max is not between zero and
>+ * %TPM_MAX_RNG_DATA.
>+ * Returns tpm_transmit_cmd() error codes when the TPM command results an
>+ * error.
> */
> int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> {
>+ u32 num_bytes = max;
>+ u8 *out_ptr = out;
>+ int retries = 5;
>+ int total = 0;
> int rc;
>
>- if (!out || max > TPM_MAX_RNG_DATA)
>+ if (!out || !max || max > TPM_MAX_RNG_DATA)
> return -EINVAL;
>
> if (!chip)
>@@ -508,11 +642,30 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> if (rc)
> return rc;
>
>- if (chip->flags & TPM_CHIP_FLAG_TPM2)
>- rc = tpm2_get_random(chip, out, max);
>- else
>- rc = tpm1_get_random(chip, out, max);
>+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>+ rc = tpm2_start_auth_session(chip);
>+ if (rc)
>+ return rc;
>+ }
>+
>+ do {
>+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
>+ rc = tpm2_get_random(chip, out_ptr, num_bytes);
>+ else
>+ rc = tpm1_get_random(chip, out_ptr, num_bytes);
>+
>+ if (rc < 0)
>+ goto err;
>+
>+ out_ptr += rc;
>+ total += rc;
>+ num_bytes -= rc;
>+ } while (retries-- && total < max);
>+
>+ tpm_put_ops(chip);
>+ return total ? total : -EIO;
>
>+err:
> tpm_put_ops(chip);
> return rc;
> }
>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>index 02c07fef41ba..f698d01401de 100644
>--- a/drivers/char/tpm/tpm.h
>+++ b/drivers/char/tpm/tpm.h
>@@ -251,7 +251,6 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> const char *desc, size_t min_cap_length);
>-int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> int tpm1_get_pcr_allocation(struct tpm_chip *chip);
> unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> int tpm_pm_suspend(struct device *dev);
>@@ -291,7 +290,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
> struct tpm_digest *digest, u16 *digest_size_ptr);
> int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> struct tpm_digest *digests);
>-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> u32 *value, const char *desc);
>
>diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>index b49a790f1bd5..0604e11c9778 100644
>--- a/drivers/char/tpm/tpm1-cmd.c
>+++ b/drivers/char/tpm/tpm1-cmd.c
>@@ -511,75 +511,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> }
> EXPORT_SYMBOL_GPL(tpm1_getcap);
>
>-#define TPM_ORD_GET_RANDOM 70
>-struct tpm1_get_random_out {
>- __be32 rng_data_len;
>- u8 rng_data[TPM_MAX_RNG_DATA];
>-} __packed;
>-
>-/**
>- * tpm1_get_random() - get random bytes from the TPM's RNG
>- * @chip: a &struct tpm_chip instance
>- * @dest: destination buffer for the random bytes
>- * @max: the maximum number of bytes to write to @dest
>- *
>- * Return:
>- * * number of bytes read
>- * * -errno (positive TPM return codes are masked to -EIO)
>- */
>-int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>-{
>- struct tpm1_get_random_out *out;
>- u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
>- struct tpm_buf buf;
>- u32 total = 0;
>- int retries = 5;
>- u32 recd;
>- int rc;
>-
>- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
>- if (rc)
>- return rc;
>-
>- do {
>- tpm_buf_append_u32(&buf, num_bytes);
>-
>- rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len),
>- "attempting get random");
>- if (rc) {
>- if (rc > 0)
>- rc = -EIO;
>- goto out;
>- }
>-
>- out = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
>-
>- recd = be32_to_cpu(out->rng_data_len);
>- if (recd > num_bytes) {
>- rc = -EFAULT;
>- goto out;
>- }
>-
>- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE +
>- sizeof(out->rng_data_len) + recd) {
>- rc = -EFAULT;
>- goto out;
>- }
>- memcpy(dest, out->rng_data, recd);
>-
>- dest += recd;
>- total += recd;
>- num_bytes -= recd;
>-
>- tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
>- } while (retries-- && total < max);
>-
>- rc = total ? (int)total : -EIO;
>-out:
>- tpm_buf_destroy(&buf);
>- return rc;
>-}
>-
> #define TPM_ORD_PCRREAD 21
> int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
>diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>index 3a77be7ebf4a..34168d5f9c78 100644
>--- a/drivers/char/tpm/tpm2-cmd.c
>+++ b/drivers/char/tpm/tpm2-cmd.c
>@@ -238,110 +238,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> return rc;
> }
>
>-struct tpm2_get_random_out {
>- __be16 size;
>- u8 buffer[TPM_MAX_RNG_DATA];
>-} __packed;
>-
>-/**
>- * tpm2_get_random() - get random bytes from the TPM RNG
>- *
>- * @chip: a &tpm_chip instance
>- * @dest: destination buffer
>- * @max: the max number of random bytes to pull
>- *
>- * Return:
>- * size of the buffer on success,
>- * -errno otherwise (positive TPM return codes are masked to -EIO)
>- */
>-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>-{
>- struct tpm2_get_random_out *out;
>- struct tpm_header *head;
>- struct tpm_buf buf;
>- u32 recd;
>- u32 num_bytes = max;
>- int err;
>- int total = 0;
>- int retries = 5;
>- u8 *dest_ptr = dest;
>- off_t offset;
>-
>- if (!num_bytes || max > TPM_MAX_RNG_DATA)
>- return -EINVAL;
>-
>- err = tpm2_start_auth_session(chip);
>- if (err)
>- return err;
>-
>- err = tpm_buf_init(&buf, 0, 0);
>- if (err) {
>- tpm2_end_auth_session(chip);
>- return err;
>- }
>-
>- do {
>- tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
>- if (tpm2_chip_auth(chip)) {
>- tpm_buf_append_hmac_session(chip, &buf,
>- TPM2_SA_ENCRYPT |
>- TPM2_SA_CONTINUE_SESSION,
>- NULL, 0);
>- } else {
>- offset = buf.handles * 4 + TPM_HEADER_SIZE;
>- head = (struct tpm_header *)buf.data;
>- if (tpm_buf_length(&buf) == offset)
>- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
>- }
>- tpm_buf_append_u16(&buf, num_bytes);
>- err = tpm_buf_fill_hmac_session(chip, &buf);
>- if (err) {
>- tpm_buf_destroy(&buf);
>- return err;
>- }
>-
>- err = tpm_transmit_cmd(chip, &buf,
>- offsetof(struct tpm2_get_random_out,
>- buffer),
>- "attempting get random");
>- err = tpm_buf_check_hmac_response(chip, &buf, err);
>- if (err) {
>- if (err > 0)
>- err = -EIO;
>- goto out;
>- }
>-
>- head = (struct tpm_header *)buf.data;
>- offset = TPM_HEADER_SIZE;
>- /* Skip the parameter size field: */
>- if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
>- offset += 4;
>-
>- out = (struct tpm2_get_random_out *)&buf.data[offset];
>- recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
>- if (tpm_buf_length(&buf) <
>- TPM_HEADER_SIZE +
>- offsetof(struct tpm2_get_random_out, buffer) +
>- recd) {
>- err = -EFAULT;
>- goto out;
>- }
>- memcpy(dest_ptr, out->buffer, recd);
>-
>- dest_ptr += recd;
>- total += recd;
>- num_bytes -= recd;
>- } while (retries-- && total < max);
>-
>- tpm_buf_destroy(&buf);
>-
>- return total ? total : -EIO;
>-out:
>- tpm_buf_destroy(&buf);
>- tpm2_end_auth_session(chip);
>- return err;
>-}
>-
> /**
> * tpm2_flush_context() - execute a TPM2_FlushContext command
> * @chip: TPM chip to use
>--
>2.39.5
>
>
J.
--
] https://www.earth.li/~noodles/ [] Generally, all generalizations [
] PGP/GPG Key @ the.earth.li [] are false.. [
] via keyserver, web or email. [] [
] RSA: 4096/0x94FA372B2DA8B985 [] [
^ permalink raw reply
* [PATCH v2] loadpin: Implement custom proc_handler for enforce
From: Joel Granados @ 2025-12-19 10:54 UTC (permalink / raw)
To: Kees Cook, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-security-module, linux-kernel, Joel Granados
Add a new static variable (loadpin_root_writable) to keep the
write-ability state of enforce. Remove set_sysctl and const qualify
loadpin_sysctl_table (moves into .rodata) as there is no longer need to
change the value of extra1. The new proc_handler_loadpin returns -EINVAL
when loadpin_root_writable is false and the kernel var (enforce) is
being written. The old way of modifying the write-ability of enforce
stayes in loadpin_check and is still set by calling sb_is_writable.
Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
Const qualifying ctl tables is part of the hardening effort in the linux
kernel.
Changes in v2:
- Removed set_sysctl
- Added new static loadpin_root_writable var to hold the writable state
- Renamed the variable holding the writable state to loadpin_root_writable
- Link to v1: https://lore.kernel.org/r/20251215-jag-const_loadpin-v1-1-6842775f4e90@kernel.org
---
---
security/loadpin/loadpin.c | 43 +++++++++++++++++++------------------------
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 273ffbd6defe1324d6688dec5f9fe6c9401283ed..2ef863af869e45658e9e830af8ac40beaad7d5a9 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -53,44 +53,41 @@ static bool deny_reading_verity_digests;
#endif
#ifdef CONFIG_SYSCTL
-static struct ctl_table loadpin_sysctl_table[] = {
+// initialized to false
+static bool loadpin_root_writable;
+
+static int proc_handler_loadpin(const struct ctl_table *table, int dir,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ if (!loadpin_root_writable && SYSCTL_USER_TO_KERN(dir))
+ return -EINVAL;
+ return proc_dointvec_minmax(table, dir, buffer, lenp, ppos);
+}
+
+static const struct ctl_table loadpin_sysctl_table[] = {
{
.procname = "enforce",
.data = &enforce,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ONE,
+ .proc_handler = proc_handler_loadpin,
+ .extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
};
-
-static void set_sysctl(bool is_writable)
-{
- /*
- * If load pinning is not enforced via a read-only block
- * device, allow sysctl to change modes for testing.
- */
- if (is_writable)
- loadpin_sysctl_table[0].extra1 = SYSCTL_ZERO;
- else
- loadpin_sysctl_table[0].extra1 = SYSCTL_ONE;
-}
-#else
-static inline void set_sysctl(bool is_writable) { }
#endif
-static void report_writable(struct super_block *mnt_sb, bool writable)
+static void report_writable(struct super_block *mnt_sb)
{
if (mnt_sb->s_bdev) {
pr_info("%pg (%u:%u): %s\n", mnt_sb->s_bdev,
MAJOR(mnt_sb->s_bdev->bd_dev),
MINOR(mnt_sb->s_bdev->bd_dev),
- writable ? "writable" : "read-only");
+ loadpin_root_writable ? "writable" : "read-only");
} else
pr_info("mnt_sb lacks block device, treating as: writable\n");
- if (!writable)
+ if (!loadpin_root_writable)
pr_info("load pinning engaged.\n");
}
@@ -131,7 +128,6 @@ static int loadpin_check(struct file *file, enum kernel_read_file_id id)
struct super_block *load_root;
const char *origin = kernel_read_file_id_str(id);
bool first_root_pin = false;
- bool load_root_writable;
/* If the file id is excluded, ignore the pinning. */
if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) &&
@@ -152,7 +148,6 @@ static int loadpin_check(struct file *file, enum kernel_read_file_id id)
}
load_root = file->f_path.mnt->mnt_sb;
- load_root_writable = sb_is_writable(load_root);
/* First loaded module/firmware defines the root for all others. */
spin_lock(&pinned_root_spinlock);
@@ -168,8 +163,8 @@ static int loadpin_check(struct file *file, enum kernel_read_file_id id)
spin_unlock(&pinned_root_spinlock);
if (first_root_pin) {
- report_writable(pinned_root, load_root_writable);
- set_sysctl(load_root_writable);
+ report_writable(pinned_root);
+ loadpin_root_writable = sb_is_writable(pinned_root);
report_load(origin, file, "pinned");
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251215-jag-const_loadpin-761f052f76c4
Best regards,
--
Joel Granados <joel.granados@kernel.org>
^ permalink raw reply related
* Re: [PATCH] loadpin: Implement custom proc_handler for enforce
From: Joel Granados @ 2025-12-19 10:55 UTC (permalink / raw)
To: Kees Cook
Cc: Paul Moore, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
In-Reply-To: <202512160028.8F11A5D19@keescook>
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
Sent out a V2
On Tue, Dec 16, 2025 at 12:37:29AM -0800, Kees Cook wrote:
> On Mon, Dec 15, 2025 at 04:43:48PM +0100, Joel Granados wrote:
> > The new proc_handler_loadpin returns -EINVAL when is_loadpin_writable is
> > false and the kernel var (enforce) is being written. Move
> > loadpin_sysctl_table to .rodata (by const qualifying it) as there is no
> > need to change the value of the extra1 entry.
> >
> > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> > ---
> > Const qualifying ctl tables is part of the hardening effort in the linux
> > kernel.
>
> Ah yes, thanks for getting through these "weird" cases! :)
>
> > ---
> > security/loadpin/loadpin.c | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> > index 273ffbd6defe1324d6688dec5f9fe6c9401283ed..f049c81b82a78265b6ae358bb2a814265cec9f16 100644
> > --- a/security/loadpin/loadpin.c
> > +++ b/security/loadpin/loadpin.c
> > @@ -53,18 +53,29 @@ static bool deny_reading_verity_digests;
> > #endif
> >
> > #ifdef CONFIG_SYSCTL
> > -static struct ctl_table loadpin_sysctl_table[] = {
> > +static bool is_loadpin_writable;
>
> I would rather that load_root_writable were declared external to
> loadpin_check(), and then we could remove set_sysctl() entirely, instead
> using load_root_writable as the thing to check in proc_handler_loadpin().
>
> And also rename load_root_writable to "loadpin_root_writable", just to
> make it a bit more clear.
>
> -Kees
>
> --
> Kees Cook
--
Joel Granados
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply
* [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT
From: Mickaël Salaün @ 2025-12-19 14:22 UTC (permalink / raw)
To: linux-security-module; +Cc: Mickaël Salaün
Until now, each landlock_request struct were allocated on the stack, even
if not really used, because is_access_to_paths_allowed() unconditionally
modified the passed references. Even if the changed landlock_request
variables are not used, the compiler is not smart enough to detect this
case.
To avoid this issue, explicitly disable the related code when
CONFIG_AUDIT is not set, which enables elision of log_request_parent*
and associated caller's stack variables thanks to dead code elimination.
This makes it possible to reduce the stack frame by 192 bytes for the
path_link and path_rename hooks, and by 96 bytes for most other
filesystem hooks.
Here is a summary of scripts/checkstack.pl before and after this change
when CONFIG_AUDIT is disabled:
Function Old size New size Diff
----------------------------------------------------------
current_check_refer_path 384 208 -176
current_check_access_path 192 112 -80
hook_file_open 208 128 -80
is_access_to_paths_allowed 240 224 -16
Also, add extra pointer checks to be more future-proof.
Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/fs.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index fe794875ad46..722f950307f6 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -939,7 +939,12 @@ static bool is_access_to_paths_allowed(
}
path_put(&walker_path);
- if (!allowed_parent1) {
+ /*
+ * Check CONFIG_AUDIT to enable elision of log_request_parent* and
+ * associated caller's stack variables thanks to dead code elimination.
+ */
+#ifdef CONFIG_AUDIT
+ if (!allowed_parent1 && log_request_parent1) {
log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent1->audit.u.path = *path;
@@ -949,7 +954,7 @@ static bool is_access_to_paths_allowed(
ARRAY_SIZE(*layer_masks_parent1);
}
- if (!allowed_parent2) {
+ if (!allowed_parent2 && log_request_parent2) {
log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
log_request_parent2->audit.u.path = *path;
@@ -958,6 +963,8 @@ static bool is_access_to_paths_allowed(
log_request_parent2->layer_masks_size =
ARRAY_SIZE(*layer_masks_parent2);
}
+#endif /* CONFIG_AUDIT */
+
return allowed_parent1 && allowed_parent2;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v1] landlock: Optimize stack usage when !CONFIG_AUDIT
From: Mickaël Salaün @ 2025-12-19 14:26 UTC (permalink / raw)
To: linux-security-module; +Cc: Günther Noack, Justin Suess
In-Reply-To: <20251219142302.744917-2-mic@digikod.net>
On Fri, Dec 19, 2025 at 03:22:59PM +0100, Mickaël Salaün wrote:
> Until now, each landlock_request struct were allocated on the stack, even
> if not really used, because is_access_to_paths_allowed() unconditionally
> modified the passed references. Even if the changed landlock_request
> variables are not used, the compiler is not smart enough to detect this
> case.
>
> To avoid this issue, explicitly disable the related code when
> CONFIG_AUDIT is not set, which enables elision of log_request_parent*
> and associated caller's stack variables thanks to dead code elimination.
> This makes it possible to reduce the stack frame by 192 bytes for the
> path_link and path_rename hooks, and by 96 bytes for most other
> filesystem hooks.
>
> Here is a summary of scripts/checkstack.pl before and after this change
> when CONFIG_AUDIT is disabled:
>
> Function Old size New size Diff
> ----------------------------------------------------------
> current_check_refer_path 384 208 -176
> current_check_access_path 192 112 -80
> hook_file_open 208 128 -80
> is_access_to_paths_allowed 240 224 -16
>
> Also, add extra pointer checks to be more future-proof.
>
Reported-by: Tingmao Wang <m@maowtm.org>
Link: https://lore.kernel.org/r/eb86863b-53b0-460b-b223-84dd31d765b9@maowtm.org
> Fixes: 2fc80c69df82 ("landlock: Log file-related denials")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/fs.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index fe794875ad46..722f950307f6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -939,7 +939,12 @@ static bool is_access_to_paths_allowed(
> }
> path_put(&walker_path);
>
> - if (!allowed_parent1) {
> + /*
> + * Check CONFIG_AUDIT to enable elision of log_request_parent* and
> + * associated caller's stack variables thanks to dead code elimination.
> + */
> +#ifdef CONFIG_AUDIT
> + if (!allowed_parent1 && log_request_parent1) {
> log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent1->audit.u.path = *path;
> @@ -949,7 +954,7 @@ static bool is_access_to_paths_allowed(
> ARRAY_SIZE(*layer_masks_parent1);
> }
>
> - if (!allowed_parent2) {
> + if (!allowed_parent2 && log_request_parent2) {
> log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> @@ -958,6 +963,8 @@ static bool is_access_to_paths_allowed(
> log_request_parent2->layer_masks_size =
> ARRAY_SIZE(*layer_masks_parent2);
> }
> +#endif /* CONFIG_AUDIT */
> +
> return allowed_parent1 && allowed_parent2;
> }
>
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v4 03/10] landlock: Suppress logging when quiet flag is present
From: Mickaël Salaün @ 2025-12-19 14:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Justin Suess, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <eb86863b-53b0-460b-b223-84dd31d765b9@maowtm.org>
On Sun, Nov 23, 2025 at 09:01:47PM +0000, Tingmao Wang wrote:
> On 11/21/25 15:27, Mickaël Salaün wrote:
> > On Sun, Nov 16, 2025 at 09:59:33PM +0000, Tingmao Wang wrote:
> >> [...]
> >> /**
> >> * landlock_log_denial - Create audit records related to a denial
> >> *
> >> * @subject: The Landlock subject's credential denying an action.
> >> * @request: Detail of the user space request.
> >> + * @rule_flags: The flags for the matched rule, or no_rule_flags (zero) if
> >> + * this is a scope request (no particular object involved).
> >> */
> >> void landlock_log_denial(const struct landlock_cred_security *const subject,
> >> - const struct landlock_request *const request)
> >> + const struct landlock_request *const request,
> >> + const struct collected_rule_flags rule_flags)
> >
> > It would be simpler and limit code change to move rule_flags/quiet_flags
> > into struct landlock_request, which means we can also remove
> > no_rule_flags.
>
> That's true, I can do that. In fact this way we also don't have to have 2
> extra parameters for is_access_to_paths_allowed - it can just operate on
> log_request_parent{1,2}->rule_flags. However I do see that
> landlock_request is intended to only be used by audit/logging code (and
> there is a comment in audit.h about not using it outside CONFIG_AUDIT to
> enable it to be optimized away, although testing a fresh build on next it
> doesn't look like it is taken out of vmlinux if compiled without
> CONFIG_AUDIT).
The log_request_parent* are indeed not elided. I sent a patch to fix
that: https://lore.kernel.org/r/20251219142302.744917-2-mic@digikod.net
> While this is fine for the purpose of this series as the
> quiet flag only affects audit logging, I wonder if this might cause a
> problem when we want to add more flags that might not be related to audit
> (e.g. Justin's LANDLOCK_ADD_RULE_NO_INHERIT).
>
> Alternatively maybe is_access_to_paths_allowed can still take extra
> parameters for rule flags, and we can make it so that the new rule_flags
> field in landlock_request is only assigned to right before
> landlock_log_denial, not from is_access_to_paths_allowed? (I won't do
> this for v5 which I will send in a minute)
Yes, we should store the rule flags in landlock_request struct when this
in only required for audit. In any case, we should always be able to
build without CONFIG_AUDIT, and in this case the stack frame should not
include landlock_request. So in your last patch series, because
landlock_request's rule flags are referenced by standalone pointers,
landlock_requset cannot be elided.
>
> >
> >> {
> >> struct audit_buffer *ab;
> >> struct landlock_hierarchy *youngest_denied;
> >> size_t youngest_layer;
> >> - access_mask_t missing;
> >> + access_mask_t missing, quiet_mask;
> >> + bool object_quiet_flag = false, quiet_applicable_to_access = false;
> >>
> >> if (WARN_ON_ONCE(!subject || !subject->domain ||
> >> !subject->domain->hierarchy || !request))
> >> @@ -409,10 +586,13 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> >> youngest_layer = get_denied_layer(
> >> subject->domain, &missing, request->layer_masks,
> >> request->layer_masks_size);
> >> + object_quiet_flag = !!(rule_flags.quiet_masks & BIT(youngest_layer));
> >> } else {
> >> youngest_layer = get_layer_from_deny_masks(
> >> &missing, request->all_existing_optional_access,
> >> - request->deny_masks);
> >> + request->deny_masks,
> >> + request->quiet_optional_accesses,
> >> + &object_quiet_flag);
> >> }
> >> youngest_denied =
> >> get_hierarchy(subject->domain, youngest_layer);
^ permalink raw reply
* [PATCH v5 00/36] Compiler-Based Context- and Locking-Analysis
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
Linus Torvalds
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
Context Analysis is a language extension, which enables statically
checking that required contexts are active (or inactive) by acquiring
and releasing user-definable "context locks". An obvious application is
lock-safety checking for the kernel's various synchronization primitives
(each of which represents a "context lock"), and checking that locking
rules are not violated.
The feature requires Clang 22 (unreleased) or later. Clang originally
called the feature "Thread Safety Analysis" [1]. This was later changed
and the feature became more flexible, gaining the ability to define
custom "capabilities". Its foundations can be found in "Capability
Systems" [2], used to specify the permissibility of operations to depend
on some "capability" being held (or not held).
Because the feature is not just able to express "capabilities" related
to synchronization primitives, and "capability" is already overloaded in
the kernel, the naming chosen for the kernel departs from Clang's
"Thread Safety" and "capability" nomenclature; we refer to the feature
as "Context Analysis" to avoid confusion. The internal implementation
still makes references to Clang's terminology in a few places, such as
`-Wthread-safety` being the warning option that also still appears in
diagnostic messages.
Additional details can be found in the added kernel-doc documentation.
An LWN article covered v2 of the series: https://lwn.net/Articles/1012990/
[1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
[2] https://www.cs.cornell.edu/talc/papers/capabilities.pdf
=== Design ===
Prior art exists in the form of Sparse's Context Tracking. Locking
annotations on functions already exist sparsely, so the concept of
analyzing locking rules is not foreign to the kernel's codebase.
However, Clang's analysis is more complete vs. Sparse's, with the
typical trade-offs in static analysis: improved completeness is
sacrificed for more possible false positives or additional annotations
required by the programmer. Numerous options exist to disable or opt out
certain code from analysis.
This series initially aimed to retain compatibility with Sparse, which
can provide tree-wide analysis of a subset of the context analysis
introduced, but it was later decided to drop Sparse compatibility. For
the most part, the new (and old) keywords used for annotations remain
the same, and many of the pre-existing annotations remain valid.
One big question is how to enable this feature, given we end up with a
new dialect of C; two approaches have been considered:
A. Tree-wide all-or-nothing approach. This approach requires tree-wide
changes, adding annotations or selective opt-outs. Making more
primitives context-analysis aware increases churn where maintainers
are unfamiliar with the feature and the analysis is unable to deal
with complex code patterns as-is.
Because we can't change the programming language (even if from one C
dialect to another) of the kernel overnight, a different approach might
cause less friction.
B. A selective, incremental, and much less intrusive approach.
Maintainers of subsystems opt in their modules or directories into
context analysis (via Makefile):
CONTEXT_ANALYSIS_foo.o := y # foo.o only
CONTEXT_ANALYSIS := y # all TUs
Most (eventually all) synchronization primitives, and more
context locks including ones that track "irq disabled",
"preemption" disabled, etc. could be supported.
The approach taken by this series is B. This ensures that only
subsystems where maintainers are willing to deal with any warnings are
opted-in. Introducing the feature can be done incrementally, without
large tree-wide changes and adding numerous opt-outs and annotations to
the majority of code.
Note: Bart Van Assche concurrently worked on enabling -Wthread-safety:
https://lore.kernel.org/all/20250206175114.1974171-1-bvanassche@acm.org/
Bart's work has shown what it might take to go with approach A
(tree-wide, restricted to 'mutex' usage). This has shown that the
analysis finds real issues when applied to enough subsystems! We hope
this serves as motivation to eventually enable the analysis in as many
subsystems as possible, particularly subsystems that are not as easily
tested by CI systems and test robots.
=== Initial Uses ===
With this initial series, the following synchronization primitives are
supported: `raw_spinlock_t`, `spinlock_t`, `rwlock_t`, `mutex`,
`seqlock_t`, `bit_spinlock`, RCU, SRCU (`srcu_struct`), `rw_semaphore`,
`local_lock_t`, `ww_mutex`.
To demonstrate use of the feature on real kernel code, the series also
enables context analysis for the following subsystems:
* kernel/kcov
* kernel/kcsan
* kernel/sched/
* lib/rhashtable
* lib/stackdepot
* mm/kfence
* security/tomoyo
* crypto/
The initial benefits are static detection of violations of locking
rules. As more context locks are supported, we would see more static
checking beyond what regular C can provide, all while remaining easy
(and quick) to use via the Clang compiler.
Note: The kernel already provides dynamic analysis tools Lockdep and
KCSAN for lock-safety checking and data-race detection respectively.
Unlike those, Clang's context analysis is a compile-time static
analysis with no runtime impact. The static analysis complements
existing dynamic analysis tools, as it may catch some issues before
even getting into a running kernel, but is *not* a replacement for
whole-kernel testing with the dynamic analysis tools enabled!
=== Appendix ===
A Clang version that supports `-Wthread-safety-pointer` and the new
alias-analysis of context lock pointers is required (from this version
onwards):
https://github.com/llvm/llvm-project/commit/7ccb5c08f0685d4787f12c3224a72f0650c5865e
The minimum required release version will be Clang 22.
This series is also available at this Git tree:
https://git.kernel.org/pub/scm/linux/kernel/git/melver/linux.git/log/?h=ctx-analysis/dev
The feature itself is usable when all patches up to the "MAINTAINERS"
patch are applied; the subsequent "enablement" patches are optional and
could be applied by the respective maintainers to their own tree after
the context analysis infrastructure is available.
=== Changelog ===
v5:
- Rename "context guard" -> "context lock".
- Rework cleanup.h support to properly release at scope guard end.
- Use new cleanup.h helpers to properly support scoped lock guards.
- Support new scoped_seqlock_read().
- Also remove include/linux/lockref.h's deprecated use of __cond_lock().
- Fix invalid __releases/__acquires in um skas/mm_id.h header
(transitively included through mm_types.h which is included by
subsystems that may want to enable context analysis).
- Better document Clang's `assert_capability` attribute.
- Rebase on v6.19-rc1.
v4: https://lore.kernel.org/all/20251120145835.3833031-2-elver@google.com/
- Rename capability -> context analysis, per Linus's suggestion:
https://lore.kernel.org/all/CAHk-=wgd-Wcp0GpYaQnU7S9ci+FvFmaNw1gm75mzf0ZWdNLxvw@mail.gmail.com/
- Minor fixes.
v3: https://lore.kernel.org/all/20250918140451.1289454-1-elver@google.com/
- Bump min. Clang version to 22+ (unreleased), which now supports:
* re-entrancy via __attribute__((reentrant_capability));
* basic form of capability alias analysis - which is the
biggest improvement since v2.
This was the result of conclusions from this discussion:
https://lore.kernel.org/all/CANpmjNPquO=W1JAh1FNQb8pMQjgeZAKCPQUAd7qUg=5pjJ6x=Q@mail.gmail.com/
- Rename __asserts_cap/__assert_cap to __assumes_cap/__assume_cap.
- Switch to DECLARE_LOCK_GUARD_1_ATTRS().
- Add __acquire_ret and __acquire_shared_ret helper macros - can be
used to define function-like macros that return objects which
contains a held capabilities. Works now because of capability alias
analysis.
- Add capability_unsafe_alias() helper, where the analysis rightfully
points out we're doing strange things with aliases but we don't
care.
- Support multi-argument attributes.
- Enable for kernel/sched/{core,fair}.c, kernel/kcsan.
- Drop drivers/tty changes (revisit later).
v2: https://lore.kernel.org/all/20250304092417.2873893-1-elver@google.com/
- Remove Sparse context tracking support - after the introduction of
Clang support, so that backports can skip removal of Sparse support.
- Remove __cond_lock() function-like helper.
- ww_mutex support.
- -Wthread-safety-addressof was reworked and committed in upstream
Clang as -Wthread-safety-pointer.
- Make __cond_acquires() and __cond_acquires_shared() take abstract
value, since compiler only cares about zero and non-zero.
- Rename __var_guarded_by to simply __guarded_by. Initially the idea
was to be explicit about if the variable itself or the pointed-to
data is guarded, but in the long-term, making this shorter might be
better.
- Likewise rename __ref_guarded_by to __pt_guarded_by.
- Introduce common header warning suppressions - this is a better
solution than guarding header inclusions with disable_ +
enable_capability_analysis(). Header suppressions are disabled when
selecting CONFIG_WARN_CAPABILITY_ANALYSIS_ALL=y. This bumps the
minimum Clang version required to 20+.
- Make the data_race() macro imply disabled capability analysis.
Writing capability_unsafe(data_race(..)) is unnecessarily verbose
and data_race() on its own already indicates something subtly unsafe
is happening. This change was made after analysis of a finding in
security/tomoyo.
- Enable analysis in the following subsystems as additional examples
of larger subsystem. Where it was obvious, the __guarded_by
attribute was added to lock-guarded variables to improve coverage.
* drivers/tty
* security/tomoyo
* crypto/
RFC v1: https://lore.kernel.org/lkml/20250206181711.1902989-1-elver@google.com
Marco Elver (36):
compiler_types: Move lock checking attributes to
compiler-context-analysis.h
compiler-context-analysis: Add infrastructure for Context Analysis
with Clang
compiler-context-analysis: Add test stub
Documentation: Add documentation for Compiler-Based Context Analysis
checkpatch: Warn about context_unsafe() without comment
cleanup: Basic compatibility with context analysis
lockdep: Annotate lockdep assertions for context analysis
locking/rwlock, spinlock: Support Clang's context analysis
compiler-context-analysis: Change __cond_acquires to take return value
locking/mutex: Support Clang's context analysis
locking/seqlock: Support Clang's context analysis
bit_spinlock: Include missing <asm/processor.h>
bit_spinlock: Support Clang's context analysis
rcu: Support Clang's context analysis
srcu: Support Clang's context analysis
kref: Add context-analysis annotations
locking/rwsem: Support Clang's context analysis
locking/local_lock: Include missing headers
locking/local_lock: Support Clang's context analysis
locking/ww_mutex: Support Clang's context analysis
debugfs: Make debugfs_cancellation a context lock struct
um: Fix incorrect __acquires/__releases annotations
compiler-context-analysis: Remove Sparse support
compiler-context-analysis: Remove __cond_lock() function-like helper
compiler-context-analysis: Introduce header suppressions
compiler: Let data_race() imply disabled context analysis
MAINTAINERS: Add entry for Context Analysis
kfence: Enable context analysis
kcov: Enable context analysis
kcsan: Enable context analysis
stackdepot: Enable context analysis
rhashtable: Enable context analysis
printk: Move locking annotation to printk.c
security/tomoyo: Enable context analysis
crypto: Enable context analysis
sched: Enable context analysis for core.c and fair.c
Documentation/dev-tools/context-analysis.rst | 145 +++++
Documentation/dev-tools/index.rst | 1 +
Documentation/dev-tools/sparse.rst | 19 -
Documentation/mm/process_addrs.rst | 6 +-
MAINTAINERS | 11 +
Makefile | 1 +
arch/um/include/shared/skas/mm_id.h | 5 +-
arch/um/kernel/skas/mmu.c | 13 +-
crypto/Makefile | 2 +
crypto/acompress.c | 6 +-
crypto/algapi.c | 2 +
crypto/api.c | 1 +
crypto/crypto_engine.c | 2 +-
crypto/drbg.c | 5 +
crypto/internal.h | 2 +-
crypto/proc.c | 3 +
crypto/scompress.c | 24 +-
.../net/wireless/intel/iwlwifi/iwl-trans.c | 4 +-
.../net/wireless/intel/iwlwifi/iwl-trans.h | 6 +-
.../intel/iwlwifi/pcie/gen1_2/internal.h | 5 +-
.../intel/iwlwifi/pcie/gen1_2/trans.c | 4 +-
fs/dlm/lock.c | 2 +-
include/crypto/internal/acompress.h | 7 +-
include/crypto/internal/engine.h | 2 +-
include/linux/bit_spinlock.h | 24 +-
include/linux/cleanup.h | 50 ++
include/linux/compiler-context-analysis.h | 441 +++++++++++++
include/linux/compiler.h | 2 +
include/linux/compiler_types.h | 18 +-
include/linux/console.h | 4 +-
include/linux/debugfs.h | 12 +-
include/linux/kref.h | 2 +
include/linux/list_bl.h | 2 +
include/linux/local_lock.h | 51 +-
include/linux/local_lock_internal.h | 73 ++-
include/linux/lockdep.h | 12 +-
include/linux/lockref.h | 4 +-
include/linux/mm.h | 33 +-
include/linux/mutex.h | 38 +-
include/linux/mutex_types.h | 4 +-
include/linux/rcupdate.h | 90 +--
include/linux/refcount.h | 6 +-
include/linux/rhashtable.h | 16 +-
include/linux/rwlock.h | 22 +-
include/linux/rwlock_api_smp.h | 43 +-
include/linux/rwlock_rt.h | 44 +-
include/linux/rwlock_types.h | 10 +-
include/linux/rwsem.h | 76 ++-
include/linux/sched.h | 6 +-
include/linux/sched/signal.h | 16 +-
include/linux/sched/task.h | 6 +-
include/linux/sched/wake_q.h | 3 +
include/linux/seqlock.h | 38 +-
include/linux/seqlock_types.h | 5 +-
include/linux/spinlock.h | 112 +++-
include/linux/spinlock_api_smp.h | 34 +-
include/linux/spinlock_api_up.h | 112 +++-
include/linux/spinlock_rt.h | 37 +-
include/linux/spinlock_types.h | 10 +-
include/linux/spinlock_types_raw.h | 5 +-
include/linux/srcu.h | 73 ++-
include/linux/srcutiny.h | 6 +
include/linux/srcutree.h | 10 +-
include/linux/ww_mutex.h | 22 +-
kernel/Makefile | 2 +
kernel/kcov.c | 36 +-
kernel/kcsan/Makefile | 2 +
kernel/kcsan/report.c | 11 +-
kernel/printk/printk.c | 2 +
kernel/sched/Makefile | 3 +
kernel/sched/core.c | 89 ++-
kernel/sched/fair.c | 7 +-
kernel/sched/sched.h | 126 +++-
kernel/signal.c | 4 +-
kernel/time/posix-timers.c | 13 +-
lib/Kconfig.debug | 44 ++
lib/Makefile | 6 +
lib/dec_and_lock.c | 8 +-
lib/lockref.c | 1 -
lib/rhashtable.c | 5 +-
lib/stackdepot.c | 20 +-
lib/test_context-analysis.c | 604 ++++++++++++++++++
mm/kfence/Makefile | 2 +
mm/kfence/core.c | 20 +-
mm/kfence/kfence.h | 14 +-
mm/kfence/report.c | 4 +-
mm/memory.c | 4 +-
mm/pgtable-generic.c | 19 +-
net/ipv4/tcp_sigpool.c | 2 +-
scripts/Makefile.context-analysis | 11 +
scripts/Makefile.lib | 10 +
scripts/checkpatch.pl | 7 +
scripts/context-analysis-suppression.txt | 33 +
security/tomoyo/Makefile | 2 +
security/tomoyo/common.c | 52 +-
security/tomoyo/common.h | 77 +--
security/tomoyo/domain.c | 1 +
security/tomoyo/environ.c | 1 +
security/tomoyo/file.c | 5 +
security/tomoyo/gc.c | 28 +-
security/tomoyo/mount.c | 2 +
security/tomoyo/network.c | 3 +
tools/include/linux/compiler_types.h | 2 -
103 files changed, 2523 insertions(+), 609 deletions(-)
create mode 100644 Documentation/dev-tools/context-analysis.rst
create mode 100644 include/linux/compiler-context-analysis.h
create mode 100644 lib/test_context-analysis.c
create mode 100644 scripts/Makefile.context-analysis
create mode 100644 scripts/context-analysis-suppression.txt
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply
* [PATCH v5 01/36] compiler_types: Move lock checking attributes to compiler-context-analysis.h
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
The conditional definition of lock checking macros and attributes is
about to become more complex. Factor them out into their own header for
better readability, and to make it obvious which features are supported
by which mode (currently only Sparse). This is the first step towards
generalizing towards "context analysis".
No functional change intended.
Signed-off-by: Marco Elver <elver@google.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
v4:
* Rename capability -> context analysis.
---
include/linux/compiler-context-analysis.h | 32 +++++++++++++++++++++++
include/linux/compiler_types.h | 18 ++-----------
2 files changed, 34 insertions(+), 16 deletions(-)
create mode 100644 include/linux/compiler-context-analysis.h
diff --git a/include/linux/compiler-context-analysis.h b/include/linux/compiler-context-analysis.h
new file mode 100644
index 000000000000..f8af63045281
--- /dev/null
+++ b/include/linux/compiler-context-analysis.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Macros and attributes for compiler-based static context analysis.
+ */
+
+#ifndef _LINUX_COMPILER_CONTEXT_ANALYSIS_H
+#define _LINUX_COMPILER_CONTEXT_ANALYSIS_H
+
+#ifdef __CHECKER__
+
+/* Sparse context/lock checking support. */
+# define __must_hold(x) __attribute__((context(x,1,1)))
+# define __acquires(x) __attribute__((context(x,0,1)))
+# define __cond_acquires(x) __attribute__((context(x,0,-1)))
+# define __releases(x) __attribute__((context(x,1,0)))
+# define __acquire(x) __context__(x,1)
+# define __release(x) __context__(x,-1)
+# define __cond_lock(x, c) ((c) ? ({ __acquire(x); 1; }) : 0)
+
+#else /* !__CHECKER__ */
+
+# define __must_hold(x)
+# define __acquires(x)
+# define __cond_acquires(x)
+# define __releases(x)
+# define __acquire(x) (void)0
+# define __release(x) (void)0
+# define __cond_lock(x, c) (c)
+
+#endif /* __CHECKER__ */
+
+#endif /* _LINUX_COMPILER_CONTEXT_ANALYSIS_H */
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1280693766b9..ddada1ed88ea 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -41,6 +41,8 @@
# define BTF_TYPE_TAG(value) /* nothing */
#endif
+#include <linux/compiler-context-analysis.h>
+
/* sparse defines __CHECKER__; see Documentation/dev-tools/sparse.rst */
#ifdef __CHECKER__
/* address spaces */
@@ -51,14 +53,6 @@
# define __rcu __attribute__((noderef, address_space(__rcu)))
static inline void __chk_user_ptr(const volatile void __user *ptr) { }
static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
-/* context/locking */
-# define __must_hold(x) __attribute__((context(x,1,1)))
-# define __acquires(x) __attribute__((context(x,0,1)))
-# define __cond_acquires(x) __attribute__((context(x,0,-1)))
-# define __releases(x) __attribute__((context(x,1,0)))
-# define __acquire(x) __context__(x,1)
-# define __release(x) __context__(x,-1)
-# define __cond_lock(x,c) ((c) ? ({ __acquire(x); 1; }) : 0)
/* other */
# define __force __attribute__((force))
# define __nocast __attribute__((nocast))
@@ -79,14 +73,6 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
# define __chk_user_ptr(x) (void)0
# define __chk_io_ptr(x) (void)0
-/* context/locking */
-# define __must_hold(x)
-# define __acquires(x)
-# define __cond_acquires(x)
-# define __releases(x)
-# define __acquire(x) (void)0
-# define __release(x) (void)0
-# define __cond_lock(x,c) (c)
/* other */
# define __force
# define __nocast
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Context Analysis is a language extension, which enables statically
checking that required contexts are active (or inactive), by acquiring
and releasing user-definable "context locks". An obvious application is
lock-safety checking for the kernel's various synchronization primitives
(each of which represents a "context lock"), and checking that locking
rules are not violated.
Clang originally called the feature "Thread Safety Analysis" [1]. This
was later changed and the feature became more flexible, gaining the
ability to define custom "capabilities". Its foundations can be found in
"Capability Systems" [2], used to specify the permissibility of
operations to depend on some "capability" being held (or not held).
Because the feature is not just able to express "capabilities" related
to synchronization primitives, and "capability" is already overloaded in
the kernel, the naming chosen for the kernel departs from Clang's
"Thread Safety" and "capability" nomenclature; we refer to the feature
as "Context Analysis" to avoid confusion. The internal implementation
still makes references to Clang's terminology in a few places, such as
`-Wthread-safety` being the warning option that also still appears in
diagnostic messages.
[1] https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
[2] https://www.cs.cornell.edu/talc/papers/capabilities.pdf
See more details in the kernel-doc documentation added in this and
subsequent changes.
Clang version 22+ is required.
Signed-off-by: Marco Elver <elver@google.com>
---
v5:
* Rename "context guard" -> "context lock".
* Better document Clang's `assert_capability` attribute.
v4:
* Rename capability -> context analysis.
v3:
* Require Clang 22 or later (reentrant capabilities, basic alias analysis).
* Rename __assert_cap/__asserts_cap -> __assume_cap/__assumes_cap (suggested by Peter).
* Add __acquire_ret and __acquire_shared_ret helper macros - can be used
to define function-like macros that return objects which contains a
held capabilities. Works now because of capability alias analysis.
* Add capability_unsafe_alias() helper, where the analysis rightfully
points out we're doing strange things with aliases but we don't care.
* Support multi-argument attributes.
v2:
* New -Wthread-safety feature rename to -Wthread-safety-pointer (was
-Wthread-safety-addressof).
* Introduce __capability_unsafe() function attribute.
* Rename __var_guarded_by to simply __guarded_by. The initial idea was
to be explicit if the variable or pointed-to data is guarded by, but
having a shorter attribute name is likely better long-term.
* Rename __ref_guarded_by to __pt_guarded_by (pointed-to guarded by).
---
Makefile | 1 +
include/linux/compiler-context-analysis.h | 464 +++++++++++++++++++++-
lib/Kconfig.debug | 30 ++
scripts/Makefile.context-analysis | 7 +
scripts/Makefile.lib | 10 +
5 files changed, 505 insertions(+), 7 deletions(-)
create mode 100644 scripts/Makefile.context-analysis
diff --git a/Makefile b/Makefile
index e404e4767944..d4c2aa2df79c 100644
--- a/Makefile
+++ b/Makefile
@@ -1118,6 +1118,7 @@ include-$(CONFIG_RANDSTRUCT) += scripts/Makefile.randstruct
include-$(CONFIG_KSTACK_ERASE) += scripts/Makefile.kstack_erase
include-$(CONFIG_AUTOFDO_CLANG) += scripts/Makefile.autofdo
include-$(CONFIG_PROPELLER_CLANG) += scripts/Makefile.propeller
+include-$(CONFIG_WARN_CONTEXT_ANALYSIS) += scripts/Makefile.context-analysis
include-$(CONFIG_GCC_PLUGINS) += scripts/Makefile.gcc-plugins
include $(addprefix $(srctree)/, $(include-y))
diff --git a/include/linux/compiler-context-analysis.h b/include/linux/compiler-context-analysis.h
index f8af63045281..afff910d8930 100644
--- a/include/linux/compiler-context-analysis.h
+++ b/include/linux/compiler-context-analysis.h
@@ -6,27 +6,477 @@
#ifndef _LINUX_COMPILER_CONTEXT_ANALYSIS_H
#define _LINUX_COMPILER_CONTEXT_ANALYSIS_H
+#if defined(WARN_CONTEXT_ANALYSIS)
+
+/*
+ * These attributes define new context lock (Clang: capability) types.
+ * Internal only.
+ */
+# define __ctx_lock_type(name) __attribute__((capability(#name)))
+# define __reentrant_ctx_lock __attribute__((reentrant_capability))
+# define __acquires_ctx_lock(...) __attribute__((acquire_capability(__VA_ARGS__)))
+# define __acquires_shared_ctx_lock(...) __attribute__((acquire_shared_capability(__VA_ARGS__)))
+# define __try_acquires_ctx_lock(ret, var) __attribute__((try_acquire_capability(ret, var)))
+# define __try_acquires_shared_ctx_lock(ret, var) __attribute__((try_acquire_shared_capability(ret, var)))
+# define __releases_ctx_lock(...) __attribute__((release_capability(__VA_ARGS__)))
+# define __releases_shared_ctx_lock(...) __attribute__((release_shared_capability(__VA_ARGS__)))
+# define __returns_ctx_lock(var) __attribute__((lock_returned(var)))
+
+/*
+ * The below are used to annotate code being checked. Internal only.
+ */
+# define __excludes_ctx_lock(...) __attribute__((locks_excluded(__VA_ARGS__)))
+# define __requires_ctx_lock(...) __attribute__((requires_capability(__VA_ARGS__)))
+# define __requires_shared_ctx_lock(...) __attribute__((requires_shared_capability(__VA_ARGS__)))
+
+/*
+ * The "assert_capability" attribute is a bit confusingly named. It does not
+ * generate a check. Instead, it tells the analysis to *assume* the capability
+ * is held. This is used for:
+ *
+ * 1. Augmenting runtime assertions, that can then help with patterns beyond the
+ * compiler's static reasoning abilities.
+ *
+ * 2. Initialization of context locks, so we can access guarded variables right
+ * after initialization (nothing else should access the same object yet).
+ */
+# define __assumes_ctx_lock(...) __attribute__((assert_capability(__VA_ARGS__)))
+# define __assumes_shared_ctx_lock(...) __attribute__((assert_shared_capability(__VA_ARGS__)))
+
+/**
+ * __guarded_by - struct member and globals attribute, declares variable
+ * only accessible within active context
+ *
+ * Declares that the struct member or global variable is only accessible within
+ * the context entered by the given context lock. Read operations on the data
+ * require shared access, while write operations require exclusive access.
+ *
+ * .. code-block:: c
+ *
+ * struct some_state {
+ * spinlock_t lock;
+ * long counter __guarded_by(&lock);
+ * };
+ */
+# define __guarded_by(...) __attribute__((guarded_by(__VA_ARGS__)))
+
+/**
+ * __pt_guarded_by - struct member and globals attribute, declares pointed-to
+ * data only accessible within active context
+ *
+ * Declares that the data pointed to by the struct member pointer or global
+ * pointer is only accessible within the context entered by the given context
+ * lock. Read operations on the data require shared access, while write
+ * operations require exclusive access.
+ *
+ * .. code-block:: c
+ *
+ * struct some_state {
+ * spinlock_t lock;
+ * long *counter __pt_guarded_by(&lock);
+ * };
+ */
+# define __pt_guarded_by(...) __attribute__((pt_guarded_by(__VA_ARGS__)))
+
+/**
+ * context_lock_struct() - declare or define a context lock struct
+ * @name: struct name
+ *
+ * Helper to declare or define a struct type that is also a context lock.
+ *
+ * .. code-block:: c
+ *
+ * context_lock_struct(my_handle) {
+ * int foo;
+ * long bar;
+ * };
+ *
+ * struct some_state {
+ * ...
+ * };
+ * // ... declared elsewhere ...
+ * context_lock_struct(some_state);
+ *
+ * Note: The implementation defines several helper functions that can acquire
+ * and release the context lock.
+ */
+# define context_lock_struct(name, ...) \
+ struct __ctx_lock_type(name) __VA_ARGS__ name; \
+ static __always_inline void __acquire_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __acquires_ctx_lock(var) { } \
+ static __always_inline void __acquire_shared_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __acquires_shared_ctx_lock(var) { } \
+ static __always_inline bool __try_acquire_ctx_lock(const struct name *var, bool ret) \
+ __attribute__((overloadable)) __no_context_analysis __try_acquires_ctx_lock(1, var) \
+ { return ret; } \
+ static __always_inline bool __try_acquire_shared_ctx_lock(const struct name *var, bool ret) \
+ __attribute__((overloadable)) __no_context_analysis __try_acquires_shared_ctx_lock(1, var) \
+ { return ret; } \
+ static __always_inline void __release_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __releases_ctx_lock(var) { } \
+ static __always_inline void __release_shared_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __no_context_analysis __releases_shared_ctx_lock(var) { } \
+ static __always_inline void __assume_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __assumes_ctx_lock(var) { } \
+ static __always_inline void __assume_shared_ctx_lock(const struct name *var) \
+ __attribute__((overloadable)) __assumes_shared_ctx_lock(var) { } \
+ struct name
+
+/**
+ * disable_context_analysis() - disables context analysis
+ *
+ * Disables context analysis. Must be paired with a later
+ * enable_context_analysis().
+ */
+# define disable_context_analysis() \
+ __diag_push(); \
+ __diag_ignore_all("-Wunknown-warning-option", "") \
+ __diag_ignore_all("-Wthread-safety", "") \
+ __diag_ignore_all("-Wthread-safety-pointer", "")
+
+/**
+ * enable_context_analysis() - re-enables context analysis
+ *
+ * Re-enables context analysis. Must be paired with a prior
+ * disable_context_analysis().
+ */
+# define enable_context_analysis() __diag_pop()
+
+/**
+ * __no_context_analysis - function attribute, disables context analysis
+ *
+ * Function attribute denoting that context analysis is disabled for the
+ * whole function. Prefer use of `context_unsafe()` where possible.
+ */
+# define __no_context_analysis __attribute__((no_thread_safety_analysis))
+
+#else /* !WARN_CONTEXT_ANALYSIS */
+
+# define __ctx_lock_type(name)
+# define __reentrant_ctx_lock
+# define __acquires_ctx_lock(...)
+# define __acquires_shared_ctx_lock(...)
+# define __try_acquires_ctx_lock(ret, var)
+# define __try_acquires_shared_ctx_lock(ret, var)
+# define __releases_ctx_lock(...)
+# define __releases_shared_ctx_lock(...)
+# define __assumes_ctx_lock(...)
+# define __assumes_shared_ctx_lock(...)
+# define __returns_ctx_lock(var)
+# define __guarded_by(...)
+# define __pt_guarded_by(...)
+# define __excludes_ctx_lock(...)
+# define __requires_ctx_lock(...)
+# define __requires_shared_ctx_lock(...)
+# define __acquire_ctx_lock(var) do { } while (0)
+# define __acquire_shared_ctx_lock(var) do { } while (0)
+# define __try_acquire_ctx_lock(var, ret) (ret)
+# define __try_acquire_shared_ctx_lock(var, ret) (ret)
+# define __release_ctx_lock(var) do { } while (0)
+# define __release_shared_ctx_lock(var) do { } while (0)
+# define __assume_ctx_lock(var) do { (void)(var); } while (0)
+# define __assume_shared_ctx_lock(var) do { (void)(var); } while (0)
+# define context_lock_struct(name, ...) struct __VA_ARGS__ name
+# define disable_context_analysis()
+# define enable_context_analysis()
+# define __no_context_analysis
+
+#endif /* WARN_CONTEXT_ANALYSIS */
+
+/**
+ * context_unsafe() - disable context checking for contained code
+ *
+ * Disables context checking for contained statements or expression.
+ *
+ * .. code-block:: c
+ *
+ * struct some_data {
+ * spinlock_t lock;
+ * int counter __guarded_by(&lock);
+ * };
+ *
+ * int foo(struct some_data *d)
+ * {
+ * // ...
+ * // other code that is still checked ...
+ * // ...
+ * return context_unsafe(d->counter);
+ * }
+ */
+#define context_unsafe(...) \
+({ \
+ disable_context_analysis(); \
+ __VA_ARGS__; \
+ enable_context_analysis() \
+})
+
+/**
+ * __context_unsafe() - function attribute, disable context checking
+ * @comment: comment explaining why opt-out is safe
+ *
+ * Function attribute denoting that context analysis is disabled for the
+ * whole function. Forces adding an inline comment as argument.
+ */
+#define __context_unsafe(comment) __no_context_analysis
+
+/**
+ * context_unsafe_alias() - helper to insert a context lock "alias barrier"
+ * @p: pointer aliasing a context lock or object containing context locks
+ *
+ * No-op function that acts as a "context lock alias barrier", where the
+ * analysis rightfully detects that we're switching aliases, but the switch is
+ * considered safe but beyond the analysis reasoning abilities.
+ *
+ * This should be inserted before the first use of such an alias.
+ *
+ * Implementation Note: The compiler ignores aliases that may be reassigned but
+ * their value cannot be determined (e.g. when passing a non-const pointer to an
+ * alias as a function argument).
+ */
+#define context_unsafe_alias(p) _context_unsafe_alias((void **)&(p))
+static inline void _context_unsafe_alias(void **p) { }
+
+/**
+ * token_context_lock() - declare an abstract global context lock instance
+ * @name: token context lock name
+ *
+ * Helper that declares an abstract global context lock instance @name, but not
+ * backed by a real data structure (linker error if accidentally referenced).
+ * The type name is `__ctx_lock_@name`.
+ */
+#define token_context_lock(name, ...) \
+ context_lock_struct(__ctx_lock_##name, ##__VA_ARGS__) {}; \
+ extern const struct __ctx_lock_##name *name
+
+/**
+ * token_context_lock_instance() - declare another instance of a global context lock
+ * @ctx: token context lock previously declared with token_context_lock()
+ * @name: name of additional global context lock instance
+ *
+ * Helper that declares an additional instance @name of the same token context
+ * lock class @ctx. This is helpful where multiple related token contexts are
+ * declared, to allow using the same underlying type (`__ctx_lock_@ctx`) as
+ * function arguments.
+ */
+#define token_context_lock_instance(ctx, name) \
+ extern const struct __ctx_lock_##ctx *name
+
+/*
+ * Common keywords for static context analysis. Both Clang's "capability
+ * analysis" and Sparse's "context tracking" are currently supported.
+ */
#ifdef __CHECKER__
/* Sparse context/lock checking support. */
# define __must_hold(x) __attribute__((context(x,1,1)))
+# define __must_not_hold(x)
# define __acquires(x) __attribute__((context(x,0,1)))
# define __cond_acquires(x) __attribute__((context(x,0,-1)))
# define __releases(x) __attribute__((context(x,1,0)))
# define __acquire(x) __context__(x,1)
# define __release(x) __context__(x,-1)
# define __cond_lock(x, c) ((c) ? ({ __acquire(x); 1; }) : 0)
+/* For Sparse, there's no distinction between exclusive and shared locks. */
+# define __must_hold_shared __must_hold
+# define __acquires_shared __acquires
+# define __cond_acquires_shared __cond_acquires
+# define __releases_shared __releases
+# define __acquire_shared __acquire
+# define __release_shared __release
+# define __cond_lock_shared __cond_acquire
#else /* !__CHECKER__ */
-# define __must_hold(x)
-# define __acquires(x)
-# define __cond_acquires(x)
-# define __releases(x)
-# define __acquire(x) (void)0
-# define __release(x) (void)0
-# define __cond_lock(x, c) (c)
+/**
+ * __must_hold() - function attribute, caller must hold exclusive context lock
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the caller must hold the given context
+ * lock instance @x exclusively.
+ */
+# define __must_hold(x) __requires_ctx_lock(x)
+
+/**
+ * __must_not_hold() - function attribute, caller must not hold context lock
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the caller must not hold the given context
+ * lock instance @x.
+ */
+# define __must_not_hold(x) __excludes_ctx_lock(x)
+
+/**
+ * __acquires() - function attribute, function acquires context lock exclusively
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function acquires the given context
+ * lock instance @x exclusively, but does not release it.
+ */
+# define __acquires(x) __acquires_ctx_lock(x)
+
+/**
+ * __cond_acquires() - function attribute, function conditionally
+ * acquires a context lock exclusively
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function conditionally acquires the
+ * given context lock instance @x exclusively, but does not release it.
+ */
+# define __cond_acquires(x) __try_acquires_ctx_lock(1, x)
+
+/**
+ * __releases() - function attribute, function releases a context lock exclusively
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function releases the given context
+ * lock instance @x exclusively. The associated context must be active on
+ * entry.
+ */
+# define __releases(x) __releases_ctx_lock(x)
+
+/**
+ * __acquire() - function to acquire context lock exclusively
+ * @x: context lock instance pointer
+ *
+ * No-op function that acquires the given context lock instance @x exclusively.
+ */
+# define __acquire(x) __acquire_ctx_lock(x)
+
+/**
+ * __release() - function to release context lock exclusively
+ * @x: context lock instance pointer
+ *
+ * No-op function that releases the given context lock instance @x.
+ */
+# define __release(x) __release_ctx_lock(x)
+
+/**
+ * __cond_lock() - function that conditionally acquires a context lock
+ * exclusively
+ * @x: context lock instance pinter
+ * @c: boolean expression
+ *
+ * Return: result of @c
+ *
+ * No-op function that conditionally acquires context lock instance @x
+ * exclusively, if the boolean expression @c is true. The result of @c is the
+ * return value; for example:
+ *
+ * .. code-block:: c
+ *
+ * #define spin_trylock(l) __cond_lock(&lock, _spin_trylock(&lock))
+ */
+# define __cond_lock(x, c) __try_acquire_ctx_lock(x, c)
+
+/**
+ * __must_hold_shared() - function attribute, caller must hold shared context lock
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the caller must hold the given context
+ * lock instance @x with shared access.
+ */
+# define __must_hold_shared(x) __requires_shared_ctx_lock(x)
+
+/**
+ * __acquires_shared() - function attribute, function acquires context lock shared
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function acquires the given
+ * context lock instance @x with shared access, but does not release it.
+ */
+# define __acquires_shared(x) __acquires_shared_ctx_lock(x)
+
+/**
+ * __cond_acquires_shared() - function attribute, function conditionally
+ * acquires a context lock shared
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function conditionally acquires the
+ * given context lock instance @x with shared access, but does not release it.
+ */
+# define __cond_acquires_shared(x) __try_acquires_shared_ctx_lock(1, x)
+
+/**
+ * __releases_shared() - function attribute, function releases a
+ * context lock shared
+ * @x: context lock instance pointer
+ *
+ * Function attribute declaring that the function releases the given context
+ * lock instance @x with shared access. The associated context must be active
+ * on entry.
+ */
+# define __releases_shared(x) __releases_shared_ctx_lock(x)
+
+/**
+ * __acquire_shared() - function to acquire context lock shared
+ * @x: context lock instance pointer
+ *
+ * No-op function that acquires the given context lock instance @x with shared
+ * access.
+ */
+# define __acquire_shared(x) __acquire_shared_ctx_lock(x)
+
+/**
+ * __release_shared() - function to release context lock shared
+ * @x: context lock instance pointer
+ *
+ * No-op function that releases the given context lock instance @x with shared
+ * access.
+ */
+# define __release_shared(x) __release_shared_ctx_lock(x)
+
+/**
+ * __cond_lock_shared() - function that conditionally acquires a context lock shared
+ * @x: context lock instance pinter
+ * @c: boolean expression
+ *
+ * Return: result of @c
+ *
+ * No-op function that conditionally acquires context lock instance @x with
+ * shared access, if the boolean expression @c is true. The result of @c is the
+ * return value.
+ */
+# define __cond_lock_shared(x, c) __try_acquire_shared_ctx_lock(x, c)
#endif /* __CHECKER__ */
+/**
+ * __acquire_ret() - helper to acquire context lock of return value
+ * @call: call expression
+ * @ret_expr: acquire expression that uses __ret
+ */
+#define __acquire_ret(call, ret_expr) \
+ ({ \
+ __auto_type __ret = call; \
+ __acquire(ret_expr); \
+ __ret; \
+ })
+
+/**
+ * __acquire_shared_ret() - helper to acquire context lock shared of return value
+ * @call: call expression
+ * @ret_expr: acquire shared expression that uses __ret
+ */
+#define __acquire_shared_ret(call, ret_expr) \
+ ({ \
+ __auto_type __ret = call; \
+ __acquire_shared(ret_expr); \
+ __ret; \
+ })
+
+/*
+ * Attributes to mark functions returning acquired context locks.
+ *
+ * This is purely cosmetic to help readability, and should be used with the
+ * above macros as follows:
+ *
+ * struct foo { spinlock_t lock; ... };
+ * ...
+ * #define myfunc(...) __acquire_ret(_myfunc(__VA_ARGS__), &__ret->lock)
+ * struct foo *_myfunc(int bar) __acquires_ret;
+ * ...
+ */
+#define __acquires_ret __no_context_analysis
+#define __acquires_shared_ret __no_context_analysis
+
#endif /* _LINUX_COMPILER_CONTEXT_ANALYSIS_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ba36939fda79..cd557e7653a4 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -621,6 +621,36 @@ config DEBUG_FORCE_WEAK_PER_CPU
To ensure that generic code follows the above rules, this
option forces all percpu variables to be defined as weak.
+config WARN_CONTEXT_ANALYSIS
+ bool "Compiler context-analysis warnings"
+ depends on CC_IS_CLANG && CLANG_VERSION >= 220000
+ # Branch profiling re-defines "if", which messes with the compiler's
+ # ability to analyze __cond_acquires(..), resulting in false positives.
+ depends on !TRACE_BRANCH_PROFILING
+ default y
+ help
+ Context Analysis is a language extension, which enables statically
+ checking that required contexts are active (or inactive) by acquiring
+ and releasing user-definable "context locks".
+
+ Clang's name of the feature is "Thread Safety Analysis". Requires
+ Clang 22 or later.
+
+ Produces warnings by default. Select CONFIG_WERROR if you wish to
+ turn these warnings into errors.
+
+ For more details, see Documentation/dev-tools/context-analysis.rst.
+
+config WARN_CONTEXT_ANALYSIS_ALL
+ bool "Enable context analysis for all source files"
+ depends on WARN_CONTEXT_ANALYSIS
+ depends on EXPERT && !COMPILE_TEST
+ help
+ Enable tree-wide context analysis. This is likely to produce a
+ large number of false positives - enable at your own risk.
+
+ If unsure, say N.
+
endmenu # "Compiler options"
menu "Generic Kernel Debugging Instruments"
diff --git a/scripts/Makefile.context-analysis b/scripts/Makefile.context-analysis
new file mode 100644
index 000000000000..70549f7fae1a
--- /dev/null
+++ b/scripts/Makefile.context-analysis
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+
+context-analysis-cflags := -DWARN_CONTEXT_ANALYSIS \
+ -fexperimental-late-parse-attributes -Wthread-safety \
+ -Wthread-safety-pointer -Wthread-safety-beta
+
+export CFLAGS_CONTEXT_ANALYSIS := $(context-analysis-cflags)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 28a1c08e3b22..e429d68b8594 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -105,6 +105,16 @@ _c_flags += $(if $(patsubst n%,, \
-D__KCSAN_INSTRUMENT_BARRIERS__)
endif
+#
+# Enable context analysis flags only where explicitly opted in.
+# (depends on variables CONTEXT_ANALYSIS_obj.o, CONTEXT_ANALYSIS)
+#
+ifeq ($(CONFIG_WARN_CONTEXT_ANALYSIS),y)
+_c_flags += $(if $(patsubst n%,, \
+ $(CONTEXT_ANALYSIS_$(target-stem).o)$(CONTEXT_ANALYSIS)$(if $(is-kernel-object),$(CONFIG_WARN_CONTEXT_ANALYSIS_ALL))), \
+ $(CFLAGS_CONTEXT_ANALYSIS))
+endif
+
#
# Enable AutoFDO build flags except some files or directories we don't want to
# enable (depends on variables AUTOFDO_PROFILE_obj.o and AUTOFDO_PROFILE).
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 03/36] compiler-context-analysis: Add test stub
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Add a simple test stub where we will add common supported patterns that
should not generate false positives for each new supported context lock.
Signed-off-by: Marco Elver <elver@google.com>
---
v5:
* Rename "context guard" -> "context lock".
v4:
* Rename capability -> context analysis.
---
lib/Kconfig.debug | 14 ++++++++++++++
lib/Makefile | 3 +++
lib/test_context-analysis.c | 18 ++++++++++++++++++
3 files changed, 35 insertions(+)
create mode 100644 lib/test_context-analysis.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cd557e7653a4..8ca42526ee43 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2835,6 +2835,20 @@ config LINEAR_RANGES_TEST
If unsure, say N.
+config CONTEXT_ANALYSIS_TEST
+ bool "Compiler context-analysis warnings test"
+ depends on EXPERT
+ help
+ This builds the test for compiler-based context analysis. The test
+ does not add executable code to the kernel, but is meant to test that
+ common patterns supported by the analysis do not result in false
+ positive warnings.
+
+ When adding support for new context locks, it is strongly recommended
+ to add supported patterns to this test.
+
+ If unsure, say N.
+
config CMDLINE_KUNIT_TEST
tristate "KUnit test for cmdline API" if !KUNIT_ALL_TESTS
depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index aaf677cf4527..89defefbf6c0 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -331,4 +331,7 @@ obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
obj-$(CONFIG_FIRMWARE_TABLE) += fw_table.o
+CONTEXT_ANALYSIS_test_context-analysis.o := y
+obj-$(CONFIG_CONTEXT_ANALYSIS_TEST) += test_context-analysis.o
+
subdir-$(CONFIG_FORTIFY_SOURCE) += test_fortify
diff --git a/lib/test_context-analysis.c b/lib/test_context-analysis.c
new file mode 100644
index 000000000000..68f075dec0e0
--- /dev/null
+++ b/lib/test_context-analysis.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Compile-only tests for common patterns that should not generate false
+ * positive errors when compiled with Clang's context analysis.
+ */
+
+#include <linux/build_bug.h>
+
+/*
+ * Test that helper macros work as expected.
+ */
+static void __used test_common_helpers(void)
+{
+ BUILD_BUG_ON(context_unsafe(3) != 3); /* plain expression */
+ BUILD_BUG_ON(context_unsafe((void)2; 3) != 3); /* does not swallow semi-colon */
+ BUILD_BUG_ON(context_unsafe((void)2, 3) != 3); /* does not swallow commas */
+ context_unsafe(do { } while (0)); /* works with void statements */
+}
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 04/36] Documentation: Add documentation for Compiler-Based Context Analysis
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Adds documentation in Documentation/dev-tools/context-analysis.rst, and
adds it to the index.
Signed-off-by: Marco Elver <elver@google.com>
---
v5:
* Rename "context guard" -> "context lock".
v4:
* Rename capability -> context analysis.
v2:
* Remove cross-reference to Sparse, since we plan to remove Sparse
support anyway.
* Mention __no_context_analysis should be avoided.
---
Documentation/dev-tools/context-analysis.rst | 144 +++++++++++++++++++
Documentation/dev-tools/index.rst | 1 +
2 files changed, 145 insertions(+)
create mode 100644 Documentation/dev-tools/context-analysis.rst
diff --git a/Documentation/dev-tools/context-analysis.rst b/Documentation/dev-tools/context-analysis.rst
new file mode 100644
index 000000000000..47eb547eb716
--- /dev/null
+++ b/Documentation/dev-tools/context-analysis.rst
@@ -0,0 +1,144 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. Copyright (C) 2025, Google LLC.
+
+.. _context-analysis:
+
+Compiler-Based Context Analysis
+===============================
+
+Context Analysis is a language extension, which enables statically checking
+that required contexts are active (or inactive) by acquiring and releasing
+user-definable "context locks". An obvious application is lock-safety checking
+for the kernel's various synchronization primitives (each of which represents a
+"context lock"), and checking that locking rules are not violated.
+
+The Clang compiler currently supports the full set of context analysis
+features. To enable for Clang, configure the kernel with::
+
+ CONFIG_WARN_CONTEXT_ANALYSIS=y
+
+The feature requires Clang 22 or later.
+
+The analysis is *opt-in by default*, and requires declaring which modules and
+subsystems should be analyzed in the respective `Makefile`::
+
+ CONTEXT_ANALYSIS_mymodule.o := y
+
+Or for all translation units in the directory::
+
+ CONTEXT_ANALYSIS := y
+
+It is possible to enable the analysis tree-wide, however, which will result in
+numerous false positive warnings currently and is *not* generally recommended::
+
+ CONFIG_WARN_CONTEXT_ANALYSIS_ALL=y
+
+Programming Model
+-----------------
+
+The below describes the programming model around using context lock types.
+
+.. note::
+ Enabling context analysis can be seen as enabling a dialect of Linux C with
+ a Context System. Some valid patterns involving complex control-flow are
+ constrained (such as conditional acquisition and later conditional release
+ in the same function).
+
+Context analysis is a way to specify permissibility of operations to depend on
+context locks being held (or not held). Typically we are interested in
+protecting data and code in a critical section by requiring a specific context
+to be active, for example by holding a specific lock. The analysis ensures that
+callers cannot perform an operation without the required context being active.
+
+Context locks are associated with named structs, along with functions that
+operate on struct instances to acquire and release the associated context lock.
+
+Context locks can be held either exclusively or shared. This mechanism allows
+assigning more precise privileges when a context is active, typically to
+distinguish where a thread may only read (shared) or also write (exclusive) to
+data guarded within a context.
+
+The set of contexts that are actually active in a given thread at a given point
+in program execution is a run-time concept. The static analysis works by
+calculating an approximation of that set, called the context environment. The
+context environment is calculated for every program point, and describes the
+set of contexts that are statically known to be active, or inactive, at that
+particular point. This environment is a conservative approximation of the full
+set of contexts that will actually be active in a thread at run-time.
+
+More details are also documented `here
+<https://clang.llvm.org/docs/ThreadSafetyAnalysis.html>`_.
+
+.. note::
+ Clang's analysis explicitly does not infer context locks acquired or
+ released by inline functions. It requires explicit annotations to (a) assert
+ that it's not a bug if a context lock is released or acquired, and (b) to
+ retain consistency between inline and non-inline function declarations.
+
+Supported Kernel Primitives
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. Currently the following synchronization primitives are supported:
+
+For context locks with an initialization function (e.g., `spin_lock_init()`),
+calling this function before initializing any guarded members or globals
+prevents the compiler from issuing warnings about unguarded initialization.
+
+Lockdep assertions, such as `lockdep_assert_held()`, inform the compiler's
+context analysis that the associated synchronization primitive is held after
+the assertion. This avoids false positives in complex control-flow scenarios
+and encourages the use of Lockdep where static analysis is limited. For
+example, this is useful when a function doesn't *always* require a lock, making
+`__must_hold()` inappropriate.
+
+Keywords
+~~~~~~~~
+
+.. kernel-doc:: include/linux/compiler-context-analysis.h
+ :identifiers: context_lock_struct
+ token_context_lock token_context_lock_instance
+ __guarded_by __pt_guarded_by
+ __must_hold
+ __must_not_hold
+ __acquires
+ __cond_acquires
+ __releases
+ __must_hold_shared
+ __acquires_shared
+ __cond_acquires_shared
+ __releases_shared
+ __acquire
+ __release
+ __cond_lock
+ __acquire_shared
+ __release_shared
+ __cond_lock_shared
+ __acquire_ret
+ __acquire_shared_ret
+ context_unsafe
+ __context_unsafe
+ disable_context_analysis enable_context_analysis
+
+.. note::
+ The function attribute `__no_context_analysis` is reserved for internal
+ implementation of context lock types, and should be avoided in normal code.
+
+Background
+----------
+
+Clang originally called the feature `Thread Safety Analysis
+<https://clang.llvm.org/docs/ThreadSafetyAnalysis.html>`_, with some keywords
+and documentation still using the thread-safety-analysis-only terminology. This
+was later changed and the feature became more flexible, gaining the ability to
+define custom "capabilities". Its foundations can be found in `Capability
+Systems <https://www.cs.cornell.edu/talc/papers/capabilities.pdf>`_, used to
+specify the permissibility of operations to depend on some "capability" being
+held (or not held).
+
+Because the feature is not just able to express capabilities related to
+synchronization primitives, and "capability" is already overloaded in the
+kernel, the naming chosen for the kernel departs from Clang's initial "Thread
+Safety" and "capability" nomenclature; we refer to the feature as "Context
+Analysis" to avoid confusion. The internal implementation still makes
+references to Clang's terminology in a few places, such as `-Wthread-safety`
+being the warning option that also still appears in diagnostic messages.
diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 4b8425e348ab..d864b3da4cc7 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -21,6 +21,7 @@ Documentation/process/debugging/index.rst
checkpatch
clang-format
coccinelle
+ context-analysis
sparse
kcov
gcov
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 05/36] checkpatch: Warn about context_unsafe() without comment
From: Marco Elver @ 2025-12-19 15:39 UTC (permalink / raw)
To: elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Bart Van Assche,
Christoph Hellwig, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Johannes Berg, Jonathan Corbet,
Josh Triplett, Justin Stitt, Kees Cook, Kentaro Takeda,
Lukas Bulwahn, Mark Rutland, Mathieu Desnoyers, Miguel Ojeda,
Nathan Chancellor, Neeraj Upadhyay, Nick Desaulniers,
Steven Rostedt, Tetsuo Handa, Thomas Gleixner, Thomas Graf,
Uladzislau Rezki, Waiman Long, kasan-dev, linux-crypto, linux-doc,
linux-kbuild, linux-kernel, linux-mm, linux-security-module,
linux-sparse, linux-wireless, llvm, rcu
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Warn about applications of context_unsafe() without a comment, to
encourage documenting the reasoning behind why it was deemed safe.
Signed-off-by: Marco Elver <elver@google.com>
---
v4:
* Rename capability -> context analysis.
* Avoid nested if.
---
scripts/checkpatch.pl | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c0250244cf7a..c4fd8bdff528 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6733,6 +6733,13 @@ sub process {
}
}
+# check for context_unsafe without a comment.
+ if ($line =~ /\bcontext_unsafe\b/ &&
+ !ctx_has_comment($first_line, $linenr)) {
+ WARN("CONTEXT_UNSAFE",
+ "context_unsafe without comment\n" . $herecurr);
+ }
+
# check of hardware specific defines
if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) {
CHK("ARCH_DEFINES",
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox