* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Günther Noack @ 2026-03-04 7:44 UTC (permalink / raw)
To: Ding Yihan
Cc: Tingmao Wang, Justin Suess, Mickaël Salaün, Paul Moore,
Jann Horn, linux-security-module, linux-kernel,
syzbot+7ea2f5e9dfd468201817
In-Reply-To: <D31C8311F753F56D+d07bcc15-ede7-4a04-829d-d80f69abda83@uniontech.com>
On Wed, Mar 04, 2026 at 10:46:39AM +0800, Ding Yihan wrote:
> Hi all,
>
> Thank you Justin for catching the test failure and the thorough
> investigation! And thanks Günther and Tingmao for diving into the
> syscall restart mechanics.
>
> I've evaluated both the `while` loop approach with `task_work_run()`
> and the `restart_syscall()` approach. I strongly lean towards using
> `restart_syscall()` as suggested by Tingmao.
>
> As Günther pointed out earlier, executing `task_work_run()` directly
> deep inside the syscall context can be risky. Task works often assume
> they are running at the kernel-user boundary with a specific state.
> Using `restart_syscall()` safely bounces us to that boundary, processes
> the works cleanly, and restarts the syscall via standard mechanisms.
Agreed. I also like the restart_syscall() solution for its simplicity
and use of a standard mechanism.
(This code path is very unlikely (and probably unintended by the
userspace programmer), so we need to protect against deadlock, but
it's not a performance critical path by far. By using the more
standard restart_syscall(), we have to worry about fewer corner cases
(e.g. what assumptions are made by task_works about the context they
get executed in). I think this robustness trumps performance tuning
in this case.)
> After some selftests,I will prepare the v4 patch series using `restart_syscall()`.
> I will also ensure all comments are properly wrapped to 80 columns as requested
> by Mickaël, and make sure to include the proper Reported-by and
> Suggested-by tags for everyone's excellent input here.
>
> Expect the v4 series shortly. Thanks again for the great collaboration!
Thanks, I'm looking forward to the revised patch. I agree with this plan. :)
–Günther
^ permalink raw reply
* Re: [PATCH v2 000/110] vfs: change inode->i_ino from unsigned long to u64
From: NeilBrown @ 2026-03-04 6:26 UTC (permalink / raw)
To: Jeff Layton
Cc: David Howells, Alexander Viro, Christian Brauner, Jan Kara,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Dan Williams,
Matthew Wilcox, Eric Biggers, Theodore Y. Ts'o, Muchun Song,
Oscar Salvador, David Hildenbrand, Paulo Alcantara,
Andreas Dilger, Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust,
Anna Schumaker, Chuck Lever, Olga Kornievskaia, Dai Ngo,
Tom Talpey, Steve French, Ronnie Sahlberg, Shyam Prasad N,
Bharath SM, Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <1c28e34c7167acf4e20c3e201476504135aa44e8.camel@kernel.org>
On Tue, 03 Mar 2026, Jeff Layton wrote:
> On Tue, 2026-03-03 at 10:55 +0000, David Howells wrote:
> > Jeff Layton <jlayton@kernel.org> wrote:
> >
> > > This version splits the change up to be more bisectable. It first adds a
> > > new kino_t typedef and a new "PRIino" macro to hold the width specifier
> > > for format strings. The conversion is done, and then everything is
> > > changed to remove the new macro and typedef.
> >
> > Why remove the typedef? It might be better to keep it.
> >
>
> Why? After this change, internel kernel inodes will be u64's -- full
> stop. I don't see what the macro or typedef will buy us at that point.
Implicit documentation?
ktime_t is (now) always s64, but we still keep the typedef;
It would be cool if we could teach vsprintf to understand some new
specifier to mean "kinode_t" or "ktime_t" etc. But that would trigger
gcc warnings.
NeilBrown
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Ding Yihan @ 2026-03-04 2:46 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Justin Suess, Mickaël Salaün, Paul Moore, Jann Horn,
linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260303.94e335a9bdaa@gnoack.org>
Hi all,
Thank you Justin for catching the test failure and the thorough
investigation! And thanks Günther and Tingmao for diving into the
syscall restart mechanics.
I've evaluated both the `while` loop approach with `task_work_run()`
and the `restart_syscall()` approach. I strongly lean towards using
`restart_syscall()` as suggested by Tingmao.
As Günther pointed out earlier, executing `task_work_run()` directly
deep inside the syscall context can be risky. Task works often assume
they are running at the kernel-user boundary with a specific state.
Using `restart_syscall()` safely bounces us to that boundary, processes
the works cleanly, and restarts the syscall via standard mechanisms.
After some selftests,I will prepare the v4 patch series using `restart_syscall()`.
I will also ensure all comments are properly wrapped to 80 columns as requested
by Mickaël, and make sure to include the proper Reported-by and
Suggested-by tags for everyone's excellent input here.
Expect the v4 series shortly. Thanks again for the great collaboration!
Best regards,
Yihan Ding
在 2026/3/4 05:19, Günther Noack 写道:
> On Tue, Mar 03, 2026 at 08:38:13PM +0000, Tingmao Wang wrote:
>> On 3/3/26 19:50, Günther Noack wrote:
>>> [...]
>>> On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
>>>> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
>>>>> [...]
>>>>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
>>>>> index de01aa899751..xxxxxxxxxxxx 100644
>>>>> --- a/security/landlock/tsync.c
>>>>> +++ b/security/landlock/tsync.c
>>>>> @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>>>>> shared_ctx.new_cred = new_cred;
>>>>> shared_ctx.set_no_new_privs = task_no_new_privs(current);
>>>>>
>>>>> + /*
>>>>> + * Serialize concurrent TSYNC operations to prevent deadlocks
>>>>> + * when multiple threads call landlock_restrict_self() simultaneously.
>>>>> + */
>>>>> + if (!down_write_trylock(¤t->signal->exec_update_lock))
>>>>> + return -ERESTARTNOINTR;
>>>> These two lines above introduced a test failure in tsync_test
>>>> completing_enablement.
>>>>
>>>> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
>>>> (this patch) and is currently in the mic/next branch.
>>>>
>>>> I noticed the test failure while testing an unrelated patch.
>>>>
>>>> The bug is because this code never actually yields or restarts the syscall.
>>>>
>>>> This is the test output I observed:
>>>>
>>>> [+] Running tsync_test:
>>>> TAP version 13
>>>> 1..4
>>>> # Starting 4 tests from 1 test cases.
>>>> # RUN global.single_threaded_success ...
>>>> # OK global.single_threaded_success
>>>> ok 1 global.single_threaded_success
>>>> # RUN global.multi_threaded_success ...
>>>> # OK global.multi_threaded_success
>>>> ok 2 global.multi_threaded_success
>>>> # RUN global.multi_threaded_success_despite_diverging_domains ...
>>>> # OK global.multi_threaded_success_despite_diverging_domains
>>>> ok 3 global.multi_threaded_success_despite_diverging_domains
>>>> # RUN global.competing_enablement ...
>>>> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
>>>
>>> The interesting part here is when you print out the errno that is
>>> returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
>>>
>>> My understanding so far: Poking around in kernel/entry/common.c, it
>>> seems that __exit_to_user_mode_loop() calls
>>> arch_do_signal_or_restart() only when there is a pending signal
>>> (_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL). So it was possible that the
>>> system call returns with the (normally internal) error code
>>> ERESTARTNOINTR, in the case where the trylock fails, but where current
>>> has not received a signal from the other competing TSYNC thread yet.
>>>
>>> So with that in mind, would it work to do this?
>>>
>>> while (try-to-acquire-the-lock) {
>>> if (current-has-task-works-pending)
>>> return -ERESTARTNOINTR;
>>>
>>> cond_resched();
>>> }
>>>
>>> Then we could avoid calling task_work_run() directly; (I find it
>>> difficult to reason about the implications of calling taks_work_run()
>>> directly, because these task works may make assumptions about the
>>> context in which they are running.)
>>
>> I've not caught up with the full discussion so might be missing some context on why RESTARTNOINTR was used here,
>> but wouldn't
>>
>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
>> index 950b63d23729..f695fe44e2f1 100644
>> --- a/security/landlock/tsync.c
>> +++ b/security/landlock/tsync.c
>> @@ -490,7 +490,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>> * when multiple threads call landlock_restrict_self() simultaneously.
>> */
>> if (!down_write_trylock(¤t->signal->exec_update_lock))
>> - return -ERESTARTNOINTR;
>> + return restart_syscall();
>>
>> /*
>> * We schedule a pseudo-signal task_work for each of the calling task's
>>
>> achieve what the original patch intended?
>
> Thanks, that's an excellent point!
>
> restart_syscall() (a) sets TIF_SIGPENDING and then (b) returns
> -ERESTARTNOINTR. (a) was the part that we have been missing for the
> restart to work (see discussion above). Together, (a) and (b) cause
> __exit_to_user_mode_loop() to restart the syscall. Given that this is
> offered in signal.h, this seems like a clean and more "official" way
> to do this than using the task works APIs.
>
> It also fixes the previously failing selftest (I tried).
>
> Yihan, Justin: Does that seem reasonable to you as well?
>
> –Günther
>
^ permalink raw reply
* Re: [PATCH v2 053/110] uprobes: use PRIino format for i_ino
From: Masami Hiramatsu @ 2026-03-04 1:12 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Mathieu Desnoyers, Dan Williams, Matthew Wilcox, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260302-iino-u64-v2-53-e5388800dae0@kernel.org>
On Mon, 02 Mar 2026 15:24:37 -0500
Jeff Layton <jlayton@kernel.org> wrote:
> Convert uprobes i_ino format strings to use the PRIino format
> macro in preparation for the widening of i_ino via kino_t.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> ---
> kernel/events/uprobes.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 923b24b321cc0fbdecaf016645cdac0457a74463..d5bf51565851223730c63b50436c493c0c05eafd 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -344,7 +344,7 @@ __update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d)
> static void update_ref_ctr_warn(struct uprobe *uprobe,
> struct mm_struct *mm, short d)
> {
> - pr_warn("ref_ctr %s failed for inode: 0x%lx offset: "
> + pr_warn("ref_ctr %s failed for inode: 0x%" PRIino "x offset: "
> "0x%llx ref_ctr_offset: 0x%llx of mm: 0x%p\n",
> d > 0 ? "increment" : "decrement", uprobe->inode->i_ino,
> (unsigned long long) uprobe->offset,
> @@ -982,7 +982,7 @@ static struct uprobe *insert_uprobe(struct uprobe *uprobe)
> static void
> ref_ctr_mismatch_warn(struct uprobe *cur_uprobe, struct uprobe *uprobe)
> {
> - pr_warn("ref_ctr_offset mismatch. inode: 0x%lx offset: 0x%llx "
> + pr_warn("ref_ctr_offset mismatch. inode: 0x%" PRIino "x offset: 0x%llx "
> "ref_ctr_offset(old): 0x%llx ref_ctr_offset(new): 0x%llx\n",
> uprobe->inode->i_ino, (unsigned long long) uprobe->offset,
> (unsigned long long) cur_uprobe->ref_ctr_offset,
>
> --
> 2.53.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] keys: Use kmalloc_flex() to improve user_preparse()
From: Jarkko Sakkinen @ 2026-03-04 0:17 UTC (permalink / raw)
To: Thorsten Blum
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-hardening, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260302111309.937726-3-thorsten.blum@linux.dev>
On Mon, Mar 02, 2026 at 12:13:11PM +0100, Thorsten Blum wrote:
> Use kmalloc_flex() when allocating a new 'struct user_key_payload' in
> user_preparse() to replace the open-coded size arithmetic and to keep
> the size type-safe.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/user_defined.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
> index 686d56e4cc85..6f88b507f927 100644
> --- a/security/keys/user_defined.c
> +++ b/security/keys/user_defined.c
> @@ -64,7 +64,7 @@ int user_preparse(struct key_preparsed_payload *prep)
> if (datalen == 0 || datalen > 32767 || !prep->data)
> return -EINVAL;
>
> - upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
> + upayload = kmalloc_flex(*upayload, data, datalen);
> if (!upayload)
> return -ENOMEM;
>
> --
> Thorsten Blum <thorsten.blum@linux.dev>
> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
>
David, do we want this?
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: Remove return variable and length check to simplify user_read
From: Jarkko Sakkinen @ 2026-03-04 0:10 UTC (permalink / raw)
To: Thorsten Blum
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260228094447.869637-1-thorsten.blum@linux.dev>
On Sat, Feb 28, 2026 at 10:44:46AM +0100, Thorsten Blum wrote:
> In user_read(), remove the unnecessary return variable 'ret' and return
> ->datalen directly. Drop the redundant 'buflen > 0' check and use min()
> to determine the number of bytes to copy. No functional changes.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Trivial cleanup commits with no function are not feasible. They only do
harm e.g., for backporting actual bug fixes.
> ---
> security/keys/user_defined.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
> index 6f88b507f927..b53e063272c2 100644
> --- a/security/keys/user_defined.c
> +++ b/security/keys/user_defined.c
> @@ -171,20 +171,14 @@ EXPORT_SYMBOL_GPL(user_describe);
> long user_read(const struct key *key, char *buffer, size_t buflen)
> {
> const struct user_key_payload *upayload;
> - long ret;
>
> upayload = user_key_payload_locked(key);
> - ret = upayload->datalen;
>
> /* we can return the data as is */
> - if (buffer && buflen > 0) {
> - if (buflen > upayload->datalen)
> - buflen = upayload->datalen;
> + if (buffer)
> + memcpy(buffer, upayload->data, min(buflen, upayload->datalen));
>
> - memcpy(buffer, upayload->data, buflen);
> - }
> -
> - return ret;
> + return upayload->datalen;
> }
>
> EXPORT_SYMBOL_GPL(user_read);
> --
> Thorsten Blum <thorsten.blum@linux.dev>
> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options
From: Jarkko Sakkinen @ 2026-03-03 21:45 UTC (permalink / raw)
To: Srish Srinivasan
Cc: linux-integrity, keyrings, James.Bottomley, zohar, stefanb, nayna,
linux-kernel, linux-security-module
In-Reply-To: <20260220183426.80446-3-ssrish@linux.ibm.com>
On Sat, Feb 21, 2026 at 12:04:26AM +0530, Srish Srinivasan wrote:
> The trusted_key_options struct contains TPM-specific fields (keyhandle,
> keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash,
> policydigest_len, policydigest, and policyhandle). This leads to the
> accumulation of backend-specific fields in the generic options structure.
>
> Define trusted_tpm_options structure and move the TPM-specific fields
> there. Store a pointer to trusted_tpm_options in trusted_key_options's
> private.
>
> No functional change intended.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> include/keys/trusted-type.h | 11 ---
> include/keys/trusted_tpm.h | 14 ++++
> security/keys/trusted-keys/trusted_tpm1.c | 95 ++++++++++++++---------
> security/keys/trusted-keys/trusted_tpm2.c | 51 ++++++------
> 4 files changed, 102 insertions(+), 69 deletions(-)
>
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f..b80f250305b8 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -39,17 +39,6 @@ struct trusted_key_payload {
>
> struct trusted_key_options {
> uint16_t keytype;
> - uint32_t keyhandle;
> - unsigned char keyauth[TPM_DIGEST_SIZE];
> - uint32_t blobauth_len;
> - unsigned char blobauth[TPM_DIGEST_SIZE];
> - uint32_t pcrinfo_len;
> - unsigned char pcrinfo[MAX_PCRINFO_SIZE];
> - int pcrlock;
> - uint32_t hash;
> - uint32_t policydigest_len;
> - unsigned char policydigest[MAX_DIGEST_SIZE];
> - uint32_t policyhandle;
> void *private;
> };
>
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index 0fadc6a4f166..355ebd36cbfd 100644
> --- a/include/keys/trusted_tpm.h
> +++ b/include/keys/trusted_tpm.h
> @@ -7,6 +7,20 @@
>
> extern struct trusted_key_ops trusted_key_tpm_ops;
>
> +struct trusted_tpm_options {
> + uint32_t keyhandle;
> + unsigned char keyauth[TPM_DIGEST_SIZE];
> + uint32_t blobauth_len;
> + unsigned char blobauth[TPM_DIGEST_SIZE];
> + uint32_t pcrinfo_len;
> + unsigned char pcrinfo[MAX_PCRINFO_SIZE];
> + int pcrlock;
> + uint32_t hash;
> + uint32_t policydigest_len;
> + unsigned char policydigest[MAX_DIGEST_SIZE];
> + uint32_t policyhandle;
> +};
> +
> int tpm2_seal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options);
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 216caef97ffc..741b1d47d9f8 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -48,12 +48,14 @@ enum {
>
> static inline void dump_options(struct trusted_key_options *o)
> {
> + struct trusted_tpm_options *private = o->private;
> +
> pr_debug("sealing key type %d\n", o->keytype);
> - pr_debug("sealing key handle %0X\n", o->keyhandle);
> - pr_debug("pcrlock %d\n", o->pcrlock);
> - pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> + pr_debug("sealing key handle %0X\n", private->keyhandle);
> + pr_debug("pcrlock %d\n", private->pcrlock);
> + pr_debug("pcrinfo %d\n", private->pcrinfo_len);
> print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> - 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> + 16, 1, private->pcrinfo, private->pcrinfo_len, 0);
> }
>
> static inline void dump_sess(struct osapsess *s)
> @@ -609,6 +611,7 @@ static int tpm_unseal(struct tpm_buf *tb,
> static int key_seal(struct trusted_key_payload *p,
> struct trusted_key_options *o)
> {
> + struct trusted_tpm_options *private = o->private;
> struct tpm_buf tb;
> int ret;
>
> @@ -619,9 +622,10 @@ static int key_seal(struct trusted_key_payload *p,
> /* include migratable flag at end of sealed key */
> p->key[p->key_len] = p->migratable;
>
> - ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
> + ret = tpm_seal(&tb, o->keytype, private->keyhandle, private->keyauth,
> p->key, p->key_len + 1, p->blob, &p->blob_len,
> - o->blobauth, o->pcrinfo, o->pcrinfo_len);
> + private->blobauth, private->pcrinfo,
> + private->pcrinfo_len);
> if (ret < 0)
> pr_info("srkseal failed (%d)\n", ret);
>
> @@ -635,6 +639,7 @@ static int key_seal(struct trusted_key_payload *p,
> static int key_unseal(struct trusted_key_payload *p,
> struct trusted_key_options *o)
> {
> + struct trusted_tpm_options *private = o->private;
> struct tpm_buf tb;
> int ret;
>
> @@ -642,8 +647,8 @@ static int key_unseal(struct trusted_key_payload *p,
> if (ret)
> return ret;
>
> - ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
> - o->blobauth, p->key, &p->key_len);
> + ret = tpm_unseal(&tb, private->keyhandle, private->keyauth, p->blob,
> + p->blob_len, private->blobauth, p->key, &p->key_len);
> if (ret < 0)
> pr_info("srkunseal failed (%d)\n", ret);
> else
> @@ -680,6 +685,7 @@ static const match_table_t key_tokens = {
> static int getoptions(char *c, struct trusted_key_payload *pay,
> struct trusted_key_options *opt)
> {
> + struct trusted_tpm_options *private = opt->private;
> substring_t args[MAX_OPT_ARGS];
> char *p = c;
> int token;
> @@ -695,7 +701,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> if (tpm2 < 0)
> return tpm2;
>
> - opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> + private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
>
> if (!c)
> return 0;
> @@ -709,11 +715,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>
> switch (token) {
> case Opt_pcrinfo:
> - opt->pcrinfo_len = strlen(args[0].from) / 2;
> - if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
> + private->pcrinfo_len = strlen(args[0].from) / 2;
> + if (private->pcrinfo_len > MAX_PCRINFO_SIZE)
> return -EINVAL;
> - res = hex2bin(opt->pcrinfo, args[0].from,
> - opt->pcrinfo_len);
> + res = hex2bin(private->pcrinfo, args[0].from,
> + private->pcrinfo_len);
> if (res < 0)
> return -EINVAL;
> break;
> @@ -722,12 +728,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> if (res < 0)
> return -EINVAL;
> opt->keytype = SEAL_keytype;
> - opt->keyhandle = handle;
> + private->keyhandle = handle;
> break;
> case Opt_keyauth:
> if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
> return -EINVAL;
> - res = hex2bin(opt->keyauth, args[0].from,
> + res = hex2bin(private->keyauth, args[0].from,
> SHA1_DIGEST_SIZE);
> if (res < 0)
> return -EINVAL;
> @@ -738,21 +744,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> * hex strings. TPM 2.0 authorizations are simple
> * passwords (although it can take a hash as well)
> */
> - opt->blobauth_len = strlen(args[0].from);
> + private->blobauth_len = strlen(args[0].from);
>
> - if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
> - res = hex2bin(opt->blobauth, args[0].from,
> + if (private->blobauth_len == 2 * TPM_DIGEST_SIZE) {
> + res = hex2bin(private->blobauth, args[0].from,
> TPM_DIGEST_SIZE);
> if (res < 0)
> return -EINVAL;
>
> - opt->blobauth_len = TPM_DIGEST_SIZE;
> + private->blobauth_len = TPM_DIGEST_SIZE;
> break;
> }
>
> - if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
> - memcpy(opt->blobauth, args[0].from,
> - opt->blobauth_len);
> + if (tpm2 &&
> + private->blobauth_len <=
> + sizeof(private->blobauth)) {
> + memcpy(private->blobauth, args[0].from,
> + private->blobauth_len);
> break;
> }
>
> @@ -770,14 +778,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> res = kstrtoul(args[0].from, 10, &lock);
> if (res < 0)
> return -EINVAL;
> - opt->pcrlock = lock;
> + private->pcrlock = lock;
> break;
> case Opt_hash:
> if (test_bit(Opt_policydigest, &token_mask))
> return -EINVAL;
> for (i = 0; i < HASH_ALGO__LAST; i++) {
> if (!strcmp(args[0].from, hash_algo_name[i])) {
> - opt->hash = i;
> + private->hash = i;
> break;
> }
> }
> @@ -789,14 +797,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> }
> break;
> case Opt_policydigest:
> - digest_len = hash_digest_size[opt->hash];
> + digest_len = hash_digest_size[private->hash];
> if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
> return -EINVAL;
> - res = hex2bin(opt->policydigest, args[0].from,
> + res = hex2bin(private->policydigest, args[0].from,
> digest_len);
> if (res < 0)
> return -EINVAL;
> - opt->policydigest_len = digest_len;
> + private->policydigest_len = digest_len;
> break;
> case Opt_policyhandle:
> if (!tpm2)
> @@ -804,7 +812,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> res = kstrtoul(args[0].from, 16, &handle);
> if (res < 0)
> return -EINVAL;
> - opt->policyhandle = handle;
> + private->policyhandle = handle;
> break;
> default:
> return -EINVAL;
> @@ -815,6 +823,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>
> static struct trusted_key_options *trusted_options_alloc(void)
> {
> + struct trusted_tpm_options *private;
> struct trusted_key_options *options;
> int tpm2;
>
> @@ -827,14 +836,23 @@ static struct trusted_key_options *trusted_options_alloc(void)
> /* set any non-zero defaults */
> options->keytype = SRK_keytype;
>
> - if (!tpm2)
> - options->keyhandle = SRKHANDLE;
> + private = kzalloc(sizeof(*private), GFP_KERNEL);
> + if (!private) {
> + kfree_sensitive(options);
> + options = NULL;
> + } else {
> + if (!tpm2)
> + private->keyhandle = SRKHANDLE;
> +
> + options->private = private;
> + }
> }
> return options;
> }
>
> static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
> {
> + struct trusted_tpm_options *private = NULL;
> struct trusted_key_options *options = NULL;
> int ret = 0;
> int tpm2;
> @@ -852,7 +870,8 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
> goto out;
> dump_options(options);
>
> - if (!options->keyhandle && !tpm2) {
> + private = options->private;
> + if (!private->keyhandle && !tpm2) {
> ret = -EINVAL;
> goto out;
> }
> @@ -866,20 +885,22 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
> goto out;
> }
>
> - if (options->pcrlock) {
> - ret = pcrlock(options->pcrlock);
> + if (private->pcrlock) {
> + ret = pcrlock(private->pcrlock);
> if (ret < 0) {
> pr_info("pcrlock failed (%d)\n", ret);
> goto out;
> }
> }
> out:
> + kfree_sensitive(options->private);
> kfree_sensitive(options);
> return ret;
> }
>
> static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> {
> + struct trusted_tpm_options *private = NULL;
> struct trusted_key_options *options = NULL;
> int ret = 0;
> int tpm2;
> @@ -897,7 +918,8 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> goto out;
> dump_options(options);
>
> - if (!options->keyhandle && !tpm2) {
> + private = options->private;
> + if (!private->keyhandle && !tpm2) {
> ret = -EINVAL;
> goto out;
> }
> @@ -909,14 +931,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> if (ret < 0)
> pr_info("key_unseal failed (%d)\n", ret);
>
> - if (options->pcrlock) {
> - ret = pcrlock(options->pcrlock);
> + if (private->pcrlock) {
> + ret = pcrlock(private->pcrlock);
> if (ret < 0) {
> pr_info("pcrlock failed (%d)\n", ret);
> goto out;
> }
> }
> out:
> + kfree_sensitive(options->private);
> kfree_sensitive(options);
> return ret;
> }
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 6340823f8b53..94e01249b921 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -24,6 +24,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> struct trusted_key_options *options,
> u8 *src, u32 len)
> {
> + struct trusted_tpm_options *private = options->private;
> const int SCRATCH_SIZE = PAGE_SIZE;
> u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
> u8 *work = scratch, *work1;
> @@ -46,7 +47,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> work = asn1_encode_oid(work, end_work, tpm2key_oid,
> asn1_oid_len(tpm2key_oid));
>
> - if (options->blobauth_len == 0) {
> + if (private->blobauth_len == 0) {
> unsigned char bool[3], *w = bool;
> /* tag 0 is emptyAuth */
> w = asn1_encode_boolean(w, w + sizeof(bool), true);
> @@ -69,7 +70,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> goto err;
> }
>
> - work = asn1_encode_integer(work, end_work, options->keyhandle);
> + work = asn1_encode_integer(work, end_work, private->keyhandle);
> work = asn1_encode_octet_string(work, end_work, pub, pub_len);
> work = asn1_encode_octet_string(work, end_work, priv, priv_len);
>
> @@ -102,6 +103,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
> struct trusted_key_options *options,
> u8 **buf)
> {
> + struct trusted_tpm_options *private = options->private;
> int ret;
> struct tpm2_key_context ctx;
> u8 *blob;
> @@ -121,7 +123,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
> return -ENOMEM;
>
> *buf = blob;
> - options->keyhandle = ctx.parent;
> + private->keyhandle = ctx.parent;
>
> memcpy(blob, ctx.priv, ctx.priv_len);
> blob += ctx.priv_len;
> @@ -233,6 +235,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options)
> {
> + struct trusted_tpm_options *private = options->private;
> off_t offset = TPM_HEADER_SIZE;
> struct tpm_buf buf, sized;
> int blob_len = 0;
> @@ -240,11 +243,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> u32 flags;
> int rc;
>
> - hash = tpm2_find_hash_alg(options->hash);
> + hash = tpm2_find_hash_alg(private->hash);
> if (hash < 0)
> return hash;
>
> - if (!options->keyhandle)
> + if (!private->keyhandle)
> return -EINVAL;
>
> rc = tpm_try_get_ops(chip);
> @@ -268,18 +271,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> goto out_put;
> }
>
> - rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> + rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
> if (rc)
> goto out;
>
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> - options->keyauth, TPM_DIGEST_SIZE);
> + private->keyauth, TPM_DIGEST_SIZE);
>
> /* sensitive */
> - tpm_buf_append_u16(&sized, options->blobauth_len);
> + tpm_buf_append_u16(&sized, private->blobauth_len);
>
> - if (options->blobauth_len)
> - tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
> + if (private->blobauth_len)
> + tpm_buf_append(&sized, private->blobauth,
> + private->blobauth_len);
>
> tpm_buf_append_u16(&sized, payload->key_len);
> tpm_buf_append(&sized, payload->key, payload->key_len);
> @@ -292,14 +296,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>
> /* key properties */
> flags = 0;
> - flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
> + flags |= private->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
> flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
> tpm_buf_append_u32(&sized, flags);
>
> /* policy */
> - tpm_buf_append_u16(&sized, options->policydigest_len);
> - if (options->policydigest_len)
> - tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
> + tpm_buf_append_u16(&sized, private->policydigest_len);
> + if (private->policydigest_len)
> + tpm_buf_append(&sized, private->policydigest,
> + private->policydigest_len);
>
> /* public parameters */
> tpm_buf_append_u16(&sized, TPM_ALG_NULL);
> @@ -373,6 +378,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> u32 *blob_handle)
> {
> u8 *blob_ref __free(kfree) = NULL;
> + struct trusted_tpm_options *private = options->private;
> struct tpm_buf buf;
> unsigned int private_len;
> unsigned int public_len;
> @@ -392,7 +398,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> }
>
> /* new format carries keyhandle but old format doesn't */
> - if (!options->keyhandle)
> + if (!private->keyhandle)
> return -EINVAL;
>
> /* must be big enough for at least the two be16 size counts */
> @@ -433,11 +439,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> return rc;
> }
>
> - rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> + rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
> if (rc)
> goto out;
>
> - tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> + tpm_buf_append_hmac_session(chip, &buf, 0, private->keyauth,
> TPM_DIGEST_SIZE);
>
> tpm_buf_append(&buf, blob, blob_len);
> @@ -481,6 +487,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> struct trusted_key_options *options,
> u32 blob_handle)
> {
> + struct trusted_tpm_options *private = options->private;
> struct tpm_header *head;
> struct tpm_buf buf;
> u16 data_len;
> @@ -502,10 +509,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> if (rc)
> goto out;
>
> - if (!options->policyhandle) {
> + if (!private->policyhandle) {
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> - options->blobauth,
> - options->blobauth_len);
> + private->blobauth,
> + private->blobauth_len);
> } else {
> /*
> * FIXME: The policy session was generated outside the
> @@ -518,9 +525,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> * could repeat our actions with the exfiltrated
> * password.
> */
> - tpm2_buf_append_auth(&buf, options->policyhandle,
> + tpm2_buf_append_auth(&buf, private->policyhandle,
> NULL /* nonce */, 0, 0,
> - options->blobauth, options->blobauth_len);
> + private->blobauth, private->blobauth_len);
> if (tpm2_chip_auth(chip)) {
> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
> } else {
> --
> 2.43.0
>
Applied.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
From: Jarkko Sakkinen @ 2026-03-03 21:36 UTC (permalink / raw)
To: Srish Srinivasan
Cc: linux-integrity, keyrings, James.Bottomley, zohar, stefanb, nayna,
linux-kernel, linux-security-module
In-Reply-To: <20260220183426.80446-2-ssrish@linux.ibm.com>
On Sat, Feb 21, 2026 at 12:04:25AM +0530, Srish Srinivasan wrote:
> The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> for logging debug information.
>
> Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> Remove TPM_DEBUG.
>
> No functional change intended.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
> 1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index c865c97aa1b4..216caef97ffc 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -46,28 +46,25 @@ enum {
> SRK_keytype = 4
> };
>
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
> static inline void dump_options(struct trusted_key_options *o)
> {
> - pr_info("sealing key type %d\n", o->keytype);
> - pr_info("sealing key handle %0X\n", o->keyhandle);
> - pr_info("pcrlock %d\n", o->pcrlock);
> - pr_info("pcrinfo %d\n", o->pcrinfo_len);
> - print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> + pr_debug("sealing key type %d\n", o->keytype);
> + pr_debug("sealing key handle %0X\n", o->keyhandle);
> + pr_debug("pcrlock %d\n", o->pcrlock);
> + pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> + print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> }
>
> static inline void dump_sess(struct osapsess *s)
> {
> - print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> + print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
> 16, 1, &s->handle, 4, 0);
> - pr_info("secret:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> + pr_debug("secret:\n");
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> - pr_info("trusted-key: enonce:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> + pr_debug("trusted-key: enonce:\n");
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> }
>
> @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
> {
> int len;
>
> - pr_info("\ntpm buffer\n");
> + pr_debug("\ntpm buffer\n");
> len = LOAD32(buf, TPM_SIZE_OFFSET);
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> -}
> -#else
> -static inline void dump_options(struct trusted_key_options *o)
> -{
> -}
> -
> -static inline void dump_sess(struct osapsess *s)
> -{
> -}
> -
> -static inline void dump_tpm_buf(unsigned char *buf)
> -{
> + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> }
> -#endif
>
> static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> unsigned int keylen, ...)
> --
> 2.43.0
>
Applied.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-03 21:32 UTC (permalink / raw)
To: Chris Fenner
Cc: Mimi Zohar, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <CAMigqh1H1NKP9gddjhf4M1v-aM=+EpW9O4KJmu=UysOWhn4ryA@mail.gmail.com>
On Fri, Feb 20, 2026 at 10:30:26AM -0800, Chris Fenner wrote:
> My conclusion about TCG_TPM2_HMAC after [1] and [2] was that
> TPM2_TCG_HMAC doesn't (or didn't at the time) actually solve the
> threat model it claims to (active interposer adversaries), while
> dramatically increasing the cost of many kernel TPM activities beyond
> the amount that would have been required to just solve for
> passive/bus-sniffer interposer adversaries. The added symmetric crypto
> required to secure a TPM transaction is almost not noticeable; the big
> performance problem is the re-bootstrapping of the session with ECDH
> for every command.
>
> My primary concern at that time was, essentially, that TCG_TPM2_HMAC
> punts on checking that the key that was used to secure the session was
> actually resident in a real TPM and not just an interposer adversary.
> I wrote up my understanding at
> https://www.dlp.rip/decorative-cryptography, for anyone who wants a
> long-form opinionated take :).
>
> Unless I'm wrong, or TCG_TPM2_HMAC has changed dramatically since
> August, I don't think "TPM2_TCG_HMAC makes this too costly" is a
> compelling reason to make a security decision. (There could be other
> reasons to make choices about whether to use the TPM as a source of
> randomness in the kernel! This just isn't one IMHO.)
>
> The version of TCG_TPM2_HMAC that I'd like to see someday would be one
> that fully admits that its threat model is only passive interposers,
> and sets up one session upon startup and ContextSaves/ContextLoads it
> back into the TPM as needed in order to secure parameter encryption
> for e.g., GetRandom() and Unseal() calls.
Neither agreeing nor disagreeing but this patch set clearly does not
move forward and I spent already enough energy for this. For better
ideas the patches are available in queue branch.
High-level takes don't move anything forward (or backward), sorry.
>
> [1]: https://lore.kernel.org/linux-integrity/CAMigqh2nwuRRxaLyOJ+QaTJ+XGmkQj=rMj5K9GP1bCcXp2OsBQ@mail.gmail.com/
> [2]: https://lore.kernel.org/linux-integrity/20250825203223.629515-1-jarkko@kernel.org/
>
> Thanks
> Chris
>
> On Fri, Feb 20, 2026 at 10:04 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > [Cc: Chris Fenner, Jonathan McDowell, Roberto]
> >
> > On Sun, 2026-01-25 at 21:25 +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.
> >
> > If the concern is the latency of encrypting the bus session, please remember
> > that:
> >
> > - Not all environments expose the TPM bus to sniffing.
> > - The current TPM trusted keys design is based on TPM RNG, but already allows it
> > to be replaced with the kernel RNG via the "trusted_rng=kernel" boot command
> > line option.
> > - The proposed patch removes that possibility for no reason.
> >
> > Mimi & Elaine
> >
> >
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-03 21:30 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Chris Fenner, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <06a08cbbe47111a1795e5dcd42fb8cc4be643a72.camel@linux.ibm.com>
On Fri, Feb 20, 2026 at 01:04:30PM -0500, Mimi Zohar wrote:
> [Cc: Chris Fenner, Jonathan McDowell, Roberto]
>
> On Sun, 2026-01-25 at 21:25 +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.
>
> If the concern is the latency of encrypting the bus session, please remember
> that:
>
> - Not all environments expose the TPM bus to sniffing.
> - The current TPM trusted keys design is based on TPM RNG, but already allows it
> to be replaced with the kernel RNG via the "trusted_rng=kernel" boot command
> line option.
> - The proposed patch removes that possibility for no reason.
>
> Mimi & Elaine
I'm keeping this patch set in queue branch, possibly picking patches to
some other patch set or they are available for picking to other patch
sets.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Günther Noack @ 2026-03-03 21:19 UTC (permalink / raw)
To: Tingmao Wang
Cc: Yihan Ding, Justin Suess, Mickaël Salaün, Paul Moore,
Jann Horn, linux-security-module, linux-kernel,
syzbot+7ea2f5e9dfd468201817
In-Reply-To: <c482a8bb-d8c5-4008-9c8d-704d6a880022@maowtm.org>
On Tue, Mar 03, 2026 at 08:38:13PM +0000, Tingmao Wang wrote:
> On 3/3/26 19:50, Günther Noack wrote:
> > [...]
> > On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
> >> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> >>> [...]
> >>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> >>> index de01aa899751..xxxxxxxxxxxx 100644
> >>> --- a/security/landlock/tsync.c
> >>> +++ b/security/landlock/tsync.c
> >>> @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >>> shared_ctx.new_cred = new_cred;
> >>> shared_ctx.set_no_new_privs = task_no_new_privs(current);
> >>>
> >>> + /*
> >>> + * Serialize concurrent TSYNC operations to prevent deadlocks
> >>> + * when multiple threads call landlock_restrict_self() simultaneously.
> >>> + */
> >>> + if (!down_write_trylock(¤t->signal->exec_update_lock))
> >>> + return -ERESTARTNOINTR;
> >> These two lines above introduced a test failure in tsync_test
> >> completing_enablement.
> >>
> >> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
> >> (this patch) and is currently in the mic/next branch.
> >>
> >> I noticed the test failure while testing an unrelated patch.
> >>
> >> The bug is because this code never actually yields or restarts the syscall.
> >>
> >> This is the test output I observed:
> >>
> >> [+] Running tsync_test:
> >> TAP version 13
> >> 1..4
> >> # Starting 4 tests from 1 test cases.
> >> # RUN global.single_threaded_success ...
> >> # OK global.single_threaded_success
> >> ok 1 global.single_threaded_success
> >> # RUN global.multi_threaded_success ...
> >> # OK global.multi_threaded_success
> >> ok 2 global.multi_threaded_success
> >> # RUN global.multi_threaded_success_despite_diverging_domains ...
> >> # OK global.multi_threaded_success_despite_diverging_domains
> >> ok 3 global.multi_threaded_success_despite_diverging_domains
> >> # RUN global.competing_enablement ...
> >> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> >
> > The interesting part here is when you print out the errno that is
> > returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
> >
> > My understanding so far: Poking around in kernel/entry/common.c, it
> > seems that __exit_to_user_mode_loop() calls
> > arch_do_signal_or_restart() only when there is a pending signal
> > (_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL). So it was possible that the
> > system call returns with the (normally internal) error code
> > ERESTARTNOINTR, in the case where the trylock fails, but where current
> > has not received a signal from the other competing TSYNC thread yet.
> >
> > So with that in mind, would it work to do this?
> >
> > while (try-to-acquire-the-lock) {
> > if (current-has-task-works-pending)
> > return -ERESTARTNOINTR;
> >
> > cond_resched();
> > }
> >
> > Then we could avoid calling task_work_run() directly; (I find it
> > difficult to reason about the implications of calling taks_work_run()
> > directly, because these task works may make assumptions about the
> > context in which they are running.)
>
> I've not caught up with the full discussion so might be missing some context on why RESTARTNOINTR was used here,
> but wouldn't
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 950b63d23729..f695fe44e2f1 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -490,7 +490,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> * when multiple threads call landlock_restrict_self() simultaneously.
> */
> if (!down_write_trylock(¤t->signal->exec_update_lock))
> - return -ERESTARTNOINTR;
> + return restart_syscall();
>
> /*
> * We schedule a pseudo-signal task_work for each of the calling task's
>
> achieve what the original patch intended?
Thanks, that's an excellent point!
restart_syscall() (a) sets TIF_SIGPENDING and then (b) returns
-ERESTARTNOINTR. (a) was the part that we have been missing for the
restart to work (see discussion above). Together, (a) and (b) cause
__exit_to_user_mode_loop() to restart the syscall. Given that this is
offered in signal.h, this seems like a clean and more "official" way
to do this than using the task works APIs.
It also fixes the previously failing selftest (I tried).
Yihan, Justin: Does that seem reasonable to you as well?
–Günther
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Justin Suess @ 2026-03-03 21:08 UTC (permalink / raw)
To: Günther Noack
Cc: Yihan Ding, Mickaël Salaün, Paul Moore, Jann Horn,
linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260303.2e4c89f9fdfe@gnoack.org>
On Tue, Mar 03, 2026 at 08:50:50PM +0100, Günther Noack wrote:
> Oof, thanks for catching this, Justin! 🏆
>
> I was convinced I had tried the selftests for that variant,
> but apparently I had not. Mea culpa.
>
> On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
> > On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> > > syzbot found a deadlock in landlock_restrict_sibling_threads().
> > > When multiple threads concurrently call landlock_restrict_self() with
> > > sibling thread restriction enabled, they can deadlock by mutually
> > > queueing task_works on each other and then blocking in kernel space
> > > (waiting for the other to finish).
> > >
> > > Fix this by serializing the TSYNC operations within the same process
> > > using the exec_update_lock. This prevents concurrent invocations
> > > from deadlocking.
> > >
> > > We use down_write_trylock() and return -ERESTARTNOINTR if the lock
> > > cannot be acquired immediately. This ensures that if a thread fails
> > > to get the lock, it will return to userspace, allowing it to process
> > > any pending TSYNC task_works from the lock holder, and then
> > > transparently restart the syscall.
> > >
> > > Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> > > Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> > > Suggested-by: Günther Noack <gnoack3000@gmail.com>
> > > Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> > > ---
> > > Changes in v3:
> > > - Replaced down_write_killable() with down_write_trylock() and
> > > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> > > blocking the execution of task_works. (Caught by Günther Noack).
> > >
> > > Changes in v2:
> > > - Use down_write_killable() instead of down_write().
> > > - Split the interrupt path cleanup into a separate patch.
> > > ---
> > > security/landlock/tsync.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > > index de01aa899751..xxxxxxxxxxxx 100644
> > > --- a/security/landlock/tsync.c
> > > +++ b/security/landlock/tsync.c
> > > @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > > shared_ctx.new_cred = new_cred;
> > > shared_ctx.set_no_new_privs = task_no_new_privs(current);
> > >
> > > + /*
> > > + * Serialize concurrent TSYNC operations to prevent deadlocks
> > > + * when multiple threads call landlock_restrict_self() simultaneously.
> > > + */
> > > + if (!down_write_trylock(¤t->signal->exec_update_lock))
> > > + return -ERESTARTNOINTR;
> > These two lines above introduced a test failure in tsync_test
> > completing_enablement.
> >
> > The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
> > (this patch) and is currently in the mic/next branch.
> >
> > I noticed the test failure while testing an unrelated patch.
> >
> > The bug is because this code never actually yields or restarts the syscall.
> >
> > This is the test output I observed:
> >
> > [+] Running tsync_test:
> > TAP version 13
> > 1..4
> > # Starting 4 tests from 1 test cases.
> > # RUN global.single_threaded_success ...
> > # OK global.single_threaded_success
> > ok 1 global.single_threaded_success
> > # RUN global.multi_threaded_success ...
> > # OK global.multi_threaded_success
> > ok 2 global.multi_threaded_success
> > # RUN global.multi_threaded_success_despite_diverging_domains ...
> > # OK global.multi_threaded_success_despite_diverging_domains
> > ok 3 global.multi_threaded_success_despite_diverging_domains
> > # RUN global.competing_enablement ...
> > # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
>
> The interesting part here is when you print out the errno that is
> returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
>
> My understanding so far: Poking around in kernel/entry/common.c, it
> seems that __exit_to_user_mode_loop() calls
> arch_do_signal_or_restart() only when there is a pending signal
> (_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL). So it was possible that the
> system call returns with the (normally internal) error code
> ERESTARTNOINTR, in the case where the trylock fails, but where current
> has not received a signal from the other competing TSYNC thread yet.
>
> So with that in mind, would it work to do this?
>
> while (try-to-acquire-the-lock) {
> if (current-has-task-works-pending)
> return -ERESTARTNOINTR;
>
> cond_resched();
> }
Returning -ERESTARTNOINTR does *work* in my testing.
But really anything that would yield to the scheduler technically would!
But it smells funny to me to restart the whole syscall to wait for a
lock to become free.
Also the documentation for that code ERESTARTNOINTR is:
"System call was interrupted by a signal and will be restarted".
That clearly isn't happening here, we're not being interrupted by a
(literal) signal, we're really waiting on a semaphore.
And this lock is held by a task in this code, so why not run the pending
tasks with task_work_run so you can get out of the lock ASAP instead of
jumping through hoops back down to userspace and back?
>
> Then we could avoid calling task_work_run() directly; (I find it
> difficult to reason about the implications of calling taks_work_run()
> directly, because these task works may make assumptions about the
> context in which they are running.)
Hmm possibly? I really don't know because there aren't many examples or
docs to look at.
My thought process for picking task_work_run was like this:
1. We have to wait for a lock.
2. So how do we do something useful while waiting to avoid deadlock?
3. We can simply help the other task get through doing whatever needs to
be done to get done the lock (doing the jobs we made with task_work_add)
And the way I decided to do that was task_work_run after grepping around
for some examples in io_uring code.
>
> (Apologies for the somewhat draft-state source code; I have not had
> the time to test my theories yet; I'll fully accept it if I am wrong
> about it.)
>
> –Günther
>
>
> > # competing_enablement: Test failed
> > # FAIL global.competing_enablement
> > not ok 4 global.competing_enablement
> > # FAILED: 3 / 4 tests passed.
> >
> >
> > Brief investigation and the additions of these pr_warn lines:
> >
> >
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index 0d66a68677b7..84909232b220 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> > @@ -574,6 +574,9 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> > const int err = landlock_restrict_sibling_threads(
> > current_cred(), new_cred);
> > if (err) {
> > + pr_warn("landlock: restrict_self tsync err pid=%d tgid=%d err=%d flags=0x%x ruleset_fd=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current), err,
> > + flags, ruleset_fd);
> > abort_creds(new_cred);
> > return err;
> > }
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index 5afc5d639b8f..deb0f0b1f081 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -489,8 +489,11 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > * Serialize concurrent TSYNC operations to prevent deadlocks when multiple
> > * threads call landlock_restrict_self() simultaneously.
> > */
> > - if (!down_write_trylock(¤t->signal->exec_update_lock))
> > + if (!down_write_trylock(¤t->signal->exec_update_lock)) {
> > + pr_warn("landlock: tsync trylock busy pid=%d tgid=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current));
> > return -ERESTARTNOINTR;
> > + }
> >
> > /*
> > * We schedule a pseudo-signal task_work for each of the calling task's
> > @@ -602,6 +605,10 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >
> > tsync_works_release(&works);
> > up_write(¤t->signal->exec_update_lock);
> > + if (atomic_read(&shared_ctx.preparation_error))
> > + pr_warn("landlock: tsync preparation_error pid=%d tgid=%d err=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current),
> > + atomic_read(&shared_ctx.preparation_error));
> >
> > return atomic_read(&shared_ctx.preparation_error);
> > }
> >
> > Resulted in the following output:
> >
> > landlock: tsync trylock busy pid=1263 tgid=1261
> > landlock: landlock: restrict_self tsync err pid=1263 tgid=1261 err=-513 flags=0x8 ruleset_fd=6
> > # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> > # competing_enablement: Test failed
> > # FAIL global.competing_enablement
> > not ok 4 global.competing_enablement
> >
> > I have a fix that I will send as a patch.
> >
> > Kind Regards,
> > Justin Suess
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Tingmao Wang @ 2026-03-03 20:38 UTC (permalink / raw)
To: Yihan Ding, Günther Noack, Justin Suess,
Mickaël Salaün
Cc: Paul Moore, Jann Horn, linux-security-module, linux-kernel,
syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260303.2e4c89f9fdfe@gnoack.org>
On 3/3/26 19:50, Günther Noack wrote:
> [...]
> On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
>> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
>>> [...]
>>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
>>> index de01aa899751..xxxxxxxxxxxx 100644
>>> --- a/security/landlock/tsync.c
>>> +++ b/security/landlock/tsync.c
>>> @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>>> shared_ctx.new_cred = new_cred;
>>> shared_ctx.set_no_new_privs = task_no_new_privs(current);
>>>
>>> + /*
>>> + * Serialize concurrent TSYNC operations to prevent deadlocks
>>> + * when multiple threads call landlock_restrict_self() simultaneously.
>>> + */
>>> + if (!down_write_trylock(¤t->signal->exec_update_lock))
>>> + return -ERESTARTNOINTR;
>> These two lines above introduced a test failure in tsync_test
>> completing_enablement.
>>
>> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
>> (this patch) and is currently in the mic/next branch.
>>
>> I noticed the test failure while testing an unrelated patch.
>>
>> The bug is because this code never actually yields or restarts the syscall.
>>
>> This is the test output I observed:
>>
>> [+] Running tsync_test:
>> TAP version 13
>> 1..4
>> # Starting 4 tests from 1 test cases.
>> # RUN global.single_threaded_success ...
>> # OK global.single_threaded_success
>> ok 1 global.single_threaded_success
>> # RUN global.multi_threaded_success ...
>> # OK global.multi_threaded_success
>> ok 2 global.multi_threaded_success
>> # RUN global.multi_threaded_success_despite_diverging_domains ...
>> # OK global.multi_threaded_success_despite_diverging_domains
>> ok 3 global.multi_threaded_success_despite_diverging_domains
>> # RUN global.competing_enablement ...
>> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
>
> The interesting part here is when you print out the errno that is
> returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
>
> My understanding so far: Poking around in kernel/entry/common.c, it
> seems that __exit_to_user_mode_loop() calls
> arch_do_signal_or_restart() only when there is a pending signal
> (_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL). So it was possible that the
> system call returns with the (normally internal) error code
> ERESTARTNOINTR, in the case where the trylock fails, but where current
> has not received a signal from the other competing TSYNC thread yet.
>
> So with that in mind, would it work to do this?
>
> while (try-to-acquire-the-lock) {
> if (current-has-task-works-pending)
> return -ERESTARTNOINTR;
>
> cond_resched();
> }
>
> Then we could avoid calling task_work_run() directly; (I find it
> difficult to reason about the implications of calling taks_work_run()
> directly, because these task works may make assumptions about the
> context in which they are running.)
I've not caught up with the full discussion so might be missing some context on why RESTARTNOINTR was used here,
but wouldn't
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 950b63d23729..f695fe44e2f1 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -490,7 +490,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
* when multiple threads call landlock_restrict_self() simultaneously.
*/
if (!down_write_trylock(¤t->signal->exec_update_lock))
- return -ERESTARTNOINTR;
+ return restart_syscall();
/*
* We schedule a pseudo-signal task_work for each of the calling task's
achieve what the original patch intended?
Kind regards,
Tingmao
^ permalink raw reply related
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Günther Noack @ 2026-03-03 19:50 UTC (permalink / raw)
To: Justin Suess
Cc: Yihan Ding, Mickaël Salaün, Paul Moore, Jann Horn,
linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <aacKOr1wywSSOAVv@suesslenovo>
Oof, thanks for catching this, Justin! 🏆
I was convinced I had tried the selftests for that variant,
but apparently I had not. Mea culpa.
On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> > syzbot found a deadlock in landlock_restrict_sibling_threads().
> > When multiple threads concurrently call landlock_restrict_self() with
> > sibling thread restriction enabled, they can deadlock by mutually
> > queueing task_works on each other and then blocking in kernel space
> > (waiting for the other to finish).
> >
> > Fix this by serializing the TSYNC operations within the same process
> > using the exec_update_lock. This prevents concurrent invocations
> > from deadlocking.
> >
> > We use down_write_trylock() and return -ERESTARTNOINTR if the lock
> > cannot be acquired immediately. This ensures that if a thread fails
> > to get the lock, it will return to userspace, allowing it to process
> > any pending TSYNC task_works from the lock holder, and then
> > transparently restart the syscall.
> >
> > Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> > Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> > Suggested-by: Günther Noack <gnoack3000@gmail.com>
> > Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> > ---
> > Changes in v3:
> > - Replaced down_write_killable() with down_write_trylock() and
> > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> > blocking the execution of task_works. (Caught by Günther Noack).
> >
> > Changes in v2:
> > - Use down_write_killable() instead of down_write().
> > - Split the interrupt path cleanup into a separate patch.
> > ---
> > security/landlock/tsync.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index de01aa899751..xxxxxxxxxxxx 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > shared_ctx.new_cred = new_cred;
> > shared_ctx.set_no_new_privs = task_no_new_privs(current);
> >
> > + /*
> > + * Serialize concurrent TSYNC operations to prevent deadlocks
> > + * when multiple threads call landlock_restrict_self() simultaneously.
> > + */
> > + if (!down_write_trylock(¤t->signal->exec_update_lock))
> > + return -ERESTARTNOINTR;
> These two lines above introduced a test failure in tsync_test
> completing_enablement.
>
> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
> (this patch) and is currently in the mic/next branch.
>
> I noticed the test failure while testing an unrelated patch.
>
> The bug is because this code never actually yields or restarts the syscall.
>
> This is the test output I observed:
>
> [+] Running tsync_test:
> TAP version 13
> 1..4
> # Starting 4 tests from 1 test cases.
> # RUN global.single_threaded_success ...
> # OK global.single_threaded_success
> ok 1 global.single_threaded_success
> # RUN global.multi_threaded_success ...
> # OK global.multi_threaded_success
> ok 2 global.multi_threaded_success
> # RUN global.multi_threaded_success_despite_diverging_domains ...
> # OK global.multi_threaded_success_despite_diverging_domains
> ok 3 global.multi_threaded_success_despite_diverging_domains
> # RUN global.competing_enablement ...
> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
The interesting part here is when you print out the errno that is
returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
My understanding so far: Poking around in kernel/entry/common.c, it
seems that __exit_to_user_mode_loop() calls
arch_do_signal_or_restart() only when there is a pending signal
(_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL). So it was possible that the
system call returns with the (normally internal) error code
ERESTARTNOINTR, in the case where the trylock fails, but where current
has not received a signal from the other competing TSYNC thread yet.
So with that in mind, would it work to do this?
while (try-to-acquire-the-lock) {
if (current-has-task-works-pending)
return -ERESTARTNOINTR;
cond_resched();
}
Then we could avoid calling task_work_run() directly; (I find it
difficult to reason about the implications of calling taks_work_run()
directly, because these task works may make assumptions about the
context in which they are running.)
(Apologies for the somewhat draft-state source code; I have not had
the time to test my theories yet; I'll fully accept it if I am wrong
about it.)
–Günther
> # competing_enablement: Test failed
> # FAIL global.competing_enablement
> not ok 4 global.competing_enablement
> # FAILED: 3 / 4 tests passed.
>
>
> Brief investigation and the additions of these pr_warn lines:
>
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..84909232b220 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -574,6 +574,9 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> const int err = landlock_restrict_sibling_threads(
> current_cred(), new_cred);
> if (err) {
> + pr_warn("landlock: restrict_self tsync err pid=%d tgid=%d err=%d flags=0x%x ruleset_fd=%d\n",
> + task_pid_nr(current), task_tgid_nr(current), err,
> + flags, ruleset_fd);
> abort_creds(new_cred);
> return err;
> }
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 5afc5d639b8f..deb0f0b1f081 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -489,8 +489,11 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> * Serialize concurrent TSYNC operations to prevent deadlocks when multiple
> * threads call landlock_restrict_self() simultaneously.
> */
> - if (!down_write_trylock(¤t->signal->exec_update_lock))
> + if (!down_write_trylock(¤t->signal->exec_update_lock)) {
> + pr_warn("landlock: tsync trylock busy pid=%d tgid=%d\n",
> + task_pid_nr(current), task_tgid_nr(current));
> return -ERESTARTNOINTR;
> + }
>
> /*
> * We schedule a pseudo-signal task_work for each of the calling task's
> @@ -602,6 +605,10 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>
> tsync_works_release(&works);
> up_write(¤t->signal->exec_update_lock);
> + if (atomic_read(&shared_ctx.preparation_error))
> + pr_warn("landlock: tsync preparation_error pid=%d tgid=%d err=%d\n",
> + task_pid_nr(current), task_tgid_nr(current),
> + atomic_read(&shared_ctx.preparation_error));
>
> return atomic_read(&shared_ctx.preparation_error);
> }
>
> Resulted in the following output:
>
> landlock: tsync trylock busy pid=1263 tgid=1261
> landlock: landlock: restrict_self tsync err pid=1263 tgid=1261 err=-513 flags=0x8 ruleset_fd=6
> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> # competing_enablement: Test failed
> # FAIL global.competing_enablement
> not ok 4 global.competing_enablement
>
> I have a fix that I will send as a patch.
>
> Kind Regards,
> Justin Suess
^ permalink raw reply
* Re: [PATCH v3] landlock: Expand restrict flags example for ABI version 8
From: Mickaël Salaün @ 2026-03-03 18:01 UTC (permalink / raw)
To: Panagiotis "Ivory" Vasilopoulos
Cc: Günther Noack, Jonathan Corbet, Shuah Khan,
linux-security-module, linux-doc, linux-kernel, Dan Cojocaru
In-Reply-To: <20260228-landlock-docs-add-tsync-example-v3-1-140ab50f0524@n0toose.net>
On Sat, Feb 28, 2026 at 10:36:59PM +0100, Panagiotis "Ivory" Vasilopoulos wrote:
> Add LANDLOCK_RESTRICT_SELF_TSYNC to the backwards compatibility example
> for restrict flags. This introduces completeness, similar to that of
> the ruleset attributes example. However, as the new example can impact
> enforcement in certain cases, an appropriate warning is also included.
>
> Additionally, I modified the two comments of the example to make them
> more consistent with the ruleset attributes example's.
>
> Signed-off-by: Panagiotis 'Ivory' Vasilopoulos <git@n0toose.net>
> Co-developed-by: Dan Cojocaru <dan@dcdev.ro>
> Signed-off-by: Dan Cojocaru <dan@dcdev.ro>
> ---
> Changes in v3:
> - Add __attribute__((fallthrough)) like in earlier example.
> - Improve comment for LANDLOCK_RESTRICT_SELF_TSYNC (ABI < 8) example.
> - Add relevant warning for ABI < 8 example based on Günther's feedback.
> - Link to v2: https://lore.kernel.org/r/20260221-landlock-docs-add-tsync-example-v2-1-60990986bba5@n0toose.net
>
> Changes in v2:
> - Fix formatting error.
> - Link to v1: https://lore.kernel.org/r/20260221-landlock-docs-add-tsync-example-v1-1-f89383809eb4@n0toose.net
> ---
> Documentation/userspace-api/landlock.rst | 28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 13134bccdd39d78ddce3daf454f32dda162ce91b..b71ac7aa308260b8141e5d35248fb68cec6dcba9 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -196,13 +196,33 @@ similar backwards compatibility check is needed for the restrict flags
> (see sys_landlock_restrict_self() documentation for available flags):
>
> .. code-block:: c
> -
> - __u32 restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
> - if (abi < 7) {
> - /* Clear logging flags unsupported before ABI 7. */
> + __u32 restrict_flags =
> + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> + LANDLOCK_RESTRICT_SELF_TSYNC;
> + switch (abi) {
> + case 1 ... 6:
> + /* Clear logging flags unsupported for ABI < 7 */
> restrict_flags &= ~(LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
> + __attribute__((fallthrough));
> + case 7:
> + /* Removes multithreaded enforcement flag unsupported for ABI < 8 */
> + /*
> + * WARNING!
> + * Don't copy-paste this just yet! This example impacts enforcement
> + * and can potentially decrease protection if misused.
What could be the use case when that would be an issue? What would be
the consequence wrt just tampering with a sibling thread?
> + *
> + * Below ABI v8, a Landlock policy can only be enforced for the calling
> + * thread and its children. This behavior remains a default for ABI v8,
> + * but the flag ``LANDLOCK_RESTRICT_SELF_TSYNC`` can now be used to
> + * enforce policies across all threads of the calling process. If an
> + * application's Landlock integration was designed under the assumption
> + * that the flag is used (such as when children threads are responsible
> + * for enforcing and/or overriding policies of parents and siblings),
> + * removing said flag can decrease protection for older Linux versions.
In this case, the application *must* check the ABI version and exit with
an error if it is not supported. That's the same use case as programs
wishing to sandbox themselves at least with a specific set of
restrictions.
> + */
> + restrict_flags &= ~LANDLOCK_RESTRICT_SELF_TSYNC;
> }
>
> The next step is to restrict the current thread from gaining more privileges
>
> ---
> base-commit: ceb977bfe9e8715e6cd3a4785c7aab8ea5cd2b77
> change-id: 20260221-landlock-docs-add-tsync-example-e8fd5c64a366
>
> Best regards,
> --
> Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
>
>
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Justin Suess @ 2026-03-03 18:13 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Yihan Ding, Günther Noack, Paul Moore, Jann Horn,
linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260303.kuSho2nooFie@digikod.net>
On Tue, Mar 03, 2026 at 06:47:30PM +0100, Mickaël Salaün wrote:
> On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
> > On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> > > syzbot found a deadlock in landlock_restrict_sibling_threads().
> > > When multiple threads concurrently call landlock_restrict_self() with
> > > sibling thread restriction enabled, they can deadlock by mutually
> > > queueing task_works on each other and then blocking in kernel space
> > > (waiting for the other to finish).
> > >
> > > Fix this by serializing the TSYNC operations within the same process
> > > using the exec_update_lock. This prevents concurrent invocations
> > > from deadlocking.
> > >
> > > We use down_write_trylock() and return -ERESTARTNOINTR if the lock
> > > cannot be acquired immediately. This ensures that if a thread fails
> > > to get the lock, it will return to userspace, allowing it to process
> > > any pending TSYNC task_works from the lock holder, and then
> > > transparently restart the syscall.
> > >
> > > Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> > > Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> > > Suggested-by: Günther Noack <gnoack3000@gmail.com>
> > > Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> > > ---
> > > Changes in v3:
> > > - Replaced down_write_killable() with down_write_trylock() and
> > > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> > > blocking the execution of task_works. (Caught by Günther Noack).
> > >
> > > Changes in v2:
> > > - Use down_write_killable() instead of down_write().
> > > - Split the interrupt path cleanup into a separate patch.
> > > ---
> > > security/landlock/tsync.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > > index de01aa899751..xxxxxxxxxxxx 100644
> > > --- a/security/landlock/tsync.c
> > > +++ b/security/landlock/tsync.c
> > > @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > > shared_ctx.new_cred = new_cred;
> > > shared_ctx.set_no_new_privs = task_no_new_privs(current);
> > >
> > > + /*
> > > + * Serialize concurrent TSYNC operations to prevent deadlocks
> > > + * when multiple threads call landlock_restrict_self() simultaneously.
> > > + */
> > > + if (!down_write_trylock(¤t->signal->exec_update_lock))
> > > + return -ERESTARTNOINTR;
> > These two lines above introduced a test failure in tsync_test
> > completing_enablement.
> >
> > The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
> > (this patch) and is currently in the mic/next branch.
> >
> > I noticed the test failure while testing an unrelated patch.
> >
> > The bug is because this code never actually yields or restarts the syscall.
> >
> > This is the test output I observed:
> >
> > [+] Running tsync_test:
> > TAP version 13
> > 1..4
> > # Starting 4 tests from 1 test cases.
> > # RUN global.single_threaded_success ...
> > # OK global.single_threaded_success
> > ok 1 global.single_threaded_success
> > # RUN global.multi_threaded_success ...
> > # OK global.multi_threaded_success
> > ok 2 global.multi_threaded_success
> > # RUN global.multi_threaded_success_despite_diverging_domains ...
> > # OK global.multi_threaded_success_despite_diverging_domains
> > ok 3 global.multi_threaded_success_despite_diverging_domains
> > # RUN global.competing_enablement ...
> > # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> > # competing_enablement: Test failed
> > # FAIL global.competing_enablement
> > not ok 4 global.competing_enablement
> > # FAILED: 3 / 4 tests passed.
> >
> >
> > Brief investigation and the additions of these pr_warn lines:
> >
> >
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index 0d66a68677b7..84909232b220 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> > @@ -574,6 +574,9 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> > const int err = landlock_restrict_sibling_threads(
> > current_cred(), new_cred);
> > if (err) {
> > + pr_warn("landlock: restrict_self tsync err pid=%d tgid=%d err=%d flags=0x%x ruleset_fd=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current), err,
> > + flags, ruleset_fd);
> > abort_creds(new_cred);
> > return err;
> > }
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index 5afc5d639b8f..deb0f0b1f081 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -489,8 +489,11 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > * Serialize concurrent TSYNC operations to prevent deadlocks when multiple
> > * threads call landlock_restrict_self() simultaneously.
> > */
> > - if (!down_write_trylock(¤t->signal->exec_update_lock))
> > + if (!down_write_trylock(¤t->signal->exec_update_lock)) {
> > + pr_warn("landlock: tsync trylock busy pid=%d tgid=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current));
> > return -ERESTARTNOINTR;
> > + }
> >
> > /*
> > * We schedule a pseudo-signal task_work for each of the calling task's
> > @@ -602,6 +605,10 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> >
> > tsync_works_release(&works);
> > up_write(¤t->signal->exec_update_lock);
> > + if (atomic_read(&shared_ctx.preparation_error))
> > + pr_warn("landlock: tsync preparation_error pid=%d tgid=%d err=%d\n",
> > + task_pid_nr(current), task_tgid_nr(current),
> > + atomic_read(&shared_ctx.preparation_error));
> >
> > return atomic_read(&shared_ctx.preparation_error);
> > }
> >
> > Resulted in the following output:
> >
> > landlock: tsync trylock busy pid=1263 tgid=1261
> > landlock: landlock: restrict_self tsync err pid=1263 tgid=1261 err=-513 flags=0x8 ruleset_fd=6
> > # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> > # competing_enablement: Test failed
> > # FAIL global.competing_enablement
> > not ok 4 global.competing_enablement
>
> You're right, I have the same issue, not sure how I missed it last time.
>
> >
> > I have a fix that I will send as a patch.
>
> I'll need to squash your fix to this fix to only have one non-buggy
> patch. So, either you send a new patch and I'll squash it with
> Co-developed-by, or Yihan takes your patch and send a new version with
> your contribution (I'll prefer the later to make it easier to follow
> this series).
>
Agreed.
The latter option is probably better. Yihan, are you ok to squash / apply
my patch [1] yourself to your next version of this series?
Feel free to do whatever you think is best. (or submit your own fix if
you think mine isn't a good fit)
[1]: https://lore.kernel.org/linux-security-module/20260303174354.1839461-1-utilityemal77@gmail.com/
> >
> > Kind Regards,
> > Justin Suess
> >
^ permalink raw reply
* Re: [PATCH v1] landlock: Fix formatting
From: Günther Noack @ 2026-03-03 18:09 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Kees Cook
In-Reply-To: <20260303173632.88040-1-mic@digikod.net>
On Tue, Mar 03, 2026 at 06:36:31PM +0100, Mickaël Salaün wrote:
> Auto-format with clang-format -i security/landlock/*.[ch]
>
> Cc: Günther Noack <gnoack@google.com>
> Cc: Kees Cook <kees@kernel.org>
> Fixes: 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/domain.c | 3 +--
> security/landlock/ruleset.c | 9 ++++-----
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index f5b78d4766cd..f0d83f43afa1 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -94,8 +94,7 @@ static struct landlock_details *get_current_details(void)
> * allocate with GFP_KERNEL_ACCOUNT because it is independent from the
> * caller.
> */
> - details =
> - kzalloc_flex(*details, exe_path, path_size);
> + details = kzalloc_flex(*details, exe_path, path_size);
> if (!details)
> return ERR_PTR(-ENOMEM);
>
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 319873586385..73018dc8d6c7 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -32,9 +32,8 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
> {
> struct landlock_ruleset *new_ruleset;
>
> - new_ruleset =
> - kzalloc_flex(*new_ruleset, access_masks, num_layers,
> - GFP_KERNEL_ACCOUNT);
> + new_ruleset = kzalloc_flex(*new_ruleset, access_masks, num_layers,
> + GFP_KERNEL_ACCOUNT);
> if (!new_ruleset)
> return ERR_PTR(-ENOMEM);
> refcount_set(&new_ruleset->usage, 1);
> @@ -559,8 +558,8 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
> if (IS_ERR(new_dom))
> return new_dom;
>
> - new_dom->hierarchy = kzalloc_obj(*new_dom->hierarchy,
> - GFP_KERNEL_ACCOUNT);
> + new_dom->hierarchy =
> + kzalloc_obj(*new_dom->hierarchy, GFP_KERNEL_ACCOUNT);
> if (!new_dom->hierarchy)
> return ERR_PTR(-ENOMEM);
>
> --
> 2.53.0
>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
Thanks!
^ permalink raw reply
* Re: [PATCH v2 105/110] security: replace PRIino with %llu/%llx format strings
From: Casey Schaufler @ 2026-03-03 18:06 UTC (permalink / raw)
To: Jeff Layton, Alexander Viro, Christian Brauner, Jan Kara,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Dan Williams,
Matthew Wilcox, Eric Biggers, Theodore Y. Ts'o, Muchun Song,
Oscar Salvador, David Hildenbrand, David Howells, Paulo Alcantara,
Andreas Dilger, Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust,
Anna Schumaker, Chuck Lever, NeilBrown, Olga Kornievskaia,
Dai Ngo, Tom Talpey, Steve French, Ronnie Sahlberg,
Shyam Prasad N, Bharath SM, Alexander Aring, Ryusuke Konishi,
Viacheslav Dubeyko, Eric Van Hensbergen, Latchesar Ionkov,
Dominique Martinet, Christian Schoenebeck, David Sterba,
Marc Dionne, Ian Kent, Luis de Bethencourt, Salah Triki,
Tigran A. Aivazian, Ilya Dryomov, Alex Markuze, Jan Harkes, coda,
Nicolas Pitre, Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
Cc: linux-fsdevel, linux-kernel, linux-trace-kernel, nvdimm, fsverity,
linux-mm, netfs, linux-ext4, linux-f2fs-devel, linux-nfs,
linux-cifs, samba-technical, linux-nilfs, v9fs, linux-afs, autofs,
ceph-devel, codalist, ecryptfs, linux-mtd, jfs-discussion, ntfs3,
ocfs2-devel, devel, linux-unionfs, apparmor,
linux-security-module, linux-integrity, selinux, amd-gfx,
dri-devel, linux-media, linaro-mm-sig, netdev, linux-perf-users,
linux-fscrypt, linux-xfs, linux-hams, linux-x25, audit,
linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260302-iino-u64-v2-105-e5388800dae0@kernel.org>
On 3/2/2026 12:25 PM, Jeff Layton wrote:
> Now that i_ino is u64 and the PRIino format macro has been removed,
> replace all uses in security with the concrete format strings.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
For the security/smack changes:
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/apparmor/apparmorfs.c | 4 ++--
> security/integrity/integrity_audit.c | 2 +-
> security/ipe/audit.c | 2 +-
> security/lsm_audit.c | 10 +++++-----
> security/selinux/hooks.c | 10 +++++-----
> security/smack/smack_lsm.c | 12 ++++++------
> 6 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index be343479f80b71566be6fda90fc4e00912faad63..7b645f40e71c956f216fa6a7d69c3ecd4e2a5ff4 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -149,7 +149,7 @@ static int aafs_count;
>
> static int aafs_show_path(struct seq_file *seq, struct dentry *dentry)
> {
> - seq_printf(seq, "%s:[%" PRIino "u]", AAFS_NAME, d_inode(dentry)->i_ino);
> + seq_printf(seq, "%s:[%llu]", AAFS_NAME, d_inode(dentry)->i_ino);
> return 0;
> }
>
> @@ -2644,7 +2644,7 @@ static int policy_readlink(struct dentry *dentry, char __user *buffer,
> char name[32];
> int res;
>
> - res = snprintf(name, sizeof(name), "%s:[%" PRIino "u]", AAFS_NAME,
> + res = snprintf(name, sizeof(name), "%s:[%llu]", AAFS_NAME,
> d_inode(dentry)->i_ino);
> if (res > 0 && res < sizeof(name))
> res = readlink_copy(buffer, buflen, name, strlen(name));
> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
> index d28dac23a4e7cf651856b80ab7756d250187ccde..d8d9e5ff1cd22b091f462d1e83d28d2d6bd983e9 100644
> --- a/security/integrity/integrity_audit.c
> +++ b/security/integrity/integrity_audit.c
> @@ -62,7 +62,7 @@ void integrity_audit_message(int audit_msgno, struct inode *inode,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> }
> audit_log_format(ab, " res=%d errno=%d", !result, errno);
> audit_log_end(ab);
> diff --git a/security/ipe/audit.c b/security/ipe/audit.c
> index 0de95dd4fbea15d4d913fc42e197c3120a9d24a0..93fb59fbddd60b56c0b22be2a38b809ef9e18b76 100644
> --- a/security/ipe/audit.c
> +++ b/security/ipe/audit.c
> @@ -153,7 +153,7 @@ void ipe_audit_match(const struct ipe_eval_ctx *const ctx,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> } else {
> audit_log_format(ab, " dev=? ino=?");
> }
> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
> index 523f2ee116f0f928003aec30a105d6d4ecb49b0b..737f5a263a8f79416133315edf363ece3d79c722 100644
> --- a/security/lsm_audit.c
> +++ b/security/lsm_audit.c
> @@ -202,7 +202,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> }
> break;
> }
> @@ -215,7 +215,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> }
> break;
> }
> @@ -228,7 +228,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> }
>
> audit_log_format(ab, " ioctlcmd=0x%hx", a->u.op->cmd);
> @@ -246,7 +246,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
> if (inode) {
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> }
> break;
> }
> @@ -265,7 +265,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
> }
> audit_log_format(ab, " dev=");
> audit_log_untrustedstring(ab, inode->i_sb->s_id);
> - audit_log_format(ab, " ino=%" PRIino "u", inode->i_ino);
> + audit_log_format(ab, " ino=%llu", inode->i_ino);
> rcu_read_unlock();
> break;
> }
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9430f44c81447708c67ddc35c5b4254f16731b8f..8f38de4d223ea59cfea6bbe73747d7b228e0c33f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1400,7 +1400,7 @@ static int inode_doinit_use_xattr(struct inode *inode, struct dentry *dentry,
> if (rc < 0) {
> kfree(context);
> if (rc != -ENODATA) {
> - pr_warn("SELinux: %s: getxattr returned %d for dev=%s ino=%" PRIino "u\n",
> + pr_warn("SELinux: %s: getxattr returned %d for dev=%s ino=%llu\n",
> __func__, -rc, inode->i_sb->s_id, inode->i_ino);
> return rc;
> }
> @@ -1412,13 +1412,13 @@ static int inode_doinit_use_xattr(struct inode *inode, struct dentry *dentry,
> def_sid, GFP_NOFS);
> if (rc) {
> char *dev = inode->i_sb->s_id;
> - kino_t ino = inode->i_ino;
> + u64 ino = inode->i_ino;
>
> if (rc == -EINVAL) {
> - pr_notice_ratelimited("SELinux: inode=%" PRIino "u on dev=%s was found to have an invalid context=%s. This indicates you may need to relabel the inode or the filesystem in question.\n",
> + pr_notice_ratelimited("SELinux: inode=%llu on dev=%s was found to have an invalid context=%s. This indicates you may need to relabel the inode or the filesystem in question.\n",
> ino, dev, context);
> } else {
> - pr_warn("SELinux: %s: context_to_sid(%s) returned %d for dev=%s ino=%" PRIino "u\n",
> + pr_warn("SELinux: %s: context_to_sid(%s) returned %d for dev=%s ino=%llu\n",
> __func__, context, -rc, dev, ino);
> }
> }
> @@ -3477,7 +3477,7 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name,
> &newsid);
> if (rc) {
> pr_err("SELinux: unable to map context to SID"
> - "for (%s, %" PRIino "u), rc=%d\n",
> + "for (%s, %llu), rc=%d\n",
> inode->i_sb->s_id, inode->i_ino, -rc);
> return;
> }
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 22b6bd322840c82697c38c07b19a4677e7da2598..2eb3368a3632b836df54ba8628c16f7215ddf3ea 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -182,7 +182,7 @@ static int smk_bu_inode(struct inode *inode, int mode, int rc)
> char acc[SMK_NUM_ACCESS_TYPE + 1];
>
> if (isp->smk_flags & SMK_INODE_IMPURE)
> - pr_info("Smack Unconfined Corruption: inode=(%s %" PRIino "u) %s\n",
> + pr_info("Smack Unconfined Corruption: inode=(%s %llu) %s\n",
> inode->i_sb->s_id, inode->i_ino, current->comm);
>
> if (rc <= 0)
> @@ -195,7 +195,7 @@ static int smk_bu_inode(struct inode *inode, int mode, int rc)
>
> smk_bu_mode(mode, acc);
>
> - pr_info("Smack %s: (%s %s %s) inode=(%s %" PRIino "u) %s\n", smk_bu_mess[rc],
> + pr_info("Smack %s: (%s %s %s) inode=(%s %llu) %s\n", smk_bu_mess[rc],
> tsp->smk_task->smk_known, isp->smk_inode->smk_known, acc,
> inode->i_sb->s_id, inode->i_ino, current->comm);
> return 0;
> @@ -214,7 +214,7 @@ static int smk_bu_file(struct file *file, int mode, int rc)
> char acc[SMK_NUM_ACCESS_TYPE + 1];
>
> if (isp->smk_flags & SMK_INODE_IMPURE)
> - pr_info("Smack Unconfined Corruption: inode=(%s %" PRIino "u) %s\n",
> + pr_info("Smack Unconfined Corruption: inode=(%s %llu) %s\n",
> inode->i_sb->s_id, inode->i_ino, current->comm);
>
> if (rc <= 0)
> @@ -223,7 +223,7 @@ static int smk_bu_file(struct file *file, int mode, int rc)
> rc = 0;
>
> smk_bu_mode(mode, acc);
> - pr_info("Smack %s: (%s %s %s) file=(%s %" PRIino "u %pD) %s\n", smk_bu_mess[rc],
> + pr_info("Smack %s: (%s %s %s) file=(%s %llu %pD) %s\n", smk_bu_mess[rc],
> sskp->smk_known, smk_of_inode(inode)->smk_known, acc,
> inode->i_sb->s_id, inode->i_ino, file,
> current->comm);
> @@ -244,7 +244,7 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
> char acc[SMK_NUM_ACCESS_TYPE + 1];
>
> if (isp->smk_flags & SMK_INODE_IMPURE)
> - pr_info("Smack Unconfined Corruption: inode=(%s %" PRIino "u) %s\n",
> + pr_info("Smack Unconfined Corruption: inode=(%s %llu) %s\n",
> inode->i_sb->s_id, inode->i_ino, current->comm);
>
> if (rc <= 0)
> @@ -253,7 +253,7 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file,
> rc = 0;
>
> smk_bu_mode(mode, acc);
> - pr_info("Smack %s: (%s %s %s) file=(%s %" PRIino "u %pD) %s\n", smk_bu_mess[rc],
> + pr_info("Smack %s: (%s %s %s) file=(%s %llu %pD) %s\n", smk_bu_mess[rc],
> sskp->smk_known, smk_of_inode(inode)->smk_known, acc,
> inode->i_sb->s_id, inode->i_ino, file,
> current->comm);
>
^ permalink raw reply
* Re: [PATCH v3 0/2] landlock: Fix TSYNC deadlock and clean up error path
From: Mickaël Salaün @ 2026-03-03 17:31 UTC (permalink / raw)
To: Yihan Ding
Cc: Günther Noack, Paul Moore, Jann Horn, linux-security-module,
linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260226015903.3158620-1-dingyihan@uniontech.com>
Thanks! It's been in -next since last week and I'll send a PR with it
soon.
On Thu, Feb 26, 2026 at 09:59:01AM +0800, Yihan Ding wrote:
> Hello,
>
> This patch series fixes a deadlock in the Landlock TSYNC multithreading
> support, originally reported by syzbot, and cleans up the associated
> interrupt recovery path.
>
> The deadlock occurs when multiple threads concurrently call
> landlock_restrict_self() with sibling thread restriction enabled,
> causing them to mutually queue task_works on each other and block
> indefinitely.
>
> * Patch 1 fixes the root cause by serializing the TSYNC operations
> within the same process using the exec_update_lock.
> * Patch 2 cleans up the interrupt recovery path by replacing an
> unnecessary wait_for_completion() with a straightforward loop break,
> avoiding Use-After-Free while unblocking remaining task_works.
>
> Changes in v3:
> - Patch 1: Changed down_write_killable() to down_write_trylock() and
> return -ERESTARTNOINTR on failure. This avoids a secondary deadlock
> where a blocking wait prevents a sibling thread from waking up to
> execute the requested TSYNC task_work. (Noted by Günther Noack.
> down_write_interruptible() was also suggested but is not implemented
> for rw_semaphores in the kernel).
> - Patch 2: No changes.
>
> Changes in v2:
> - Split the changes into a 2-patch series.
> - Patch 1: Adopted down_write_killable() instead of down_write().
> - Patch 2: Removed wait_for_completion(&shared_ctx.all_prepared) and
> replaced it with a `break` to prevent UAF.
>
> Link to v2: https://lore.kernel.org/all/20260225024734.3024732-1-dingyihan@uniontech.com/
> Link to v1: https://lore.kernel.org/all/20260224062729.2908692-1-dingyihan@uniontech.com/
>
> Yihan Ding (2):
> landlock: Serialize TSYNC thread restriction
> landlock: Clean up interrupted thread logic in TSYNC
>
> security/landlock/tsync.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
> --
> 2.51.0
^ permalink raw reply
* [PATCH v1] landlock: Fix formatting
From: Mickaël Salaün @ 2026-03-03 17:36 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Kees Cook
Auto-format with clang-format -i security/landlock/*.[ch]
Cc: Günther Noack <gnoack@google.com>
Cc: Kees Cook <kees@kernel.org>
Fixes: 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/domain.c | 3 +--
security/landlock/ruleset.c | 9 ++++-----
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index f5b78d4766cd..f0d83f43afa1 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -94,8 +94,7 @@ static struct landlock_details *get_current_details(void)
* allocate with GFP_KERNEL_ACCOUNT because it is independent from the
* caller.
*/
- details =
- kzalloc_flex(*details, exe_path, path_size);
+ details = kzalloc_flex(*details, exe_path, path_size);
if (!details)
return ERR_PTR(-ENOMEM);
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 319873586385..73018dc8d6c7 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -32,9 +32,8 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
{
struct landlock_ruleset *new_ruleset;
- new_ruleset =
- kzalloc_flex(*new_ruleset, access_masks, num_layers,
- GFP_KERNEL_ACCOUNT);
+ new_ruleset = kzalloc_flex(*new_ruleset, access_masks, num_layers,
+ GFP_KERNEL_ACCOUNT);
if (!new_ruleset)
return ERR_PTR(-ENOMEM);
refcount_set(&new_ruleset->usage, 1);
@@ -559,8 +558,8 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
if (IS_ERR(new_dom))
return new_dom;
- new_dom->hierarchy = kzalloc_obj(*new_dom->hierarchy,
- GFP_KERNEL_ACCOUNT);
+ new_dom->hierarchy =
+ kzalloc_obj(*new_dom->hierarchy, GFP_KERNEL_ACCOUNT);
if (!new_dom->hierarchy)
return ERR_PTR(-ENOMEM);
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Mickaël Salaün @ 2026-03-03 17:47 UTC (permalink / raw)
To: Justin Suess
Cc: Yihan Ding, Günther Noack, Paul Moore, Jann Horn,
linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <aacKOr1wywSSOAVv@suesslenovo>
On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> > syzbot found a deadlock in landlock_restrict_sibling_threads().
> > When multiple threads concurrently call landlock_restrict_self() with
> > sibling thread restriction enabled, they can deadlock by mutually
> > queueing task_works on each other and then blocking in kernel space
> > (waiting for the other to finish).
> >
> > Fix this by serializing the TSYNC operations within the same process
> > using the exec_update_lock. This prevents concurrent invocations
> > from deadlocking.
> >
> > We use down_write_trylock() and return -ERESTARTNOINTR if the lock
> > cannot be acquired immediately. This ensures that if a thread fails
> > to get the lock, it will return to userspace, allowing it to process
> > any pending TSYNC task_works from the lock holder, and then
> > transparently restart the syscall.
> >
> > Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> > Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> > Suggested-by: Günther Noack <gnoack3000@gmail.com>
> > Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> > ---
> > Changes in v3:
> > - Replaced down_write_killable() with down_write_trylock() and
> > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> > blocking the execution of task_works. (Caught by Günther Noack).
> >
> > Changes in v2:
> > - Use down_write_killable() instead of down_write().
> > - Split the interrupt path cleanup into a separate patch.
> > ---
> > security/landlock/tsync.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index de01aa899751..xxxxxxxxxxxx 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> > shared_ctx.new_cred = new_cred;
> > shared_ctx.set_no_new_privs = task_no_new_privs(current);
> >
> > + /*
> > + * Serialize concurrent TSYNC operations to prevent deadlocks
> > + * when multiple threads call landlock_restrict_self() simultaneously.
> > + */
> > + if (!down_write_trylock(¤t->signal->exec_update_lock))
> > + return -ERESTARTNOINTR;
> These two lines above introduced a test failure in tsync_test
> completing_enablement.
>
> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
> (this patch) and is currently in the mic/next branch.
>
> I noticed the test failure while testing an unrelated patch.
>
> The bug is because this code never actually yields or restarts the syscall.
>
> This is the test output I observed:
>
> [+] Running tsync_test:
> TAP version 13
> 1..4
> # Starting 4 tests from 1 test cases.
> # RUN global.single_threaded_success ...
> # OK global.single_threaded_success
> ok 1 global.single_threaded_success
> # RUN global.multi_threaded_success ...
> # OK global.multi_threaded_success
> ok 2 global.multi_threaded_success
> # RUN global.multi_threaded_success_despite_diverging_domains ...
> # OK global.multi_threaded_success_despite_diverging_domains
> ok 3 global.multi_threaded_success_despite_diverging_domains
> # RUN global.competing_enablement ...
> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> # competing_enablement: Test failed
> # FAIL global.competing_enablement
> not ok 4 global.competing_enablement
> # FAILED: 3 / 4 tests passed.
>
>
> Brief investigation and the additions of these pr_warn lines:
>
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..84909232b220 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -574,6 +574,9 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> const int err = landlock_restrict_sibling_threads(
> current_cred(), new_cred);
> if (err) {
> + pr_warn("landlock: restrict_self tsync err pid=%d tgid=%d err=%d flags=0x%x ruleset_fd=%d\n",
> + task_pid_nr(current), task_tgid_nr(current), err,
> + flags, ruleset_fd);
> abort_creds(new_cred);
> return err;
> }
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 5afc5d639b8f..deb0f0b1f081 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -489,8 +489,11 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> * Serialize concurrent TSYNC operations to prevent deadlocks when multiple
> * threads call landlock_restrict_self() simultaneously.
> */
> - if (!down_write_trylock(¤t->signal->exec_update_lock))
> + if (!down_write_trylock(¤t->signal->exec_update_lock)) {
> + pr_warn("landlock: tsync trylock busy pid=%d tgid=%d\n",
> + task_pid_nr(current), task_tgid_nr(current));
> return -ERESTARTNOINTR;
> + }
>
> /*
> * We schedule a pseudo-signal task_work for each of the calling task's
> @@ -602,6 +605,10 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
>
> tsync_works_release(&works);
> up_write(¤t->signal->exec_update_lock);
> + if (atomic_read(&shared_ctx.preparation_error))
> + pr_warn("landlock: tsync preparation_error pid=%d tgid=%d err=%d\n",
> + task_pid_nr(current), task_tgid_nr(current),
> + atomic_read(&shared_ctx.preparation_error));
>
> return atomic_read(&shared_ctx.preparation_error);
> }
>
> Resulted in the following output:
>
> landlock: tsync trylock busy pid=1263 tgid=1261
> landlock: landlock: restrict_self tsync err pid=1263 tgid=1261 err=-513 flags=0x8 ruleset_fd=6
> # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> # competing_enablement: Test failed
> # FAIL global.competing_enablement
> not ok 4 global.competing_enablement
You're right, I have the same issue, not sure how I missed it last time.
>
> I have a fix that I will send as a patch.
I'll need to squash your fix to this fix to only have one non-buggy
patch. So, either you send a new patch and I'll squash it with
Co-developed-by, or Yihan takes your patch and send a new version with
your contribution (I'll prefer the later to make it easier to follow
this series).
>
> Kind Regards,
> Justin Suess
>
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Mickaël Salaün @ 2026-03-03 17:51 UTC (permalink / raw)
To: Yihan Ding
Cc: Günther Noack, Paul Moore, Jann Horn, linux-security-module,
linux-kernel, syzbot+7ea2f5e9dfd468201817
In-Reply-To: <20260226015903.3158620-2-dingyihan@uniontech.com>
On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
> syzbot found a deadlock in landlock_restrict_sibling_threads().
> When multiple threads concurrently call landlock_restrict_self() with
> sibling thread restriction enabled, they can deadlock by mutually
> queueing task_works on each other and then blocking in kernel space
> (waiting for the other to finish).
>
> Fix this by serializing the TSYNC operations within the same process
> using the exec_update_lock. This prevents concurrent invocations
> from deadlocking.
>
> We use down_write_trylock() and return -ERESTARTNOINTR if the lock
> cannot be acquired immediately. This ensures that if a thread fails
> to get the lock, it will return to userspace, allowing it to process
> any pending TSYNC task_works from the lock holder, and then
> transparently restart the syscall.
>
> Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> Suggested-by: Günther Noack <gnoack3000@gmail.com>
> Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> ---
> Changes in v3:
> - Replaced down_write_killable() with down_write_trylock() and
> returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> blocking the execution of task_works. (Caught by Günther Noack).
>
> Changes in v2:
> - Use down_write_killable() instead of down_write().
> - Split the interrupt path cleanup into a separate patch.
> ---
> security/landlock/tsync.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index de01aa899751..xxxxxxxxxxxx 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -447,6 +447,13 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> shared_ctx.new_cred = new_cred;
> shared_ctx.set_no_new_privs = task_no_new_privs(current);
>
> + /*
> + * Serialize concurrent TSYNC operations to prevent deadlocks
> + * when multiple threads call landlock_restrict_self() simultaneously.
Please format this comment and the next patch ones to fit (and fill) in
80 columns.
> + */
> + if (!down_write_trylock(¤t->signal->exec_update_lock))
> + return -ERESTARTNOINTR;
> +
> /*
> * We schedule a pseudo-signal task_work for each of the calling task's
> * sibling threads. In the task work, each thread:
> @@ -556,6 +563,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> wait_for_completion(&shared_ctx.all_finished);
>
> tsync_works_release(&works);
> + up_write(¤t->signal->exec_update_lock);
>
> return atomic_read(&shared_ctx.preparation_error);
> }
> --
> 2.51.0
>
^ permalink raw reply
* [PATCH] landlock: Yield if unable to acquire exec_update_lock
From: Justin Suess @ 2026-03-03 17:43 UTC (permalink / raw)
To: linux-security-module, Yihan Ding
Cc: Mickaël Salaün, Günther Noack, Tingmao Wang,
Justin Suess, Günther Noack
Instead of returning -ERESTARTNOINTR when there is no pending
signal, run the pending tasks in the current thread and yield
until the exec_update_lock can be acquired.
This allows other tasks to run and allows the lock contention
to resolve in the kernel's task scheduler.
Cc: Yihan Ding <dingyihan@uniontech.com>
Cc: Günther Noack <gnoack3000@gmail.com>
Fixes: 3d6327c306b3 ("landlock: Serialize TSYNC thread restriction")
Closes: https://lore.kernel.org/linux-security-module/aacKOr1wywSSOAVv@suesslenovo/
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Notes:
This fixes the failure in tsync_test.competing_enablement:
landlock: tsync trylock busy pid=1263 tgid=1261
landlock: landlock: restrict_self tsync err pid=1263 tgid=1261 err=-513 flags=0x8 ruleset_fd=6
# tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
# competing_enablement: Test failed
# FAIL global.competing_enablement
not ok 4 global.competing_enablement
The test passes after applying this patch.
security/landlock/tsync.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 956a64cb6945..83938849620f 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -487,10 +487,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
/*
* Serialize concurrent TSYNC operations to prevent deadlocks when multiple
- * threads call landlock_restrict_self() simultaneously.
+ * threads call landlock_restrict_self() simultaneously. If we are unable to
+ * acquire the lock, we yield nicely and retry.
*/
- if (!down_write_trylock(¤t->signal->exec_update_lock))
- return -ERESTARTNOINTR;
+ while (!down_write_trylock(¤t->signal->exec_update_lock)) {
+ if (task_work_pending(current))
+ task_work_run();
+ cond_resched();
+ }
/*
* We schedule a pseudo-signal task_work for each of the calling task's
base-commit: 8ff74a72b8af3672beca7f6b6b72557a9db94382
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
From: Matthew Wilcox @ 2026-03-03 17:18 UTC (permalink / raw)
To: Jeff Layton
Cc: Christoph Hellwig, Darrick J. Wong, Theodore Tso, Alexander Viro,
Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Dan Williams, Eric Biggers, Muchun Song,
Oscar Salvador, David Hildenbrand, David Howells, Paulo Alcantara,
Andreas Dilger, Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust,
Anna Schumaker, Chuck Lever, NeilBrown, Olga Kornievskaia,
Dai Ngo, Tom Talpey, Steve French, Ronnie Sahlberg,
Shyam Prasad N, Bharath SM, Alexander Aring, Ryusuke Konishi,
Viacheslav Dubeyko, Eric Van Hensbergen, Latchesar Ionkov,
Dominique Martinet, Christian Schoenebeck, David Sterba,
Marc Dionne, Ian Kent, Luis de Bethencourt, Salah Triki,
Tigran A. Aivazian, Ilya Dryomov, Alex Markuze, Jan Harkes, coda,
Nicolas Pitre, Tyler Hicks, Amir Goldstein,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Martin Schiller,
Eric Paris, Joerg Reuter, Marcel Holtmann, Johan Hedberg,
Luiz Augusto von Dentz, Oliver Hartkopp, Marc Kleine-Budde,
David Ahern, Neal Cardwell, Steffen Klassert, Herbert Xu,
Remi Denis-Courmont, Marcelo Ricardo Leitner, Xin Long,
Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <1310fc5c09cce52ec00344b936275fe584c88dea.camel@kernel.org>
On Tue, Mar 03, 2026 at 09:19:42AM -0500, Jeff Layton wrote:
> There are really only three options here:
>
> 1/ Do (almost) all of the changes in one giant patch
>
> 2/ Accept that the build may break during the interim stages
>
> 3/ This series: using a typedef and macro to work around the breakage
> until the type can be changed, at the expense of some extra churn in
> the codebase
4/ Don't do anything, drop the patch series (I'm not in favour of this,
but it is an option)
5/ Do the conversion(s) _once_ per filesystem. Here's one way to do
it:
- unsigned long i_ino;
+ union {
+ u64 i_ino64;
+ struct {
+#if defined(CONFIG_64BIT)
+ unsigned long i_ino;
+#elif defined(CONFIG_CPU_BIG_ENDIAN)
+ unsigned long i_ino;
+ unsigned long i_ino_pad;
+#else
+ unsigned long i_ino_pad;
+ unsigned long i_ino;
+#endif
+ };
+ };
[...]
#define i_ino(inode) (inode)->i_ino64
So that's patch one. All plain references to i_ino access the lower
bits of i_ino64, so everything will continue to work as it does today.
Once you've got the VFS core in shape, you can convert filesystems one
at a time to use i_ino(inode). Once you're done you can delete the
scaffolding from the core and go back to calling i_ino64 just i_ino.
You could delete the i_ino() uses from filesystems at that point, but
why bother?
I'm sure there are other ways to do it, this is just the one I came up
with. But for the love of god stop spamming hundreds of people on the
cc of this patchset. In fact, take me off for next time -- I get each
one of these fucking patches four times.
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2026-03-03 16:46 UTC (permalink / raw)
To: Stephen Smalley
Cc: Ondrej Mosnacek, linux-security-module, selinux, John Johansen
In-Reply-To: <CAEjxPJ4urh7mUbDJEi-DbdiAifMM_uDH3m35tLeTdx6z+qhPyg@mail.gmail.com>
On Tue, Mar 3, 2026 at 8:30 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Wed, Feb 25, 2026 at 7:05 PM Paul Moore <paul@paul-moore.com> wrote:
> > I spent a few hours this afternoon re-reading this thread and tweaking
> > the original proposal to address everything discussed. The revised
> > proposal is below, with a bit more detail than before, please take a
> > look and let us all know what you think ...
[Yet another reminder to please consider trimming your replies.]
> I think my only caveat here is that your proposal is quite a bit more
> complex than what I implemented here:
> [1] https://lore.kernel.org/selinux/20251003190959.3288-2-stephen.smalley.work@gmail.com/
> [2] https://lore.kernel.org/selinux/20251003191328.3605-1-stephen.smalley.work@gmail.com/
> and I'm not sure the extra complexity is worth it.
>
> In particular:
> 1. Immediately unsharing the namespace upon lsm_set_self_attr() allows
> the caller to immediately and unambiguously know if the operation is
> supported and allowed ...
Performing the unshare operation immediately looks much less like a
LSM attribute and more like its own syscall. That isn't a problem in
my eyes, it just means if this is the direction we want to go we
should implement a lsm_unshare(2) API, or something similar.
> 3. We don't need to introduce a new CLONE_* flag at all or introduce
> new or changed LSM hooks to clone/unshare,
While I think we could get away with a new lsm_unshare(2) syscall, I
have zero interest in an lsm_clone(2) syscall. If we do away with the
namespace related LSM attributes and rely entirely on a lsm_unshare(2)
syscall, would everyone be okay with that?
(I think we would still want/need the procfs API)
> All that said, I'm willing to wire up the SELinux namespaces
> implementation to the proposed interface if someone implements the
> necessary plumbing, but I'm not sure it's better.
I'd really like to hear from some of the other LSMs before we start
diving into the code. It may sound funny, but from my perspective
doing the work to get the API definition "right" is far more important
than implementing it.
--
paul-moore.com
^ 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