* Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
From: Theodore Tso @ 2026-03-03 15:16 UTC (permalink / raw)
To: Jeff Layton
Cc: Darrick J. Wong, Alexander Viro, Christian Brauner, Jan Kara,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Dan Williams,
Matthew Wilcox, 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, 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, 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: <33228005140684201de2ca0c157441d3b6a06413.camel@kernel.org>
On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote:
>
> Like I said to Ted, this is just temporary scaffolding for the change.
> The PRIino macro is removed in the end. Given that, perhaps you can
> overlook the bikeshed's color in this instance?
I didn't realize that this was going to disappear in the end. That
makes me feel much better about the change. I'd suggest changing the
commit description where it claims that we're using something that
follows the inttypes.h convention and making it clear that this is
temporary and only to preserve bisectability.
One question though --- are there *really* places that are using
signed inode numbers and trying to print them? If people are trying
to use negative inodes to signal an error or some such, the it implies
that at least for some file systems, an inode number larger than 2**63
might be problematic. If there is core VFS code that uses a negative
inode number then this could be a real potential trap.
So are there really code which is doing a printf of 'PRIino "d"'? Or
was this to allow the use of of 'PRiino "x"'?
- Ted
^ permalink raw reply
* Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
From: Christoph Hellwig @ 2026-03-03 15:23 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, Matthew Wilcox, 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: <4d3b9b92da613ad329b822f3f6043fa08f534451.camel@kernel.org>
On Tue, Mar 03, 2026 at 10:14:27AM -0500, Jeff Layton wrote:
> I think we'll be looking at close to a 1000 line patch that touches
> nearly 200 files if go that route. Roughly:
>
> 182 files changed, 910 insertions(+), 912 deletions(-)
That's not actually a lot, especially for a patch that should be
scriped and mechnanical.
I also really don't understand the backport argument. It's not like
you could backport any of the split out patches individually anyway.
^ permalink raw reply
* Re: [PATCH v2 001/110] vfs: introduce kino_t typedef and PRIino format macro
From: Jeff Layton @ 2026-03-03 15:25 UTC (permalink / raw)
To: Theodore Tso
Cc: Darrick J. Wong, Alexander Viro, Christian Brauner, Jan Kara,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Dan Williams,
Matthew Wilcox, 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, 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, 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: <20260303151617.GD6520@macsyma-wired.lan>
On Tue, 2026-03-03 at 10:16 -0500, Theodore Tso wrote:
> On Tue, Mar 03, 2026 at 05:53:39AM -0500, Jeff Layton wrote:
> >
> > Like I said to Ted, this is just temporary scaffolding for the change.
> > The PRIino macro is removed in the end. Given that, perhaps you can
> > overlook the bikeshed's color in this instance?
>
> I didn't realize that this was going to disappear in the end. That
> makes me feel much better about the change. I'd suggest changing the
> commit description where it claims that we're using something that
> follows the inttypes.h convention and making it clear that this is
> temporary and only to preserve bisectability.
>
> One question though --- are there *really* places that are using
> signed inode numbers and trying to print them? If people are trying
> to use negative inodes to signal an error or some such, the it implies
> that at least for some file systems, an inode number larger than 2**63
> might be problematic. If there is core VFS code that uses a negative
> inode number then this could be a real potential trap.
>
> So are there really code which is doing a printf of 'PRIino "d"'? Or
> was this to allow the use of of 'PRiino "x"'?
>
Mostly it's to allow 'PRIino "x"'. There are a number of places that
(for whatever reason) print the inode number in hex. I don't want to
change those.
There are also some places that print it as a signed value (PRIino
"d"). I suspect most of those are bugs, or just holdovers from a
simpler time when we didn't worry so much about the signedness of inode
numbers. I fixed a few of those in the context of this series, fwiw.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 003/110] audit: widen ino fields to u64
From: Paul Moore @ 2026-03-03 16:03 UTC (permalink / raw)
To: Jeff Layton
Cc: 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, 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: <7a0165fe39e82a1effd8cce5c2c4e82d6a42cb3a.camel@kernel.org>
On Tue, Mar 3, 2026 at 6:05 AM Jeff Layton <jlayton@kernel.org> wrote:
> On Mon, 2026-03-02 at 18:44 -0500, Paul Moore wrote:
> > On Mon, Mar 2, 2026 at 3:25 PM Jeff Layton <jlayton@kernel.org> wrote:
> > >
> > > inode->i_ino is being widened from unsigned long to u64. The audit
> > > subsystem uses unsigned long ino in struct fields, function parameters,
> > > and local variables that store inode numbers from arbitrary filesystems.
> > > On 32-bit platforms this truncates inode numbers that exceed 32 bits,
> > > which will cause incorrect audit log entries and broken watch/mark
> > > comparisons.
> > >
> > > Widen all audit ino fields, parameters, and locals to u64, and update
> > > the inode format string from %lu to %llu to match.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > > include/linux/audit.h | 2 +-
> > > kernel/audit.h | 9 ++++-----
> > > kernel/audit_fsnotify.c | 4 ++--
> > > kernel/audit_watch.c | 8 ++++----
> > > kernel/auditsc.c | 2 +-
> > > 5 files changed, 12 insertions(+), 13 deletions(-)
> >
> > We should also update audit_hash_ino() in kernel/audit.h. It is a
> > *very* basic hash function, so I think leaving the function as-is and
> > just changing the inode parameter from u32 to u64 should be fine.
...
> It doesn't look like changing the argument type will make any material
> difference. Given that it should still work without that change, can we
> leave this cleanup for you to do in a follow-on patchset?
I would prefer if you made the change as part of the patch, mainly to
keep a patch record of this being related.
Ideally I'd really like to see kino_t used in the audit code instead
of u64, but perhaps that is done in a later patch that I didn't see.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 003/110] audit: widen ino fields to u64
From: Jeff Layton @ 2026-03-03 16:12 UTC (permalink / raw)
To: Paul Moore
Cc: 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, 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: <CAHC9VhTyhnG7-ojnTnVdh_m1x=rKxw9YEH9g7Xp9m4F78aA5cA@mail.gmail.com>
On Tue, 2026-03-03 at 11:03 -0500, Paul Moore wrote:
> On Tue, Mar 3, 2026 at 6:05 AM Jeff Layton <jlayton@kernel.org> wrote:
> > On Mon, 2026-03-02 at 18:44 -0500, Paul Moore wrote:
> > > On Mon, Mar 2, 2026 at 3:25 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > >
> > > > inode->i_ino is being widened from unsigned long to u64. The audit
> > > > subsystem uses unsigned long ino in struct fields, function parameters,
> > > > and local variables that store inode numbers from arbitrary filesystems.
> > > > On 32-bit platforms this truncates inode numbers that exceed 32 bits,
> > > > which will cause incorrect audit log entries and broken watch/mark
> > > > comparisons.
> > > >
> > > > Widen all audit ino fields, parameters, and locals to u64, and update
> > > > the inode format string from %lu to %llu to match.
> > > >
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > > include/linux/audit.h | 2 +-
> > > > kernel/audit.h | 9 ++++-----
> > > > kernel/audit_fsnotify.c | 4 ++--
> > > > kernel/audit_watch.c | 8 ++++----
> > > > kernel/auditsc.c | 2 +-
> > > > 5 files changed, 12 insertions(+), 13 deletions(-)
> > >
> > > We should also update audit_hash_ino() in kernel/audit.h. It is a
> > > *very* basic hash function, so I think leaving the function as-is and
> > > just changing the inode parameter from u32 to u64 should be fine.
>
> ...
>
> > It doesn't look like changing the argument type will make any material
> > difference. Given that it should still work without that change, can we
> > leave this cleanup for you to do in a follow-on patchset?
>
> I would prefer if you made the change as part of the patch, mainly to
> keep a patch record of this being related.
>
Ok, I'll see about factoring that in.
> Ideally I'd really like to see kino_t used in the audit code instead
> of u64, but perhaps that is done in a later patch that I didn't see.
I think I didn't make this clear enough in the cover letter, but kino_t
is removed at the end of the series. It's just there to support the
change during the interim.
If HCH gets his way to do the changes as one big patch, it'll go away
entirely.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 003/110] audit: widen ino fields to u64
From: Paul Moore @ 2026-03-03 16:17 UTC (permalink / raw)
To: Jeff Layton
Cc: 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, 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: <add39953250a4a1b2fe2b09deb3373c2a7482b88.camel@kernel.org>
On Tue, Mar 3, 2026 at 11:12 AM Jeff Layton <jlayton@kernel.org> wrote:
> On Tue, 2026-03-03 at 11:03 -0500, Paul Moore wrote:
> > On Tue, Mar 3, 2026 at 6:05 AM Jeff Layton <jlayton@kernel.org> wrote:
> > > On Mon, 2026-03-02 at 18:44 -0500, Paul Moore wrote:
> > > > On Mon, Mar 2, 2026 at 3:25 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > >
> > > > > inode->i_ino is being widened from unsigned long to u64. The audit
> > > > > subsystem uses unsigned long ino in struct fields, function parameters,
> > > > > and local variables that store inode numbers from arbitrary filesystems.
> > > > > On 32-bit platforms this truncates inode numbers that exceed 32 bits,
> > > > > which will cause incorrect audit log entries and broken watch/mark
> > > > > comparisons.
> > > > >
> > > > > Widen all audit ino fields, parameters, and locals to u64, and update
> > > > > the inode format string from %lu to %llu to match.
> > > > >
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > > include/linux/audit.h | 2 +-
> > > > > kernel/audit.h | 9 ++++-----
> > > > > kernel/audit_fsnotify.c | 4 ++--
> > > > > kernel/audit_watch.c | 8 ++++----
> > > > > kernel/auditsc.c | 2 +-
> > > > > 5 files changed, 12 insertions(+), 13 deletions(-)
> > > >
> > > > We should also update audit_hash_ino() in kernel/audit.h. It is a
> > > > *very* basic hash function, so I think leaving the function as-is and
> > > > just changing the inode parameter from u32 to u64 should be fine.
> >
> > ...
> >
> > > It doesn't look like changing the argument type will make any material
> > > difference. Given that it should still work without that change, can we
> > > leave this cleanup for you to do in a follow-on patchset?
> >
> > I would prefer if you made the change as part of the patch, mainly to
> > keep a patch record of this being related.
>
> Ok, I'll see about factoring that in.
Thanks.
> > Ideally I'd really like to see kino_t used in the audit code instead
> > of u64, but perhaps that is done in a later patch that I didn't see.
>
> I think I didn't make this clear enough in the cover letter, but kino_t
> is removed at the end of the series. It's just there to support the
> change during the interim.
Ah, gotcha, thanks for the education :)
> If HCH gets his way to do the changes as one big patch, it'll go away
> entirely.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 1/2] landlock: Serialize TSYNC thread restriction
From: Justin Suess @ 2026-03-03 16:20 UTC (permalink / raw)
To: Yihan Ding
Cc: Mickaël Salaün, 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.
> + */
> + 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
I have a fix that I will send as a patch.
Kind Regards,
Justin Suess
^ permalink raw reply related
* 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
* 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
* [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 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
* 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
* [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 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
* 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 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 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 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: 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 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: 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: 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 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 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 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
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