* Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-15 9:32 UTC (permalink / raw)
To: Sumit Garg
Cc: Jens Wiklander, 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, 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: <aT--ox375kg2Mzh-@sumit-X1>
[-- Attachment #1: Type: text/plain, Size: 3035 bytes --]
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. Maybe it's
sensible to merge these two files into a single one.
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.
- 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.)
- Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
optee devices in its error path (optee_unregister_devices())?
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] host/roots: Sandbox xdg-desktop-portal-spectrum-host
From: Mickaël Salaün @ 2025-12-15 11:27 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Günther Noack, Alyssa Ross, Spectrum OS Development,
linux-security-module, landlock, Ryan B. Sullivan,
Günther Noack
In-Reply-To: <2a681b48-bc6e-4257-8e0f-3b6aff25ac67@gmail.com>
On Mon, Dec 15, 2025 at 03:54:25AM -0500, Demi Marie Obenour wrote:
> On 12/15/25 03:20, Günther Noack wrote:
> > On Sun, Dec 14, 2025 at 08:50:45PM +0100, Mickaël Salaün wrote:
> >> On Sat, Dec 13, 2025 at 11:49:11PM -0500, Demi Marie Obenour wrote:
> >>> On 12/13/25 20:39, Alyssa Ross wrote:
> >>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> >>>>
> >>>>> On 12/13/25 16:42, Alyssa Ross wrote:
> >>>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> >>>>>>
> >>>>>>> On 12/13/25 14:12, Alyssa Ross wrote:
> >>>>>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
> >>>>>>>>
> >>>>>>>>> It is quite possible that these Landlock rules are unnecessarily
> >>>>>>>>> permissive, but all of the paths to which read and execute access is
> >>>>>>>>> granted are part of the root filesystem and therefore assumed to be
> >>>>>>>>> public knowledge. Removing access from any of them would only increase
> >>>>>>>>> the risk of accidental breakage in the future, and would not provide any
> >>>>>>>>> security improvements. seccomp *could* provide some improvements, but
> >>>>>>>>> the effort needed is too high for now.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> >>>>>>>>> ---
> >>>>>>>>> .../template/data/service/xdg-desktop-portal-spectrum-host/run | 8 ++++++++
> >>>>>>>>> 1 file changed, 8 insertions(+)
> >>>>>>>>
> >>>>>>>> Are you sure this is working as intended? There's no rule allowing
> >>>>>>>> access to Cloud Hypervisor's VSOCK socket, and yet it still seems to be
> >>>>>>>> able to access that. Don't you need to set a rule that *restricts*
> >>>>>>>> filesystem access and then add holes? Did you ever see this deny
> >>>>>>>> anything?
> >>>>>>>
> >>>>>>> 'man 1 setpriv' states that '--landlock-access fs' blocks all
> >>>>>>> filesystem access unless a subsequent --landlock-rule permits it.
> >>>>>>> I tried running with no --landlock-rule flags and the execve of
> >>>>>>> xdg-desktop-portal-spectrum-host failed as expected.
> >>>>>>>
> >>>>>>> The socket is passed over stdin, and I'm pretty sure Landlock
> >>>>>>> doesn't restrict using an already-open file descriptor.
> >>>>>>> xdg-desktop-portal-spectrum-host does need to find the path to the
> >>>>>>> socket, but I don't think it ever accesses that path.
> >>>>>>
> >>>>>> I've been looking into this a bit myself, and from what I can tell
> >>>>>> Landlock just doesn't restrict connecting to sockets at all, even if
> >>>>>> they're inside directories that would otherwise be inaccessible. It's
> >>>>>> able to connect to both Cloud Hypervisor's VSOCK socket and the D-Bus
> >>>>>> socket even with a maximally restrictive landlock rule. So you were
> >>>>>> right after all, sorry!
> >>>>>
> >>>>> That's not good at all! It's a trivial sandbox escape in so many cases.
> >>>>> For instance, with access to D-Bus I can just call `systemd-run`.
> >>>>>
> >>>>> I'm CCing the Landlock and LSM mailing lists because if you are
> >>>>> correct, then this is a bad security hole.
> >>>>
> >>>> I don't find it that surprising given the way landlock works. "connect"
> >>>> (to a non-abstract AF_UNIX socket) is not an operation there's a
> >>>> landlock action for, and it's not like the other actions care about
> >>>> access to parent directories and the like — I was able to execute a
> >>>> program via a symlink after only giving access to the symlink's target,
> >>>> without any access to the directory containing the symlink or the
> >>>> symlink itself, for example. Landlock, as I understand it, is intended
> >>>> to block a specified set of operations (on particular file hierarchies),
> >>>> rather than to completely prevent access to those hierarchies like
> >>>> permissions or mount namespaces could, so the lack of a way to block
> >>>> connecting to a socket is more of a missing feature than a security
> >>>> hole.
> >>>
> >>> 'man 7 unix' states:
> >>>
> >>> On Linux, connecting to a stream socket object requires write
> >>> permission on that socket; sending a datagram to a datagram socket
> >>> likewise requires write permission on that socket.
> >>>
> >>> Landlock is definitely being inconsistent with DAC here. Also, this
> >>> allows real-world sandbox breakouts. On systemd systems, the simplest
> >>> way to escape is to use systemd-run to execute arbitrary commands.
> >>
> >> The Linux kernel is complex and the link between the VFS and named UNIX
> >> sockets is "special" (see the linked GitHub issue). Landlock doesn't
> >> handle named UNIX sockets yet for the same reason it doesn't handle some
> >> other kind of kernel resources or access rights: someone needs to
> >> implement it (including tests, doc...). There is definitely interest to
> >> add this feature, it has been discussed for some time, but it's not
> >> trivial. It is a work in progress though:
> >> https://github.com/landlock-lsm/linux/issues/36
> >
> > Agreed, this would be the change that would let us restrict connect()
> > on AF_UNIX sockets.
> >
> > Additionally, *in the case that you do not actually need* Unix
> > sockets, the other patch set that would be of interest is the one for
> > restricting the creation of new socket FDs:
> > https://github.com/landlock-lsm/linux/issues/6
> >
> > In this talk in 2014, I explained my mental model around the
> > network-related restrictions:
> > https://youtu.be/K2onopkMhuM?si=LCObEX6HhGdzPnks&t=2030
> >
> > The discussed socket control feature continues to be a central missing
> > piece, as the TCP port restrictions do not make much sense as long as
> > you can still create sockets for other protocol types.
> >
> > Issue #6 that should fix this is still under active development -- an
> > updated version of the patch was posted just last month.
> >
> > To bridge the gap, the same thing can also be emulated with seccomp,
> > but as you noted above as well in this thread, this is harder.
> >
> > –Günther
>
> I'm a bit surprised that this needs to be separate from other access
> controls. To me, it seems like a bit of a misdesign in the core kernel
> (not Landlock).
>
> I would go as far as to state that enabling other filesystem
> restrictions should also restrict AF_UNIX filesystem sockets
> automatically, as that is what users and administrators will expect.
I agree, that would be nice, but it's legacy.
>
> What surprises me somewhat is that Linux does not have any sort of
> unified hook for filesystem path walks. My mental model of Landlock
> is that from a filesystem perspective, the result should be equivalent
> to creating an empty mount namespace, putting a tmpfs on its root,
> and bind-mounting the allowed paths. That this mental model does
> not match reality was quite surprising.
The approach taken by Landlock is to not break user space with kernel
updates, follow an incremental approach, and have a nice compatibility
story. This is why a ruleset is created with a set of well-defined
"handled" access rights and restrictions. Each new handled access right
or restriction must come with an explicit request from user space (e.g.
set new bits in struct landlock_ruleset_attr).
It would have been appealing to start from scratch, drop all access
rights, and then gradually implement access rights, but that would have
make Landlock unusable in most use cases. Instead, we gradually extend
the scope of Landlock with well-defined and well-tested restrictions.
This approach eases the integration of Landlock and when new kernel
features are available, it's easy to update an integration to leverage
the new features. From day one, Landlock has been useful, even if it
doesn't cover all use cases and environments. When sandboxing a program
(or properly securing an OS), most of the time, the difficult part is to
get a good security architecture with proper privilege separation. Once
this is done, it's much easier to gradually improve it.
We should probably improve the Landlock documentation, especially by
extending the warning block with some other relevant syscalls:
https://docs.kernel.org/userspace-api/landlock.html#filesystem-flags
^ permalink raw reply
* Re: [RFC][PATCH v2] ima: Add support for staging measurements for deletion and trimming
From: Roberto Sassu @ 2025-12-15 12:41 UTC (permalink / raw)
To: Paul Moore
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, jmorris, serge,
linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <CAHC9VhRUQxayj=XcdfbfHka-=N+B8cNk7Grg3QWGOTOz3BKfgw@mail.gmail.com>
On Fri, 2025-12-12 at 21:06 -0500, Paul Moore wrote:
> On Fri, Dec 12, 2025 at 12:19 PM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Introduce the ability of staging the entire (or a portion of the) IMA
> > measurement list for deletion. Staging means moving the current content of
> > the measurement list to a separate location, and allowing users to read and
> > delete it. This causes the measurement list to be atomically truncated
> > before new measurements can be added. Staging can be done only once at a
> > time. In the event of kexec(), staging is reverted and staged entries will
> > be carried over to the new kernel.
> >
> > User space is responsible to concatenate the staged IMA measurements list
> > portions following the temporal order in which the operations were done,
> > together with the current measurement list. Then, it can send the collected
> > data to the remote verifiers.
> >
> > Also introduce the ability of trimming N measurements entries from the IMA
> > measurements list, provided that user space has already read them. Trimming
> > combines staging and deletion in one operation.
> >
> > The benefit of these solutions is the ability to free precious kernel
> > memory, in exchange of delegating user space to reconstruct the full
> > measurement list from the chunks. No trust needs to be given to user space,
> > since the integrity of the measurement list is protected by the TPM.
> >
> > By default, staging/trimming the measurements list does not alter the hash
> > table. When staging/trimming are done, IMA is still able to detect
> > collisions on the staged and later deleted measurement entries, by keeping
> > the entry digests (only template data are freed).
> >
> > However, since during the measurements list serialization only the SHA1
> > digest is passed, and since there are no template data to recalculate the
> > other digests from, the hash table is currently not populated with digests
> > from staged/deleted entries after kexec().
> >
> > Introduce the new kernel option ima_flush_htable to decide whether or not
> > the digests of staged measurement entries are flushed from the hash table.
> >
> > Then, introduce ascii_runtime_measurements_staged_<algo> and
> > binary_runtime_measurement_staged_<algo> interfaces to stage/trim/delete
> > the measurements. Use 'echo A > <IMA interface>' and
> > 'echo D > <IMA interface>' to respectively stage and delete the entire
> > measurements list. Use 'echo N > <IMA interface>', with N between 1 and
> > LONG_MAX, to stage the selected portion of the measurements list, and
> > 'echo -N > <IMA interface>' to trim N measurements entries.
>
> In an effort to help preserve the sanity of admins, I might suggest
> avoiding commands that start with a dash/'-'. I'd probably also
> simplify the commands a bit and drop all/'A' since the measurement
> list could change at any time, stick with an explicit number and just
> let the admin go over, e.g. write LONG_MAX, which effectively becomes
> 'A'. I think you could do everything you need with just two commands:
>
> <NUM>: stage <NUM> entries
> D: delete staged entries
If the goal is that the verifier always receives a TPM quote aligned
with the measurements, the remote attestation agent in the target
system has to walk over the measurements to find N.
The difference between the approach I was suggesting and Steven's is
that I calculate N after staging all measurements and store the
exceeding measurements locally until the next attestation request. If
the verifier supports it, the exceeding measurements could be stored
also there.
That means that I don't need to walk in the measurement list to stage,
because I stage the entire list (with list_replace()). I do a walk
after detaching, without interfering with the processes adding new
measurements (hot path).
Steven's approach is to read the measurements list to calculate N and
stage/trim the measurement based on N. As Steven/Gregory pointed out,
at this point you could already trim N because you already have the
measurements list.
However, in this case you have to walk through the measurements list as
an RCU reader in the hot path, calculate N, and walk through the
measurements list again as an RCU writer in the hot path to stage/trim
N. In the next attestation request, you would read the previous
exceeding measurements again.
One major obstacle of my approach, as Gregory pointed out, was that
staged measurements were not carried over during kexec(). While I
thought about coordinating remote attestation requests with kexec() in
a management engine, there can be cases where this is harder to
achieve.
I managed to solve that by introducing a third linked list containing
the measurements to delete, by doing another list replace between
staged and measurements to delete (when the 'D' command is issued),
under the hot path lock. That allowed me to take the hot path lock
during kexec() and prepend the staged measurements before the non-
staged ones (that reminded me that I should properly inform user space
if kexec() consumed staged measurements before the 'D' command was
executed, i.e. it lost the race with kexec()).
The approach to keep the stage N approach would be necessary if
exceeding measurements cannot be stored either locally or at the
verifier side.
For me it would be fine to keep both approaches, but I still see
advantages of the stage all approach.
Thanks
Roberto
> I intentionally left out the trim/'T' command, because I'm not sure it
> is really necessary if you are going to implement the phased
> stage/delete process. Yes, you have to do two operations (stage and
> delete) as opposed to just the trim, but I'd probably take the
> simplicity of just supporting a single approach over the trivial
> necessity of having to do two operations in userspace.
>
> Staging also has the benefit of having a sane way of handling two
> tasks racing to stage the measurement list. I could see cases where
> multiple tasks race to trim the list and end up trimming more than was
> intended since they both hit in sequence.
>
> If you did want to take a trim approach over a stage/delete approach,
> I could see something like this working:
>
> 1. process opens the measurement list
> 2. process reads from the measurement list, keeps the fd open
> 3. process does whatever it wants to preserve the list
> 4. process writes <NUM> to the measurement list, kernel trims <NUM> entries
> 5. process closes fd
>
> ... error handling shouldn't be too bad. The process only writes
> <NUM> to the fd if it has already finished whatever it needs to do to
> preserve the list outside the kernel, think of it as a "commit"
> operation on a transaction. If the fd is closed for some reason
> (error, interruption, killed) before the process writes <NUM> to the
> fd then IMA does nothing - no trim takes place.
>
> Multiple process racing can easily be solved when the log is opened;
> only one open(O_RDWR) is allowed at a time, other racing processes
> will get EBUSY. Yes, one process could block others from trimming by
> holding the fd open for an extended period of time, but I would expect
> that CAP_SYS_ADMIN and root fs perms would be required to open the log
> read/write (not to mention any LSM access rights in place).
>
> I know I mentioned this basic idea to someone at some point, but there
> have been various discussion threads and multiple people over a fairly
> lengthy time that I've lost track of where it was mentioned. If it
> was already discussed on-list and rejected for a good reason you can
> simply ignore the above approach ... although I still think the
> stage/delete API could be simplified as described :)
>
> [UPDATE: as I'm reading Steven's replies it looks like he has proposed
> something very similar to the above]
>
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Marco Elver @ 2025-12-15 13:38 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: <20251212110928.GP3911114@noisy.programming.kicks-ass.net>
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.
------ >8 ------
diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index b5563dc83aba..5162962b4b26 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h
@@ -1249,6 +1249,7 @@ struct ss_tmp {
};
static __always_inline void __scoped_seqlock_cleanup(struct ss_tmp *sst)
+ __no_context_analysis
{
if (sst->lock)
spin_unlock(sst->lock);
@@ -1278,6 +1279,7 @@ extern void __scoped_seqlock_bug(void);
static __always_inline void
__scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
+ __no_context_analysis
{
switch (sst->state) {
case ss_done:
@@ -1320,9 +1322,18 @@ __scoped_seqlock_next(struct ss_tmp *sst, seqlock_t *lock, enum ss_state target)
}
}
+/*
+ * Context analysis helper to release seqlock at the end of the for-scope; the
+ * alias analysis of the compiler will recognize that the pointer @s is is an
+ * alias to @_seqlock passed to read_seqbegin(_seqlock) below.
+ */
+static __always_inline void __scoped_seqlock_cleanup_ctx(struct ss_tmp **s)
+ __releases_shared(*((seqlock_t **)s)) __no_context_analysis {}
+
#define __scoped_seqlock_read(_seqlock, _target, _s) \
for (struct ss_tmp _s __cleanup(__scoped_seqlock_cleanup) = \
- { .state = ss_lockless, .data = read_seqbegin(_seqlock) }; \
+ { .state = ss_lockless, .data = read_seqbegin(_seqlock) }, \
+ *__UNIQUE_ID(ctx) __cleanup(__scoped_seqlock_cleanup_ctx) = (struct ss_tmp *)_seqlock; \
_s.state != ss_done; \
__scoped_seqlock_next(&_s, _seqlock, _target))
diff --git a/lib/test_context-analysis.c b/lib/test_context-analysis.c
index 4612025a1065..3f72b1ab2300 100644
--- a/lib/test_context-analysis.c
+++ b/lib/test_context-analysis.c
@@ -261,6 +261,13 @@ static void __used test_seqlock_writer(struct test_seqlock_data *d)
write_sequnlock_irqrestore(&d->sl, flags);
}
+static void __used test_seqlock_scoped(struct test_seqlock_data *d)
+{
+ scoped_seqlock_read (&d->sl, ss_lockless) {
+ (void)d->counter;
+ }
+}
+
struct test_rwsem_data {
struct rw_semaphore sem;
int counter __guarded_by(&sem);
^ permalink raw reply related
* Re: (subset) [PATCH 00/46] Allow inlining C helpers into Rust when using LTO
From: Mark Brown @ 2025-12-15 13:59 UTC (permalink / raw)
To: rust-for-linux, Alice Ryhl
Cc: 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, Danilo Krummrich, 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, 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, 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, Rae Moar
In-Reply-To: <20251202-define-rust-helper-v1-0-a2e13cbc17a6@google.com>
On Tue, 02 Dec 2025 19:37:24 +0000, Alice Ryhl wrote:
> This patch series adds __rust_helper to every single rust helper. The
> patches do not depend on each other, so maintainers please go ahead and
> pick up any patches relevant to your subsystem! Or provide your Acked-by
> so that Miguel can pick them up.
>
> These changes were generated by adding __rust_helper and running
> ClangFormat. Unrelated formatting changes were removed manually.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[35/46] rust: regulator: add __rust_helper to helpers
commit: 03d281f384768610bf90697bce9e35d3d596de77
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH V2 1/1] IMA event log trimming
From: Mimi Zohar @ 2025-12-15 14:02 UTC (permalink / raw)
To: steven chen, linux-integrity
Cc: roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet, serge,
paul, jmorris, linux-security-module, anirudhve, gregorylumen,
nramas, sushring, linux-doc
In-Reply-To: <20251210235314.3341-2-chenste@linux.microsoft.com>
Hi Steven,
The main difference between this patch and Roberto's version is the length of
time needed for locking the measurement list, which prevents new entries from
being appended to the measurement list. In Roberto's version, the list head is
moved quickly and the lock released. Measuring the total amount of time needed
to trim the measurement list ignores the benefit of his version. I plan on
reviewing both this version and his (hopefully today).
There are a number of other things missing from this patch, which I'll enumerate
when I review it.
On Wed, 2025-12-10 at 15:53 -0800, steven chen wrote:
> This patch is for trimming N entries of the IMA event logs. It will also
> cleaning the hash table if ima_flush_htable is set.
Please refer to "Describe your changes in imperative mood" in the "Describe your
changes" section of Documentation/process/submitting-patches.rst.
>
> It provides a userspace interface ima_trim_log that can be used to input
> number N to let kernel to trim N entries of IMA event logs. When read
> this interface, it returns number of entries trimmed last time.
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
--
thanks,
Mimi
^ permalink raw reply
* [PATCH v2 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, 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, Uwe Kleine-König, 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
Cc: 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
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
^ permalink raw reply
* [PATCH v2 14/17] KEYS: trusted: Migrate to use tee specific driver registration function
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, Sumit Garg, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn
Cc: linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
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
^ permalink raw reply related
* [PATCH v2 15/17] KEYS: trusted: Make use of tee bus methods
From: Uwe Kleine-König @ 2025-12-15 14:16 UTC (permalink / raw)
To: Jens Wiklander, Sumit Garg, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn
Cc: linux-integrity, keyrings, linux-security-module, op-tee,
linux-kernel, Sumit Garg
In-Reply-To: <cover.1765791463.git.u.kleine-koenig@baylibre.com>
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;
@@ -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
^ permalink raw reply related
* Re: [PATCH] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing()
From: Christian Brauner @ 2025-12-15 14:19 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Al Viro, Amir Goldstein, NeilBrown, Val Packett, Jan Kara,
linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Chuck Lever, Olga Kornievskaia,
Dai Ngo, Namjae Jeon, Steve French, Sergey Senozhatsky,
Carlos Maiolino, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux
In-Reply-To: <20251205-unmoralisch-jahrtausend-cca02ad0e4fa@brauner>
On Fri, Dec 05, 2025 at 02:09:41PM +0100, Christian Brauner wrote:
> On Mon, Dec 01, 2025 at 03:03:08PM +0100, Miklos Szeredi wrote:
> > On Mon, 1 Dec 2025 at 09:33, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > >
> > > On Mon, Dec 01, 2025 at 09:22:54AM +0100, Amir Goldstein wrote:
> > >
> > > > I don't think there is a point in optimizing parallel dir operations
> > > > with FUSE server cache invalidation, but maybe I am missing
> > > > something.
> > >
> > > The interesting part is the expected semantics of operation;
> > > d_invalidate() side definitely doesn't need any of that cruft,
> > > but I would really like to understand what that function
> > > is supposed to do.
> > >
> > > Miklos, could you post a brain dump on that?
> >
> > This function is supposed to invalidate a dentry due to remote changes
> > (FUSE_NOTIFY_INVAL_ENTRY). Originally it was supplied a parent ID and
> > a name and called d_invalidate() on the looked up dentry.
> >
> > Then it grew a variant (FUSE_NOTIFY_DELETE) that was also supplied a
> > child ID, which was matched against the looked up inode. This was
> > commit 451d0f599934 ("FUSE: Notifying the kernel of deletion."),
> > Apparently this worked around the fact that at that time
> > d_invalidate() returned -EBUSY if the target was still in use and
> > didn't unhash the dentry in that case.
> >
> > That was later changed by commit bafc9b754f75 ("vfs: More precise
> > tests in d_invalidate") to unconditionally unhash the target, which
> > effectively made FUSE_NOTIFY_INVAL_ENTRY and FUSE_NOTIFY_DELETE
> > equivalent and the code in question unnecessary.
> >
> > For the future, we could also introduce FUSE_NOTIFY_MOVE, that would
> > differentiate between a delete and a move, while
> > FUSE_NOTIFY_INVAL_ENTRY would continue to be the common (deleted or
> > moved) notification.
> >
> > Attaching untested patch to remove this cruft.
>
> Should we revert the fuse specific bits of c9ba789dad15 ("VFS: introduce
> start_creating_noperm() and start_removing_noperm()") and then apply
> your changes afterwards?
I think we shouldn't have this sitting around indefinitely so it would
be good if we'd get a nod that this is ok or someone sending revert +
fix that I can pick up. :)
^ permalink raw reply
* [PATCH] loadpin: Implement custom proc_handler for enforce
From: Joel Granados @ 2025-12-15 15:43 UTC (permalink / raw)
To: Kees Cook, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-security-module, linux-kernel, Joel Granados
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.
---
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;
+
+static int proc_handler_loadpin(const struct ctl_table *table, int dir,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ if (!is_loadpin_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)
{
/*
@@ -72,9 +83,9 @@ static void set_sysctl(bool is_writable)
* device, allow sysctl to change modes for testing.
*/
if (is_writable)
- loadpin_sysctl_table[0].extra1 = SYSCTL_ZERO;
+ is_loadpin_writable = true;
else
- loadpin_sysctl_table[0].extra1 = SYSCTL_ONE;
+ is_loadpin_writable = false;
}
#else
static inline void set_sysctl(bool is_writable) { }
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251215-jag-const_loadpin-761f052f76c4
Best regards,
--
Joel Granados <joel.granados@kernel.org>
^ permalink raw reply related
* 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
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