* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-15 15:53 UTC (permalink / raw)
To: Peter Zijlstra
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: <aUAPbFJSv0alh_ix@elver.google.com>
On Mon, 15 Dec 2025 at 14:38, Marco Elver <elver@google.com> wrote:
>
> On Fri, Dec 12, 2025 at 12:09PM +0100, Peter Zijlstra wrote:
> > On Fri, Dec 12, 2025 at 11:15:29AM +0100, Marco Elver wrote:
> > > On Fri, 12 Dec 2025 at 10:43, Peter Zijlstra <peterz@infradead.org> wrote:
> > > [..]
> > > > > Correct. We're trading false negatives over false positives at this
> > > > > point, just to get things to compile cleanly.
> > > >
> > > > Right, and this all 'works' right up to the point someone sticks a
> > > > must_not_hold somewhere.
> > > >
> > > > > > > Better support for Linux's scoped guard design could be added in
> > > > > > > future if deemed critical.
> > > > > >
> > > > > > I would think so, per the above I don't think this is 'right'.
> > > > >
> > > > > It's not sound, but we'll avoid false positives for the time being.
> > > > > Maybe we can wrangle the jigsaw of macros to let it correctly acquire
> > > > > and then release (via a 2nd cleanup function), it might be as simple
> > > > > as marking the 'constructor' with the right __acquires(..), and then
> > > > > have a 2nd __attribute__((cleanup)) variable that just does a no-op
> > > > > release via __release(..) so we get the already supported pattern
> > > > > above.
> > > >
> > > > Right, like I mentioned in my previous email; it would be lovely if at
> > > > the very least __always_inline would get a *very* early pass such that
> > > > the above could be resolved without inter-procedural bits. I really
> > > > don't consider an __always_inline as another procedure.
> > > >
> > > > Because as I already noted yesterday, cleanup is now all
> > > > __always_inline, and as such *should* all end up in the one function.
> > > >
> > > > But yes, if we can get a magical mash-up of __cleanup and __release (let
> > > > it be knows as __release_on_cleanup ?) that might also work I suppose.
> > > > But I vastly prefer __always_inline actually 'working' ;-)
> > >
> > > The truth is that __always_inline working in this way is currently
> > > infeasible. Clang and LLVM's architecture simply disallow this today:
> > > the semantic analysis that -Wthread-safety does happens over the AST,
> > > whereas always_inline is processed by early passes in the middle-end
> > > already within LLVM's pipeline, well after semantic analysis. There's
> > > a complexity budget limit for semantic analysis (type checking,
> > > warnings, assorted other errors), and path-sensitive &
> > > intra-procedural analysis over the plain AST is outside that budget.
> > > Which is why tools like clang-analyzer exist (symbolic execution),
> > > where it's possible to afford that complexity since that's not
> > > something that runs for a normal compile.
> > >
> > > I think I've pushed the current version of Clang's -Wthread-safety
> > > already far beyond what folks were thinking is possible (a variant of
> > > alias analysis), but even my healthy disregard for the impossible
> > > tells me that making path-sensitive intra-procedural analysis even if
> > > just for __always_inline functions is quite possibly a fool's errand.
> >
> > Well, I had to propose it. Gotta push the envelope :-)
> >
> > > So either we get it to work with what we have, or give up.
> >
> > So I think as is, we can start. But I really do want the cleanup thing
> > sorted, even if just with that __release_on_cleanup mashup or so.
>
> Working on rebasing this to v6.19-rc1 and saw this new scoped seqlock
> abstraction. For that one I was able to make it work like I thought we
> could (below). Some awkwardness is required to make it work in
> for-loops, which only let you define variables with the same type.
>
> For <linux/cleanup.h> it needs some more thought due to extra levels of
> indirection.
For cleanup.h, the problem is that to instantiate we use
"guard(class)(args..)". If it had been designed as "guard(class,
args...)", i.e. just use __VA_ARGS__ explicitly instead of the
implicit 'args...', it might have been possible to add a second
cleanup variable to do the same (with some additional magic to extract
the first arg if one exists). Unfortunately, the use of the current
guard()() idiom has become so pervasive that this is a bigger
refactor. I'm going to leave cleanup.h as-is for now, if we think we
want to give this a go in the current state.
One observation from the rebase: Generally synchronization primitives
do not change much and the annotations are relatively stable, but e.g.
RCU & sched (latter is optional and depends on the sched-enablement
patch) receive disproportionally more changes, and while new
annotations required for v6.19-rc1 were trivial, it does require
compiling with a Clang version that does produce the warnings to
notice.
While Clang 22-dev is being tested on CI, I doubt maintainers already
use it, so it's possible we'll see some late warnings due to missing
annotations when things hit -next. This might be an acceptable churn
cost, if we think the outcome is worthwhile. Things should get better
when Clang 22 is released properly, but until then things might be a
little bumpy if there are large changes across the core
synchronization primitives.
Thanks,
-- Marco
^ permalink raw reply
* Re: A formal request for process clarifications.
From: Dr. Greg @ 2025-12-15 16:57 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, corbet
In-Reply-To: <CAHk-=whqzs-3u6Y7UC03A_XJEy6H1kNWvFO_A8jqsuob7SZCLA@mail.gmail.com>
On Mon, Dec 15, 2025 at 07:38:58PM +1200, Linus Torvalds wrote:
Good morning Linus, thanks for taking the time to respond.
> On Mon, 15 Dec 2025 at 19:13, Dr. Greg <greg@enjellic.com> wrote:
> >
> > Three years ago our team had submitted for review our TSEM LSM that
> > provides a framework for generic security modeling,
> If you can't convince the LSM people to take your code, you sure can't
> convince me.
>
> I already think we have too many of those pointless things. There's a
> fine line between diversity and "too much confusion because everybody
> thinks they know best". And the linux security modules passed that
> line years ago.
>
> So my suggestion is to standardize on normal existing security models
> instead of thinking that you can do better by making yet another one.
> Or at least work with the existing people instead of trying to bypass
> them and ignoring what they tell you.
>
> Yes, I know that security people always think they know best, and they
> all disagree with each other, which is why we already have tons of
> security modules. Ask ten people what model is the right one, and you
> get fifteen different answers.
>
> I'm not in the least interested in becoming some kind of arbiter or
> voice of sanity in this.
First, to be very clear, we are not asking for any kind of
intervention or arbitration on your part.
Second and most importantly. You've been belly-aching about this
problem for as long as I can remember and you I go back to 1992
together with Linux.
You, and only you, can fix the problem if you want it fixed. Issue an
immediate statement that you will no longer accept any code that
implements an 'LSM'.
That will drive security development out of the kernel, which is where
it is going to go eventually anyway OR it will drive the security
community to try and fix what it considers to be the challenges with
eBPF when it comes to building security solutions.
Somewhat paradoxically in all of this, TSEM isn't even an LSM that
implements security policy. It is generic infrastructure that was
built to address the very problem you are bitching about.
If Linux is really about technology, as you have continually
advocated, then there has to be an open playing field for
contributors. Absent that, Linux will balkanize, the same way the
commercial Unix implementations did, around corporate driven
interests and motivations.
We will pursue the open playing field issue through the TAB if
necessary.
> Linus
Once again, with all due respect, fix the problem if it annoys you,
you would be doing a lot of people a favor.
Best wishes for a pleasant holiday season to you and your family.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [RFC 00/11] Reintroduce Hornet LSM
From: Ryan Foster @ 2025-12-15 17:45 UTC (permalink / raw)
To: bboscaccy
Cc: James.Bottomley, akpm, bpf, corbet, dhowells, gnoack, jmorris,
linux-doc, linux-kernel, linux-security-module, linux, mic, paul,
serge, Ryan
In-Reply-To: <20251211021257.1208712-1-bboscaccy@linux.microsoft.com>
From: Ryan <foster.ryan.r@gmail.com>
Hi all,
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.
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 Hornet is aiming for, and is attach 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,
Ryan
^ permalink raw reply
* Re: An opinion about Linux security
From: Casey Schaufler @ 2025-12-15 17:44 UTC (permalink / raw)
To: Dr. Greg; +Cc: Timur Chernykh, torvalds, linux-security-module
In-Reply-To: <20251215045524.GA6104@wind.enjellic.com>
On 12/14/2025 8:55 PM, Dr. Greg wrote:
> On Fri, Dec 12, 2025 at 03:43:07PM -0800, Casey Schaufler wrote:
>
> Good morning Casey, pleasant as always to hear from you.
>
>> On 12/11/2025 9:45 PM, Dr. Greg wrote:
>>> On Wed, Dec 10, 2025 at 03:15:39AM +0300, Timur Chernykh wrote:
>>>
>>> Good morning Timur, I hope this note finds your week having gone well.
>>>
>>>> Hello Linus,
>>>>
>>>> I'm writing to ask for your opinion. What do you think about Linux's
>>>> current readiness for security-focused commercial products? I'm
>>>> particularly interested in several areas.
>>> I don't expect you will receive an answer.
>>>
>>> Based on his previous comments and long standing position on this
>>> issue, I believe it can be fairly stated that he looks at the LSM as
>>> an unnecessary evil.
>>>
>>> So in his absence, some 'in loco parentis' reflections on the issues
>>> you raise.
>>>
>>> I've been advised, more than once, that in this day and age, no one is
>>> interested in reading more than a two sentence paragraph, so a short
>>> response to your issues here and a bit more detail for anyone who
>>> wants to read more, at the end.
>>>
>>> There is active art available to address the shortcomings you outline
>>> in your post below. Our TSEM LSM was designed to service the
>>> realitities of the modern security environment and where it is going.
>>> In a manner that doesn't provide any restrictions on how 'security'
>>> can be implemented.
>>>
>>> We've done four releases over three years and we believe an unbiased
>>> observer would conclude they have received no substantive technical
>>> review that would support interest in upstream integration.
>> Stop. Really, I mean it. I put significant effort into trying to teach
>> you how to submit a patch set that could be reviewed. You ignored it.
>> I can't speak to what an "unbiased observer" would conclude because
>> your behavior has certainly left me with bias. Rather than writing
>> full length novels about why you submitted patches the way you've
>> done it you might consider heeding the advice. Grrr.
> No, we are not going to stop, see immediately below.
Rather than addressing the issues you again explain, in great detail,
why you're right about everything. And I never hit the enter key with my
pinky.
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 19:43 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <aT_Lh8l3E2yQJYI7@kernel.org>
On Mon, Dec 15, 2025 at 10:49:15AM +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 15, 2025 at 04:55:58PM +0900, James Bottomley wrote:
> > On Mon, 2025-12-15 at 08:43 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Dec 15, 2025 at 07:18:41AM +0900, James Bottomley wrote:
> > > > On Sun, 2025-12-14 at 23:32 +0200, Jarkko Sakkinen wrote:
> > > > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and
> > > > > thus its use should be pooled rather than directly used. This
> > > > > both reduces latency and improves its predictability.
> > > > >
> > > > > 2. Linux is better off overall if every subsystem uses the same
> > > > > source for the random bistream as the de-facto choice, unless
> > > > > *force majeure* reasons point to some other direction.
> > > > >
> > > > > In the case, of TPM there is no reason for trusted keys to invoke
> > > > > TPM directly.
> > > >
> > > > That assertion isn't correct: you seem to have forgotten we had
> > > > this argument six or seven years ago, but even that was a reprise
> > > > of an even earlier one. Lore doesn't go back far enough for the
> > > > intermediate one on the tpm list, but the original was cc'd to
> > > > lkml:
> > > >
> > > > https://lore.kernel.org/all/1378920168.26698.64.camel@localhost/
> > > >
> > > > The decision then was to use the same random source as the key
> > > > protection. Unfortunately most of the active participants have
> > > > moved on from IBM and I don't have their current email addresses,
> > > > but the bottom line is there were good reasons to do trusted keys
> > > > this way that your assertions above don't overcome. I'm not saying
> > > > we shouldn't reconsider the situation, but we need a reasoned
> > > > debate rather than simply doing it by fiat.
> > >
> > > The way I see this is that given that kernel is not running inside
> > > TPM, FIPS certification of the RNG does not have any measurable
> > > value.
> > >
> > > Random data generation should happen as part of object creation
> > > process i.e. should be fully self-contained process within the TPM in
> > > order for FIPS to matter.
> >
> > In FIPS terms, there's no distinction between keeping the whole
> > generation process internal to the TPM and using the FIPS certified rng
> > of the TPM to source the contents of a kernel protected key. Both
> > provide equally valid, and FIPS certified data.
>
> I understand being "FIPS certified" embedding the premise that kernel
> is also FIPS certified, which covers also crypto etc. This is the case
> with enterprise kernels.
>
> I have understanding FIPS certification dies at the point when random
> data is acquired by a kernel, which is not FIPS certified. It's not
> really a safe closure.
>
> Using same code path for RNG universally should actually help with any
> certification processes.
I think there is misunderstanding with FIPS.
Having FIPS certificated RNG in TPM matters but it only matters only in
the sense that callers can be FIPS certified as they use that RNG as a
source.
Using FIPS certified RNG does not magically make callers be FIPS
ceritified actors. The data is contaminated in that sense at the point
when kernel acquires it.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: James Bottomley @ 2025-12-15 20:01 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <aUBk2nUpd2V8p9qc@kernel.org>
On Mon, 2025-12-15 at 21:43 +0200, Jarkko Sakkinen wrote:
[...]
> I think there is misunderstanding with FIPS.
>
> Having FIPS certificated RNG in TPM matters but it only matters only
> in the sense that callers can be FIPS certified as they use that RNG
> as a source.
>
> Using FIPS certified RNG does not magically make callers be FIPS
> ceritified actors. The data is contaminated in that sense at the
> point when kernel acquires it.
I think FIPS certification is a red herring. The point being made in
the original thread is about RNG quality. The argument essentially
being that the quality of the TPM RNG is known at all points in time
but the quality of the kernel RNG (particularly at start of day when
the entropy pool is new) is less certain.
Regards,
James
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Eric Biggers @ 2025-12-15 20:09 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,
Jason A. Donenfeld
In-Reply-To: <20251214213236.339586-1-jarkko@kernel.org>
On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> the random bistream as the de-facto choice, unless *force majeure*
> reasons point to some other direction.
>
> In the case, of TPM there is no reason for trusted keys to invoke TPM
> directly.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..33b7739741c3 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Agreed that kernel code should prefer the standard Linux RNG whenever
possible. Note that the standard Linux RNG already incorporates entropy
from hardware RNGs, when available.
- Eric
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 20:25 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <5446f517848338b4ccac8d7bbedf4cc1ed315cb4.camel@HansenPartnership.com>
On Mon, Dec 15, 2025 at 09:01:49PM +0100, James Bottomley wrote:
> On Mon, 2025-12-15 at 21:43 +0200, Jarkko Sakkinen wrote:
> [...]
> > I think there is misunderstanding with FIPS.
> >
> > Having FIPS certificated RNG in TPM matters but it only matters only
> > in the sense that callers can be FIPS certified as they use that RNG
> > as a source.
> >
> > Using FIPS certified RNG does not magically make callers be FIPS
> > ceritified actors. The data is contaminated in that sense at the
> > point when kernel acquires it.
>
> I think FIPS certification is a red herring. The point being made in
> the original thread is about RNG quality. The argument essentially
> being that the quality of the TPM RNG is known at all points in time
> but the quality of the kernel RNG (particularly at start of day when
> the entropy pool is new) is less certain.
OK, that's fair point.
I.e., using TPM2_GetRandom here makes sense, not because of FIPS
certification per se but because it is guarantees matching entropy to
other types of keys generated with TPM2_Create (as everything uses the
same RNG).
I can buy this but think it would really make sense to add a comment to
the source code.
I was thinking something along the lines of:
/*
* tpm_get_random() is used here directly instead of relying kernel's
* RNG in order to match RNGs with objects generated by TPM internally.
*/
It does not mention FIPS explicitly because I think this is already
enforcing condition and thus enough. And e.g., applies also when one
uses an emulator (and thus useful tidbit for that use and purpose).
>
> Regards,
>
> James
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 20:35 UTC (permalink / raw)
To: Eric Biggers
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,
Jason A. Donenfeld
In-Reply-To: <20251215200939.GA10539@google.com>
On Mon, Dec 15, 2025 at 08:09:39PM +0000, Eric Biggers wrote:
> On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> > use should be pooled rather than directly used. This both reduces
> > latency and improves its predictability.
> >
> > 2. Linux is better off overall if every subsystem uses the same source for
> > the random bistream as the de-facto choice, unless *force majeure*
> > reasons point to some other direction.
> >
> > In the case, of TPM there is no reason for trusted keys to invoke TPM
> > directly.
> >
> > Thus, unset '.get_random', which causes fallback to kernel_get_random().
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 636acb66a4f6..33b7739741c3 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> > return ret;
> > }
> >
> > -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> > -{
> > - return tpm_get_random(chip, key, key_len);
> > -}
> > -
> > static int __init init_digests(void)
> > {
> > int i;
> > @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> > .init = trusted_tpm_init,
> > .seal = trusted_tpm_seal,
> > .unseal = trusted_tpm_unseal,
> > - .get_random = trusted_tpm_get_random,
> > .exit = trusted_tpm_exit,
> > };
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
>
> Agreed that kernel code should prefer the standard Linux RNG whenever
> possible. Note that the standard Linux RNG already incorporates entropy
> from hardware RNGs, when available.
I get also the argument of using TPM RNG here just for the sake of
matching the creation with fully internally generated TPM objects.
I'm a bit little in-between what to do with this patch.
I suggested a comment to James. Other alternative would be do this
change and update this patch with a comment:
/*
* tpm_get_random() was used previously here as the RNG in order to match
* rng with the objects generated internally inside the TPM. However, since
* e.g., FIPS certification requires kernel crypto and rng to be FIPS
* certified, formally kernel_get_random() is equally legit source for
* the random numbers.
*/
It's longish but I think this fully covers the whole issue.
And if there is ever need to return to this, it's a good remainder of
the design choices.
>
> - Eric
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 21:09 UTC (permalink / raw)
To: Eric Biggers
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,
Jason A. Donenfeld
In-Reply-To: <aUBxKqL5hFibwI3r@kernel.org>
On Mon, Dec 15, 2025 at 10:35:57PM +0200, Jarkko Sakkinen wrote:
> On Mon, Dec 15, 2025 at 08:09:39PM +0000, Eric Biggers wrote:
> > On Sun, Dec 14, 2025 at 11:32:36PM +0200, Jarkko Sakkinen wrote:
> > > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> > > use should be pooled rather than directly used. This both reduces
> > > latency and improves its predictability.
> > >
> > > 2. Linux is better off overall if every subsystem uses the same source for
> > > the random bistream as the de-facto choice, unless *force majeure*
> > > reasons point to some other direction.
> > >
> > > In the case, of TPM there is no reason for trusted keys to invoke TPM
> > > directly.
> > >
> > > Thus, unset '.get_random', which causes fallback to kernel_get_random().
> > >
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > > security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > > index 636acb66a4f6..33b7739741c3 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > > @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> > > return ret;
> > > }
> > >
> > > -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> > > -{
> > > - return tpm_get_random(chip, key, key_len);
> > > -}
> > > -
> > > static int __init init_digests(void)
> > > {
> > > int i;
> > > @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> > > .init = trusted_tpm_init,
> > > .seal = trusted_tpm_seal,
> > > .unseal = trusted_tpm_unseal,
> > > - .get_random = trusted_tpm_get_random,
> > > .exit = trusted_tpm_exit,
> > > };
> >
> > Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> >
> > Agreed that kernel code should prefer the standard Linux RNG whenever
> > possible. Note that the standard Linux RNG already incorporates entropy
> > from hardware RNGs, when available.
>
> I get also the argument of using TPM RNG here just for the sake of
> matching the creation with fully internally generated TPM objects.
>
> I'm a bit little in-between what to do with this patch.
>
> I suggested a comment to James. Other alternative would be do this
> change and update this patch with a comment:
>
> /*
> * tpm_get_random() was used previously here as the RNG in order to match
> * rng with the objects generated internally inside the TPM. However, since
> * e.g., FIPS certification requires kernel crypto and rng to be FIPS
> * certified, formally kernel_get_random() is equally legit source for
> * the random numbers.
> */
>
> It's longish but I think this fully covers the whole issue.
>
> And if there is ever need to return to this, it's a good remainder of
> the design choices.
I'll supplement the patch with that explanatory comment. I think the
previous discussions pointed out by James were useful reflection point
and that comment summarizes that discussion.
I'll add your reviewd-by to the next version, as no additional code
changes will be implemented.
I think that this discussion also implies that the callback itself is
somewhat questionable, perhaps even harmful. Same arguments apply also
to e.g., TEE trusted keys. IMHO, would be overall best for Linux to a
have a one single call path for generating random numbers.
Using combined entropy also decreases corrateral damage caused by e.g.,
a buggy TPM firmware, which does happen sometimes in the wild.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Danilo Krummrich @ 2025-12-15 21:40 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, linux-kernel, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Peter Zijlstra, Boqun Feng,
Elle Rhumsaa, Carlos Llamas, Yury Norov, Andreas Hindborg,
linux-block, FUJITA Tomonori, Miguel Ojeda, Michael Turquette,
Stephen Boyd, linux-clk, Benno Lossin, Thomas Gleixner,
Rafael J. Wysocki, Viresh Kumar, linux-pm, Paul Moore,
Serge Hallyn, linux-security-module, Daniel Almeida,
Abdiel Janulgue, Robin Murphy, Lyude Paul, Alexander Viro,
Christian Brauner, Jan Kara, linux-fsdevel, Josh Poimboeuf,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Brendan Higgins,
David Gow, Rae Moar, linux-kselftest, Andrew Morton,
Liam R. Howlett, Andrew Ballance, maple-tree, linux-mm,
Lorenzo Stoakes, Uladzislau Rezki, Vitaly Wool, Rob Herring,
Saravana Kannan, devicetree, Bjorn Helgaas,
Krzysztof Wilczyński, linux-pci, Remo Senekowitsch,
Paul E. McKenney, rcu, Will Deacon, Fiona Behrens, Gary Guo,
Liam Girdwood, Mark Brown, Alexandre Courbot, Vlastimil Babka,
Christoph Lameter, David Rientjes, Ingo Molnar, Waiman Long,
Mitchell Levy, Frederic Weisbecker, Anna-Maria Behnsen,
John Stultz, linux-usb, Tejun Heo, Lai Jiangshan, Matthew Wilcox,
Tamir Duberstein
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
On Tue Dec 2, 2025 at 8:37 PM CET, Alice Ryhl wrote:
Applied to driver-core-testing, thanks!
> Alice Ryhl (46):
> rust: auxiliary: add __rust_helper to helpers
> rust: device: add __rust_helper to helpers
> rust: dma: add __rust_helper to helpers
> rust: io: add __rust_helper to helpers
> rust: irq: add __rust_helper to helpers
> rust: pci: add __rust_helper to helpers
[ Consider latest helper additions. - Danilo ]
> rust: platform: add __rust_helper to helpers
> rust: property: add __rust_helper to helpers
> rust: scatterlist: add __rust_helper to helpers
^ permalink raw reply
* Re: [PATCH v2 14/17] KEYS: trusted: Migrate to use tee specific driver registration function
From: Jarkko Sakkinen @ 2025-12-15 22:01 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Sumit Garg, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <687c004c32718ba7044ffa9165f33842267a745d.1765791463.git.u.kleine-koenig@baylibre.com>
On Mon, Dec 15, 2025 at 03:16:44PM +0100, Uwe Kleine-König wrote:
> The tee subsystem recently got a set of dedicated functions to register
> (and unregister) a tee driver. Make use of them. These care for setting the
> driver's bus (so the explicit assignment can be dropped) and the driver
> owner (which is an improvement this driver benefits from).
>
> Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index aa3d477de6db..3cea9a377955 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -264,7 +264,6 @@ static struct tee_client_driver trusted_key_driver = {
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .bus = &tee_bus_type,
> .probe = trusted_key_probe,
> .remove = trusted_key_remove,
> },
> @@ -272,12 +271,12 @@ static struct tee_client_driver trusted_key_driver = {
>
> static int trusted_tee_init(void)
> {
> - return driver_register(&trusted_key_driver.driver);
> + return tee_client_driver_register(&trusted_key_driver);
> }
>
> static void trusted_tee_exit(void)
> {
> - driver_unregister(&trusted_key_driver.driver);
> + tee_client_driver_unregister(&trusted_key_driver);
> }
>
> struct trusted_key_ops trusted_key_tee_ops = {
> --
> 2.47.3
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 15/17] KEYS: trusted: Make use of tee bus methods
From: Jarkko Sakkinen @ 2025-12-15 22:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Sumit Garg, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <ad8aaa343c1e8523659259290f63aea8be906977.1765791463.git.u.kleine-koenig@baylibre.com>
On Mon, Dec 15, 2025 at 03:16:45PM +0100, Uwe Kleine-König wrote:
> The tee bus got dedicated callbacks for probe and remove.
> Make use of these. This fixes a runtime warning about the driver needing
> to be converted to the bus methods.
>
> Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> security/keys/trusted-keys/trusted_tee.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index 3cea9a377955..6e465c8bef5e 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -202,9 +202,9 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> return 0;
> }
>
> -static int trusted_key_probe(struct device *dev)
> +static int trusted_key_probe(struct tee_client_device *rng_device)
> {
> - struct tee_client_device *rng_device = to_tee_client_device(dev);
> + struct device *dev = &rng_device->dev;
> int ret;
> struct tee_ioctl_open_session_arg sess_arg;
I'm sorry but cannot help saying but these not being in reverse tree
order hurts my eyes ;-)
I.e., I'd personally move declaration of sess_arg right after rng_device
despite being additional change to the scope of the patch.
That said, Sumit has the ultimate veto right here, and this not any kind
of fault in this patch so I will obviously ack the patch;
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> @@ -244,13 +244,11 @@ static int trusted_key_probe(struct device *dev)
> return ret;
> }
>
> -static int trusted_key_remove(struct device *dev)
> +static void trusted_key_remove(struct tee_client_device *dev)
> {
> unregister_key_type(&key_type_trusted);
> tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
> tee_client_close_context(pvt_data.ctx);
> -
> - return 0;
> }
>
> static const struct tee_client_device_id trusted_key_id_table[] = {
> @@ -261,11 +259,11 @@ static const struct tee_client_device_id trusted_key_id_table[] = {
> MODULE_DEVICE_TABLE(tee, trusted_key_id_table);
>
> static struct tee_client_driver trusted_key_driver = {
> + .probe = trusted_key_probe,
> + .remove = trusted_key_remove,
> .id_table = trusted_key_id_table,
> .driver = {
> .name = DRIVER_NAME,
> - .probe = trusted_key_probe,
> - .remove = trusted_key_remove,
> },
> };
>
> --
> 2.47.3
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v5 1/6] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-12-15 22:21 UTC (permalink / raw)
To: m; +Cc: gnoack, jack, linux-security-module, mic, utilityemal77, xandfury
In-Reply-To: <ef02e290-84b0-4de9-85aa-bf94d38c0c44@maowtm.org>
On 12/14/25 17:53, Tingmao Wang wrote:
> On 12/14/25 17:05, Justin Suess wrote:
>> [...]
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index d4f47d20361a..6ab3e7bd1c81 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -127,10 +127,39 @@ struct landlock_ruleset_attr {
>> * allowed_access in the passed in rule_attr. When this flag is
>> * present, the caller is also allowed to pass in an empty
>> * allowed_access.
>> + * %LANDLOCK_ADD_RULE_NO_INHERIT
>> + * When set on a rule being added to a ruleset, this flag disables the
>> + * inheritance of access rights and flags from parent objects.
>> + *
>> + * This flag currently applies only to filesystem rules. Adding it to
>> + * non-filesystem rules will return -EINVAL, unless future extensions
>> + * of Landlock define other hierarchical object types.
>> + *
>> + * By default, Landlock filesystem rules inherit allowed accesses from
>> + * ancestor directories: if a parent directory grants certain rights,
>> + * those rights also apply to its children. A rule marked with
>> + * LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
>> + * covered by the rule. Descendants of that directory continue to inherit
>> + * normally unless they also have rules using this flag.
>> + *
>> + * If a regular file is marked with this flag, it will not inherit any
>> + * access rights from its parent directories; only the accesses explicitly
>> + * allowed by the rule will apply to that file.
>> + *
>> + * This flag also enforces parent-directory restrictions: rename, rmdir,
>> + * link, and other operations that would change the directory's immediate
>> + * parent subtree are denied up to the VFS root. This prevents
>> + * sandboxed processes from manipulating the filesystem hierarchy to evade
>> + * restrictions (e.g., via sandbox-restart attacks).
>> + *
>> + * In addition, this flag blocks the inheritance of rule-layer flags
> tbh I feel that it's less confusing to just say "rule flags" (instead of
> "rule-layer flags").
Agreed. I'll change it here and in any other locations it pops up, I'll have to see.
>> + * (such as the quiet flag) from parent directories to the object covered
>> + * by this rule.
>> */
>>
>> /* clang-format off */
>> #define LANDLOCK_ADD_RULE_QUIET (1U << 0)
>> +#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
>> /* clang-format on */
>>
>> /**
>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>> index 0b589263ea42..8d8623ea857f 100644
>> --- a/security/landlock/fs.c
>> +++ b/security/landlock/fs.c
>> @@ -317,6 +317,37 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
>> LANDLOCK_ACCESS_FS_IOCTL_DEV)
>> /* clang-format on */
>>
>> +enum landlock_walk_result {
>> + LANDLOCK_WALK_CONTINUE,
>> + LANDLOCK_WALK_STOP_REAL_ROOT,
>> + LANDLOCK_WALK_MOUNT_ROOT,
>> +};
>> +
>> +static enum landlock_walk_result landlock_walk_path_up(struct path *const path)
>> +{
>> + while (path->dentry == path->mnt->mnt_root) {
>> + if (!follow_up(path))
>> + return LANDLOCK_WALK_STOP_REAL_ROOT;
>> + }
>> +
>> + if (unlikely(IS_ROOT(path->dentry))) {
>> + if (likely(path->mnt->mnt_flags & MNT_INTERNAL))
>> + return LANDLOCK_WALK_MOUNT_ROOT;
> imo, LANDLOCK_WALK_MOUNT_ROOT is a somewhat confusing name for this,
> especially in the context that if we see this in
> is_access_to_paths_allowed() we allow access unconditionally.
>
> Would LANDLOCK_WALK_INTERNAL be a better name here?
>
Yeah that seems better. LANDLOCK_WALK_INTERNAL seems like a better name.
Plus some documenting comments in the landlock_walk_result are warranted.
I'll fix it in the next version.
>> + dput(path->dentry);
>> + path->dentry = dget(path->mnt->mnt_root);
>> + return LANDLOCK_WALK_CONTINUE;
>> + }
>> +
>> + struct dentry *const parent = dget_parent(path->dentry);
>> +
>> + dput(path->dentry);
>> + path->dentry = parent;
>> + return LANDLOCK_WALK_CONTINUE;
>> +}
>> +
>> +static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
>> + const struct dentry *const dentry);
>> +
>> /*
>> * @path: Should have been checked by get_path_from_fd().
>> */
>> @@ -344,6 +375,48 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>> return PTR_ERR(id.key.object);
>> mutex_lock(&ruleset->lock);
>> err = landlock_insert_rule(ruleset, id, access_rights, flags);
>> + if (err || !(flags & LANDLOCK_ADD_RULE_NO_INHERIT))
>> + goto out_unlock;
>> +
>> + /* Create ancestor rules and set has_no_inherit_descendant flags */
>> + struct path walker = *path;
>> +
>> + path_get(&walker);
>> + while (landlock_walk_path_up(&walker) != LANDLOCK_WALK_STOP_REAL_ROOT) {
> Why not landlock_walk_path_up(&walker) == LANDLOCK_WALK_CONTINUE here?
> I'm not sure if it's actually possible to end up with an infinite loop by
> ignoring LANDLOCK_WALK_MOUNT_ROOT (i.e. not sure if "internal" mounts can
> have disconnected dentries), but it seems safer to write to loop in a way
> such that if that happens, we exit.
I don't *think* it's possible to end up in an infinite loop this way, but you never know.
I'll definitely take your suggestion because it's semantically clearer at the very least.
>
>> + struct landlock_rule *ancestor_rule;
>> +
>> + if (WARN_ON_ONCE(!walker.dentry || d_is_negative(walker.dentry))) {
>> + err = -EIO;
>> + break;
>> + }
>> +
>> + ancestor_rule = (struct landlock_rule *)find_rule(ruleset, walker.dentry);
>> + if (!ancestor_rule) {
>> + struct landlock_id ancestor_id = {
>> + .type = LANDLOCK_KEY_INODE,
>> + .key.object = get_inode_object(d_backing_inode(walker.dentry)),
>> + };
>> +
>> + if (IS_ERR(ancestor_id.key.object)) {
>> + err = PTR_ERR(ancestor_id.key.object);
>> + break;
>> + }
>> + err = landlock_insert_rule(ruleset, ancestor_id, 0, 0);
>> + landlock_put_object(ancestor_id.key.object);
>> + if (err)
>> + break;
>> +
>> + ancestor_rule = (struct landlock_rule *)
>> + find_rule(ruleset, walker.dentry);
>> + }
>> + if (WARN_ON_ONCE(!ancestor_rule || ancestor_rule->num_layers != 1)) {
>> + err = -EIO;
>> + break;
>> + }
>> + ancestor_rule->layers[0].flags.has_no_inherit_descendant = true;
>> + }
>> + path_put(&walker);
>> +out_unlock:
>> mutex_unlock(&ruleset->lock);
>> /*
>> * No need to check for an error because landlock_insert_rule()
>> @@ -772,8 +845,10 @@ static bool is_access_to_paths_allowed(
>> _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
>> layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
>> (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
>> - struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
>> - struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
>> + struct collected_rule_flags *rule_flags_parent1 =
>> + &log_request_parent1->rule_flags;
>> + struct collected_rule_flags *rule_flags_parent2 =
>> + log_request_parent2 ? &log_request_parent2->rule_flags : NULL;
> Good point, I think the original was still safe because it would not be
> used by landlock_unmask_layers anyway, but this is better. I will take
> this in the next version, thanks!
No problem. I actually meant to put this as a review under your patch as
a comment but I pulled it in accidentally.
Rebasing off your patch has been a breeze btw 🙂
>
>> if (!access_request_parent1 && !access_request_parent2)
>> return true;
>> @@ -784,7 +859,7 @@ static bool is_access_to_paths_allowed(
>> if (is_nouser_or_private(path->dentry))
>> return true;
>>
>> - if (WARN_ON_ONCE(!layer_masks_parent1))
>> + if (WARN_ON_ONCE(!layer_masks_parent1 || !log_request_parent1))
>> return false;
>>
>> allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
>> @@ -851,6 +926,7 @@ static bool is_access_to_paths_allowed(
>> */
>> while (true) {
>> const struct landlock_rule *rule;
>> + enum landlock_walk_result walk_res;
>>
>> /*
>> * If at least all accesses allowed on the destination are
>> @@ -910,46 +986,14 @@ static bool is_access_to_paths_allowed(
>> if (allowed_parent1 && allowed_parent2)
>> break;
>>
>> -jump_up:
>> - if (walker_path.dentry == walker_path.mnt->mnt_root) {
>> - if (follow_up(&walker_path)) {
>> - /* Ignores hidden mount points. */
>> - goto jump_up;
>> - } else {
>> - /*
>> - * Stops at the real root. Denies access
>> - * because not all layers have granted access.
>> - */
>> - break;
>> - }
>> - }
>> -
>> - if (unlikely(IS_ROOT(walker_path.dentry))) {
>> - if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
>> - /*
>> - * Stops and allows access when reaching disconnected root
>> - * directories that are part of internal filesystems (e.g. nsfs,
>> - * which is reachable through /proc/<pid>/ns/<namespace>).
>> - */
>> - allowed_parent1 = true;
>> - allowed_parent2 = true;
>> - break;
>> - }
>> -
>> - /*
>> - * We reached a disconnected root directory from a bind mount.
>> - * Let's continue the walk with the mount point we missed.
>> - */
> I think we might want to preserve these comments.
Agreed. Thank you, I missed those. I'll preserve them in the next version.
>
>> - dput(walker_path.dentry);
>> - walker_path.dentry = walker_path.mnt->mnt_root;
>> - dget(walker_path.dentry);
>> - } else {
>> - struct dentry *const parent_dentry =
>> - dget_parent(walker_path.dentry);
>> -
>> - dput(walker_path.dentry);
>> - walker_path.dentry = parent_dentry;
>> + walk_res = landlock_walk_path_up(&walker_path);
>> + if (walk_res == LANDLOCK_WALK_MOUNT_ROOT) {
>> + allowed_parent1 = true;
>> + allowed_parent2 = true;
>> + break;
>> }
>> + if (walk_res != LANDLOCK_WALK_CONTINUE)
>> + break;
>> }
>> path_put(&walker_path);
>>
>> @@ -963,7 +1007,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;
>> @@ -1037,8 +1081,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>> * collect_domain_accesses - Walk through a file path and collect accesses
>> *
>> * @domain: Domain to check against.
>> - * @mnt_root: Last directory to check.
>> - * @dir: Directory to start the walk from.
>> + * @mnt_root: Last path element to check.
>> + * @dir: Directory path to start the walk from.
>> * @layer_masks_dom: Where to store the collected accesses.
>> *
>> * This helper is useful to begin a path walk from the @dir directory to a
>> @@ -1060,29 +1104,31 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>> */
>> static bool collect_domain_accesses(
>> const struct landlock_ruleset *const domain,
>> - const struct dentry *const mnt_root, struct dentry *dir,
>> + const struct path *const mnt_root, const struct path *const dir,
>> layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
>> struct collected_rule_flags *const rule_flags)
>> {
> This function only walks up to the mountpoint of dir. If dir is changed
> from a *dentry to a *path, wouldn't mnt_root be redundant? Since
> mnt_root->dentry is always going to be dir->mnt->mnt_root. This also
> means that they can't accidentally not be the same.
Good catch, yeah they should be redundant.
I'll remove the mnt_root parameter in the next version.
>
>> unsigned long access_dom;
>> bool ret = false;
>> + struct path walker;
>>
>> if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
>> return true;
>> - if (is_nouser_or_private(dir))
>> + if (is_nouser_or_private(dir->dentry))
>> return true;
>>
>> access_dom = landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
>> layer_masks_dom,
>> LANDLOCK_KEY_INODE);
>>
>> - dget(dir);
>> + walker = *dir;
>> + path_get(&walker);
>> while (true) {
>> - struct dentry *parent_dentry;
>> + enum landlock_walk_result walk_res;
>>
>> /* Gets all layers allowing all domain accesses. */
>> if (landlock_unmask_layers(
>> - find_rule(domain, dir), access_dom, layer_masks_dom,
>> + find_rule(domain, walker.dentry), access_dom, layer_masks_dom,
>> ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
>> /*
>> * Stops when all handled accesses are allowed by at
>> @@ -1091,22 +1137,69 @@ static bool collect_domain_accesses(
>> ret = true;
>> break;
>> }
>> -
>> - /*
>> - * Stops at the mount point or the filesystem root for a disconnected
>> - * directory.
>> - */
>> - if (dir == mnt_root || unlikely(IS_ROOT(dir)))
>> + if (walker.dentry == mnt_root->dentry && walker.mnt == mnt_root->mnt)
>> + break;
>> + walk_res = landlock_walk_path_up(&walker);
>> + if (walk_res != LANDLOCK_WALK_CONTINUE)
>> break;
>> -
>> - parent_dentry = dget_parent(dir);
>> - dput(dir);
>> - dir = parent_dentry;
>> }
>> - dput(dir);
>> + path_put(&walker);
>> return ret;
>> }
>>
>> +/**
>> + * deny_no_inherit_topology_change - deny topology changes on sealed paths
>> + * @subject: Subject performing the operation (contains the domain).
>> + * @path: Path whose dentry is the target of the topology modification.
>> + *
>> + * Checks whether any domain layers are sealed against topology changes at
>> + * @path. If so, emit an audit record and return -EACCES. Otherwise return 0.
>> + */
>> +static int deny_no_inherit_topology_change(const struct landlock_cred_security
>> + *subject,
>> + const struct path *const path)
> Since you're not using path->mnt here (except for a NULL check), would it
> be easier to just pass the dentry instead? In that case you wouldn't have
> to do an inline initializer in current_check_refer_path / hook_path_*
> below as well.
Yeah, this was leftover before I did some refactoring and removed
the mark_no_inherit_ancestors. Good catch.
I'll address this in the next version.
>
>> +{
>> + layer_mask_t sealed_layers = 0;
>> + layer_mask_t override_layers = 0;
>> + const struct landlock_rule *rule;
>> + u32 layer_index;
>> + unsigned long audit_layer_index;
>> +
>> + if (WARN_ON_ONCE(!subject || !path || !path->dentry || !path->mnt ||
>> + d_is_negative(path->dentry)))
>> + return 0;
>> +
>> + rule = find_rule(subject->domain, path->dentry);
>> + if (!rule)
>> + return 0;
>> +
>> + for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
>> + const struct landlock_layer *layer = &rule->layers[layer_index];
>> + layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
>> +
>> + if (layer->flags.no_inherit ||
>> + layer->flags.has_no_inherit_descendant)
>> + sealed_layers |= layer_bit;
>> + else
>> + override_layers |= layer_bit;
>> + }
>> +
>> + sealed_layers &= ~override_layers;
>> + if (!sealed_layers)
>> + return 0;
>> +
>> + audit_layer_index = __ffs((unsigned long)sealed_layers);
>> + landlock_log_denial(subject, &(struct landlock_request) {
>> + .type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
>> + .audit = {
>> + .type = LSM_AUDIT_DATA_DENTRY,
>> + .u.dentry = path->dentry,
>> + },
>> + .layer_plus_one = audit_layer_index + 1,
>> + });
>> + return -EACCES;
>> +}
>> +
>> /**
>> * current_check_refer_path - Check if a rename or link action is allowed
>> *
>> @@ -1191,6 +1284,21 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>> access_request_parent2 =
>> get_mode_access(d_backing_inode(old_dentry)->i_mode);
>> if (removable) {
>> + int err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = new_dir->mnt,
>> + .dentry = old_dentry });
>> +
>> + if (err)
>> + return err;
>> + if (exchange) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = new_dir->mnt,
>> + .dentry = new_dentry });
>> + if (err)
>> + return err;
>> + }
>> access_request_parent1 |= maybe_remove(old_dentry);
>> access_request_parent2 |= maybe_remove(new_dentry);
>> }
>> @@ -1232,12 +1340,15 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>> old_dentry->d_parent;
>>
>> /* new_dir->dentry is equal to new_dentry->d_parent */
>> - allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
>> - old_parent,
>> + allow_parent1 = collect_domain_accesses(subject->domain,
>> + &mnt_dir,
>> + &(struct path){ .mnt = new_dir->mnt,
>> + .dentry = old_parent },
>> &layer_masks_parent1,
>> &request1.rule_flags);
>> - allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
>> - new_dir->dentry,
>> + allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
>> + &(struct path){ .mnt = new_dir->mnt,
>> + .dentry = new_dir->dentry },
>> &layer_masks_parent2,
>> &request2.rule_flags);
>>
>> @@ -1583,12 +1694,37 @@ static int hook_path_symlink(const struct path *const dir,
>> static int hook_path_unlink(const struct path *const dir,
>> struct dentry *const dentry)
>> {
>> + const struct landlock_cred_security *const subject =
>> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
>> + int err;
>> +
>> + if (subject) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = dir->mnt,
>> + .dentry = dentry });
>> + if (err)
>> + return err;
>> + }
>> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
>> }
>>
>> static int hook_path_rmdir(const struct path *const dir,
>> struct dentry *const dentry)
>> {
>> + const struct landlock_cred_security *const subject =
>> + landlock_get_applicable_subject(current_cred(), any_fs, NULL);
>> + int err;
>> +
>> + if (subject) {
>> + err = deny_no_inherit_topology_change(subject,
>> + &(struct path)
>> + { .mnt = dir->mnt,
>> + .dentry = dentry });
>> + if (err)
>> + return err;
>> + }
>> +
>> return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
>> }
>>
>> [...]
Overall I'm feeling pretty good about this series, but if either you or Mickaël have any more feedback I'd like to hear it.
I'll wait until your next quiet flag version comes and do a rebase before sending the revisions.
Sorry for the double tap Tingmao, I forgot to cc the mailing list :(
Regards,
Justin
^ permalink raw reply
* [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:14 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used so that generated trusted keys
have the matching entropy with the TPM internally generated objects.
This argument does some weight into it but as far cryptography goes, FIPS
certification sets the exact bar, not which exact FIPS certified RNG will
be used. Thus, the rational choice is obviously to pick the lowest latency
path.
Finally, there also some actual defence in depth benefits on using kernel
RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
implementation, which do happen in the wild occasionally.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Added Eric's reviewed-by tag.
- Addressed concerns from James by writing more details to the commit
message and documenting random number generation to the source
code.
---
security/keys/trusted-keys/trusted_tpm1.c | 6 ------
security/keys/trusted-keys/trusted_tpm2.c | 9 +++++++++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..33b7739741c3 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a7ea4a1c3bed..d16be47f1305 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -2,6 +2,15 @@
/*
* Copyright (C) 2004 IBM Corporation
* Copyright (C) 2014 Intel Corporation
+
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
*/
#include <linux/asn1_encoder.h>
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:17 UTC (permalink / raw)
To: linux-integrity
Cc: 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: <20251215231438.565522-1-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 01:14:38AM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used so that generated trusted keys
> have the matching entropy with the TPM internally generated objects.
>
> This argument does some weight into it but as far cryptography goes, FIPS
> certification sets the exact bar, not which exact FIPS certified RNG will
> be used. Thus, the rational choice is obviously to pick the lowest latency
> path.
>
> Finally, there also some actual defence in depth benefits on using kernel
> RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
> implementation, which do happen in the wild occasionally.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> - Added Eric's reviewed-by tag.
> - Addressed concerns from James by writing more details to the commit
> message and documenting random number generation to the source
> code.
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 6 ------
> security/keys/trusted-keys/trusted_tpm2.c | 9 +++++++++
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..33b7739741c3 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -936,11 +936,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +987,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index a7ea4a1c3bed..d16be47f1305 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -2,6 +2,15 @@
> /*
> * Copyright (C) 2004 IBM Corporation
> * Copyright (C) 2014 Intel Corporation
> +
> +/**
> + * DOC: Random Number Generation
> + *
> + * tpm_get_random() was previously used here as the RNG in order to have equal
> + * entropy with the objects fully inside the TPM. However, as far as goes,
> + * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
> + * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
> + * associated with the RNG.
> */
Sorry, this should have gone to trusted_tpm1.c :-)
>
> #include <linux/asn1_encoder.h>
> --
> 2.39.5
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:25 UTC (permalink / raw)
To: linux-integrity
Cc: 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: <20251215231438.565522-1-jarkko@kernel.org>
On Tue, Dec 16, 2025 at 01:14:38AM +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used so that generated trusted keys
> have the matching entropy with the TPM internally generated objects.
>
> This argument does some weight into it but as far cryptography goes, FIPS
> certification sets the exact bar, not which exact FIPS certified RNG will
> be used. Thus, the rational choice is obviously to pick the lowest latency
> path.
>
> Finally, there also some actual defence in depth benefits on using kernel
> RNG. E.g., it helps to mitigate TPM firmware bugs concerning RNG
> implementation, which do happen in the wild occasionally.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
I noticed also some typos in the commit message.
I think I will also supplement this with a patch that unexports
tpm_get_random(), as the patch zeros the external call sites.
Full encapsulation to the driver is exactly should aim for in order to
make hwrng easier target for further optimizations.
BR, Jarkko
^ permalink raw reply
* [PATCH v3] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-15 23:49 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used for the generated trusted keys,
so that they have matching entropy with the TPM internally generated
objects.
This argument does have some weight into it but as far cryptography goes,
FIPS certification sets the exact bar, not which exact FIPS certified RNG
will be used. Thus, the rational choice is obviously to pick the lowest
latency path, which is kernel RNG.
Finally, there is an actual defence in depth benefit when using kernel RNG
as it helps to mitigate TPM firmware bugs concerning RNG implementation,
given the obfuscation by the other entropy sources.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
- Fixed typos in the commit message.
- Moved the documentation comment to the correct location.
v2:
- Added Eric's reviewed-by tag.
- Addressed concerns from James by writing more details to the commit
message and documenting random number generation to the source
code.
---
security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..7ce7e31bcdfb 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,16 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
+ */
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
@@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] lsm: fix kernel-doc struct member names
From: Paul Moore @ 2025-12-16 2:51 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Randy Dunlap, James Morris, Serge E. Hallyn,
linux-security-module
In-Reply-To: <20251214201539.2188497-1-rdunlap@infradead.org>
On Dec 14, 2025 Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Use the correct struct member names to avoid kernel-doc warnings:
>
> Warning: include/linux/lsm_hooks.h:83 struct member 'name' not described
> in 'lsm_id'
> Warning: include/linux/lsm_hooks.h:183 struct member 'initcall_device' not
> described in 'lsm_info'
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
> ---
> include/linux/lsm_hooks.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks Randy, merged into lsm/dev.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Use get_random-fallback for TPM
From: James Bottomley @ 2025-12-16 6:48 UTC (permalink / raw)
To: Jarkko Sakkinen, Eric Biggers
Cc: linux-integrity, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list, Jason A. Donenfeld
In-Reply-To: <aUB5IsJeWhFvX-cA@kernel.org>
On Mon, 2025-12-15 at 23:09 +0200, Jarkko Sakkinen wrote:
> Using combined entropy also decreases corrateral damage caused by
> e.g., a buggy TPM firmware, which does happen sometimes in the wild.
Just to allay concerns on this point: the random number generator of a
physical TPM is always based on a hardware entropy generating element.
NIST specifies (and FIPS testing requires) that this hardware element
conform to SP 800-90B which is about 84 pages of how a RNG should be
conditioned and tested (and certified), so there should be very little
chance of issues here.
While TPMs have had problems caused by buggy firmware in the past, it's
always affected areas that the FIPS testing doesn't cover in such depth
(like the Infineon weak prime problem). People should feel confident
in the TPM random number generator (particularly because it's the
primary boot time entropy source for the in-kernel RNG on most
laptops).
Regards,
James
^ permalink raw reply
* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Sumit Garg @ 2025-12-16 7:38 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Sumit Garg, Jens Wiklander, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Ilias Apalodimas, Jan Kiszka,
Sudeep Holla, Christophe JAILLET, Michael Chan, Pavan Chebbi,
Rafał Miłecki, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, Peter Huewe, op-tee, linux-kernel, linux-crypto,
linux-rtc, linux-efi, linux-stm32, linux-arm-kernel,
Cristian Marussi, arm-scmi, netdev, linux-mips, linux-integrity,
keyrings, linux-security-module, Jason Gunthorpe
In-Reply-To: <dhunzydod4d7vj73llpuqemxb5er2ja4emxusr66irwf77jhhb@es4yd2axzl25>
Hi Uwe,
On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello Sumit,
>
> On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König 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.)
> > >
> > > The patches #4 - #17 depend on the first two, so if they should be
> > > applied to their respective subsystem trees these must contain the first
> > > two patches first.
> >
> > Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> > drivers.
>
> Thanks for your feedback. I will prepare a v2 and address your comments
> (whitespace issues and wrong callback in the shutdown method).
>
> > > Note that after patch #2 is applied, unconverted drivers provoke a
> > > warning in driver_register(), so it would be good for the user
> > > experience if the whole series goes in during a single merge window.
> >
> > +1
> >
> > I suggest the whole series goes via the Jens tree since there shouldn't
> > be any chances for conflict here.
> >
> > > So
> > > I guess an immutable branch containing the frist three patches that can
> > > be merged into the other subsystem trees would be sensible.
> > >
> > > After all patches are applied, tee_bus_type can be made private to
> > > drivers/tee as it's not used in other places any more.
> > >
> >
> > Feel free to make the tee_bus_type private as the last patch in the series
> > such that any followup driver follows this clean approach.
>
> There is a bit more to do for that than I'm willing to invest. With my
> patch series applied `tee_bus_type` is still used in
> drivers/tee/optee/device.c and drivers/tee/tee_core.c.
Oh I see, I guess we need to come with some helpers around device
register/unregister from TEE subsystem as well. Let's plan that for a
followup patch-set, I don't want this patch-set to be bloated more.
> Maybe it's
> sensible to merge these two files into a single one.
It's not possible as the design for TEE bus is to have TEE
implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
register devices on the bus.
>
> The things I wonder about additionally are:
>
> - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> drivers but not devices.
Yeah since the devices are rather added by the TEE implementation driver.
>
> - optee_register_device() calls device_create_file() on
> &optee_device->dev after device_register(&optee_device->dev).
> (Attention half-knowledge!) I think device_create_file() should not
> be called on an already registered device (or you have to send a
> uevent afterwards). This should probably use type attribute groups.
> (Or the need_supplicant attribute should be dropped as it isn't very
> useful. This would maybe be considered an ABI change however.)
The reasoning for this attribute should be explained by commit:
7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
In summary it's due to a weird dependency for devices we have with the
user-space daemon: tee-supplicant.
>
> - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> optee devices in its error path (optee_unregister_devices())?
This is mostly to take care of if any device got registered before the
failure occured. Let me know if you have a better way to address that.
-Sumit
^ permalink raw reply
* [PATCH v7 00/12] Streamline TPM2 HMAC sessions
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
This patch set contains accumulated patches, which gradually improve
TPM2 HMAC session management and TPM driver memory management.
RNG test
========
I run this test both TPM1 and TPM2 chips using QEMU and swtpm:
#!/bin/sh
ctrl_c() {
kill -15 $TEST_PID
echo 0 > tracing_on
echo nop > current_tracer
echo > kprobe_events
echo > set_ftrace_filter
echo BYE
exit
}
trap ctrl_c EXIT INT
mount -t tracefs none /sys/kernel/tracing
set -e
cd /sys/kernel/tracing
echo function > current_tracer
echo p:tpm_get_random tpm_get_random > kprobe_events
echo tpm_get_random > set_ftrace_filter
echo 1 > tracing_on
TEST_PID=$(cat /dev/hwrng > /dev/null &)
echo > trace
cat trace_pipe &
sleep 10
Change Log
==========
v7:
- Updated cover letter to match better the current state of the patch
set.
v6:
- OK, so I decided to send one more update with managed allocations
moved to the tail so that it does not block reviewing more trivial
patches.
- Trimmed some of the patches and improved commit messages.
v5:
- I decided to add the managed allocation patch to this and take it from
the master branch for the time being, as it needs more eyes despite
having already one reviewed-by tag (especially tested-by tags).
Jarkko Sakkinen (12):
KEYS: trusted: Use get_random-fallback for TPM
KEYS: trusted: Use get_random_bytes_wait() instead of tpm_get_random()
tpm2-sessions: Define TPM2_NAME_MAX_SIZE
KEYS: trusted: Open code tpm2_buf_append()
KEYS: trusted: Remove dead branch from tpm2_unseal_cmd
KEYS: trusted: Re-orchestrate tpm2_read_public() calls
tpm2-sessions: Remove AUTH_MAX_NAMES
tpm: Orchestrate TPM commands in tpm_get_random()
tpm: Change tpm_get_random() opportunistic
tpm-buf: Merge TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW
tpm-buf: Implement managed allocations
tpm-buf: Remove tpm_buf_append_handle
drivers/char/tpm/tpm-buf.c | 154 ++++-----
drivers/char/tpm/tpm-interface.c | 145 ++++++++-
drivers/char/tpm/tpm-sysfs.c | 23 +-
drivers/char/tpm/tpm.h | 3 -
drivers/char/tpm/tpm1-cmd.c | 198 ++++--------
drivers/char/tpm/tpm2-cmd.c | 371 +++++++---------------
drivers/char/tpm/tpm2-sessions.c | 272 ++++++----------
drivers/char/tpm/tpm2-space.c | 44 ++-
drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-
include/linux/tpm.h | 77 +++--
security/keys/trusted-keys/trusted_tpm1.c | 70 ++--
security/keys/trusted-keys/trusted_tpm2.c | 329 ++++++++++---------
12 files changed, 776 insertions(+), 940 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH v7 01/12] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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: <20251216074454.2192499-1-jarkko@kernel.org>
1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
use should be pooled rather than directly used. This both reduces
latency and improves its predictability.
2. Linux is better off overall if every subsystem uses the same source for
generating the random numbers required.
Thus, unset '.get_random', which causes fallback to kernel_get_random().
One might argue that TPM RNG should be used for the generated trusted keys,
so that they have matching entropy with the TPM internally generated
objects.
This argument does have some weight into it but as far cryptography goes,
FIPS certification sets the exact bar, not which exact FIPS certified RNG
will be used. Thus, the rational choice is obviously to pick the lowest
latency path, which is kernel RNG.
Finally, there is an actual defence in depth benefit when using kernel RNG
as it helps to mitigate TPM firmware bugs concerning RNG implementation,
given the obfuscation by the other entropy sources.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- A new patch. Simplifies follow up patches.
---
security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..7ce7e31bcdfb 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,16 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+/**
+ * DOC: Random Number Generation
+ *
+ * tpm_get_random() was previously used here as the RNG in order to have equal
+ * entropy with the objects fully inside the TPM. However, as far as goes,
+ * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
+ * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
+ * associated with the RNG.
+ */
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
@@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
return ret;
}
-static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
-{
- return tpm_get_random(chip, key, key_len);
-}
-
static int __init init_digests(void)
{
int i;
@@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
.init = trusted_tpm_init,
.seal = trusted_tpm_seal,
.unseal = trusted_tpm_unseal,
- .get_random = trusted_tpm_get_random,
.exit = trusted_tpm_exit,
};
--
2.39.5
^ permalink raw reply related
* [PATCH v7 02/12] KEYS: trusted: Use get_random_bytes_wait() instead of tpm_get_random()
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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: <20251216074454.2192499-1-jarkko@kernel.org>
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.
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
^ permalink raw reply related
* [PATCH v7 03/12] tpm2-sessions: Define TPM2_NAME_MAX_SIZE
From: Jarkko Sakkinen @ 2025-12-16 7:44 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, 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: <20251216074454.2192499-1-jarkko@kernel.org>
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.
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
+ * 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
^ 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