* Re: [PATCH RESEND 09/12] mm: make vm_area_desc utilise vma_flags_t only
From: Lorenzo Stoakes @ 2026-01-20 16:46 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module
In-Reply-To: <20260120152245.GC1134360@nvidia.com>
On Tue, Jan 20, 2026 at 11:22:45AM -0400, Jason Gunthorpe wrote:
> On Tue, Jan 20, 2026 at 03:10:54PM +0000, Lorenzo Stoakes wrote:
> > The natural implication of what you're saying is that we can no longer use this
> > from _anywhere_ because - hey - passing this by value is bad so now _everything_
> > has to be re-written as:
>
> No, I'm not saying that, I'm saying this specific case where you are
> making an accessor to reach an unknown value located on the heap
OK it would have been helpful for you to say that! Sometimes reviews feel
like a ratcheting series of 'what do you actually mean?'s... :)
> should be using a pointer as both a matter of style and to simplify
> life for the compiler.
OK fine.
>
> > vma_flags_t flags_to_set = mk_vma_flags(<flags>);
> >
> > if (vma_flags_test(&flags, &flags_to_set)) { ... }
>
> This is quite a different situation, it is a known const at compile
> time value located on the stack.
Well as a const time thing it'll be optimised to just a value assuming
nothing changes flags_to_set in the mean time. You'd hope.
Note that we have xxx_mask() variants, such that you can do, e.g.:
vma_flags_t flags1 = mk_vma_flags(...);
vma_flags_t flags2 = mk_vma_flags(...);
if (vma_flags_test_mask(flags1, flags2)) {
...
}
ASIDE ->
NOTE: A likely use of this, and one I already added is so we can do
e.g.:
#define VMA_REMAP_FLAGS mk_vma_flags(VMA_IO_BIT, VMA_PFNMAP_BIT, \
VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)
...
if (vma_flagss_test_mask(flags, VMA_REMAP_FLAGS)) { ... }
Which would be effectively a const input anyway.
<- ASIDE
Or in a world where flags1 is a const pointer now:
if (vma_flags_test_mask(&flags1, flags2)) { ... }
Which makes the form... kinda weird. Then again it's consistent with other
forms which update flags1, ofc we name this separately, e.g. flags, to_test
or flags, to_set so I guess not such a problme.
Now, nobody is _likely_ to do e.g.:
if (vma_flags_test_mask(&vma1->flags, vma2->flags)) { ... }
In this situation, but they could.
However perhaps having one value pass-by-const-pointer and the other
by-value essentially documents the fact you're being dumb.
And if somebody really needs something like this (not sure why) we could
add something.
But yeah ok, I'll change this. It's more than this case it's also all the
test stuff but shouldn't be a really huge change.
>
> > If it was just changing this one function I'd still object as it makes it differ
> > from _every other test predicate_ using vma_flags_t but maybe to humour you I'd
> > change it, but surely by this argument you're essentially objecting to the whole
> > series?
>
> I only think that if you are taking a heap input that is not of known
> value you should continue to pass by pointer as is generally expected
> in the C style we use.
Ack.
>
> And it isn't saying anything about the overall technique in the
> series, just a minor note about style.
OK good, though Arnd's reply feels more like a comment on the latter,
though only really doing pass-by-value for const values (in nearly all sane
cases) should hopefully mitigate.
>
> > I am not sure about this 'idiomatic kernel style' thing either, it feels rather
> > conjured. Yes you wouldn't ordinarily pass something larger than a register size
> > by-value, but here the intent is for it to be inlined anyway right?
>
> Well, exactly, we don't normally pass things larger than an interger
> by value, that isn't the style, so I don't think it is such a great
> thing to introduce here kind of unnecessarily.
>
> The troubles I recently had were linked to odd things like gcov and
> very old still supported versions of gcc. Also I saw a power compiler
> make a very strange choice to not inline something that evaluated to a
> constant.
Right ok.
>
> Jason
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH RESEND 09/12] mm: make vm_area_desc utilise vma_flags_t only
From: Lorenzo Stoakes @ 2026-01-20 16:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jason Gunthorpe, Andrew Morton, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Greg Kroah-Hartman, Dan Williams, Vishal Verma,
Dave Jiang, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Dave Airlie, Simona Vetter, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Christian König, Huang Rui,
Matthew Auld, Matthew Brost, Alexander Viro, Christian Brauner,
Jan Kara, Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador,
David Hildenbrand (Red Hat), Konstantin Komarov, Mike Marshall,
Martin Brandenburg, Tony Luck, Reinette Chatre, Dave Martin,
James Morse, Babu Moger, Carlos Maiolino, Damien Le Moal,
Naohiro Aota, Johannes Thumshirn, Matthew Wilcox, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Hugh Dickins, Baolin Wang, Zi Yan, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Jann Horn, Pedro Falcato,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Yury Norov, Rasmus Villemoes, linux-sgx, linux-kernel, nvdimm,
linux-cxl, dri-devel, intel-gfx, linux-fsdevel, linux-aio,
linux-erofs, linux-ext4, linux-mm, ntfs3, devel, linux-xfs,
keyrings, linux-security-module
In-Reply-To: <9ff58468-a72d-4984-95f4-d0a60554705d@app.fastmail.com>
On Tue, Jan 20, 2026 at 05:44:29PM +0100, Arnd Bergmann wrote:
> On Tue, Jan 20, 2026, at 17:22, Lorenzo Stoakes wrote:
> > On Tue, Jan 20, 2026 at 05:00:28PM +0100, Arnd Bergmann wrote:
> >> On Tue, Jan 20, 2026, at 16:10, Lorenzo Stoakes wrote:
> >> >
> >> > It strikes me that the key optimisation here is the inlining, now if the issue
> >> > is that ye olde compiler might choose not to inline very small functions (seems
> >> > unlikely) we could always throw in an __always_inline?
> >>
> >> I can think of three specific things going wrong with structures passed
> >> by value:
> >
> > I mean now you seem to be talking about it _in general_ which, _in theory_,
> > kills the whole concept of bitmap VMA flags _altogether_ really, or at
> > least any workable version of them.
>
> No, what I'm saying is "understand what the pitfalls are", not
> "don't do it". I think that is what Jason was also getting at.
>
> Arnd
Ack sure and your input is appreciated :) It's important to kick the tyres
and be aware of possible issues.
Actually I think now I understand where Jason's coming from - the by-value
cases will be const value for the most part - which should make life MUCH
easier for the compiler and avoid a lot of the issues you raised.
So _hopefully_ we're mitigated. Again as I said, in cases where we might
not be, I will take action to figure out workarounds.
I'm excited by the proposed approach in general (+ again thanks to Jason to
opening my eyes to the possibility in the first place), so perhaps a
_little_ defensive, as it allows for a like-for-like replacement generally
which should HUGELY speed up + simplify the transition :)
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
From: Bart Van Assche @ 2026-01-20 18:24 UTC (permalink / raw)
To: Marco Elver, Peter Zijlstra, Ingo Molnar
Cc: Thomas Gleixner, Will Deacon, Boqun Feng, Waiman Long,
Christoph Hellwig, Steven Rostedt, kasan-dev, llvm, linux-crypto,
linux-doc, linux-security-module, linux-kernel
In-Reply-To: <20260119094029.1344361-1-elver@google.com>
On 1/19/26 1:05 AM, Marco Elver wrote:
> This series proposes a solution to this by introducing scoped init
> guards which Peter suggested, using the guard(type_init)(&lock) or
> scoped_guard(type_init, ..) interface.
Although I haven't had the time yet to do an in-depth review, from a
quick look all patches in this series look good to me.
Thanks,
Bart.
^ permalink raw reply
* [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-20 20:20 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
introduced a means to track change detection for an inode
via ctime updates, opposed to setting kstat.change_cookie to
an i_version when calling into xfs_vn_getattr().
This introduced a regression for IMA such that an action
performed on a LOWER inode on a stacked file systems always
requires a re-evaluation if the LOWER file system does not
leverage kstat.change_cookie to track inode i_version or lacks
i_version support all together.
In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
will require re-evaluation. Stacking TPMFS on XFS for instance, once the
inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
leverages generic_fillattr() to update the change cookie.
This is because IMA caches kstat.change_cookie to compare against an
inode's i_version directly in integrity_inode_attrs_changed(), and thus
could be out of date depending on how file systems set
kstat.change_cookie.
To address this, require integrity_inode_attrs_changed() to query
vfs_getattr_nosec() to compare the cached version against
kstat.change_cookie directly. This ensures that when updates occur,
we're accessing the same changed inode version on changes, and fallback
to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
result mask.
Lastly, because EVM still relies on querying and caching a inode's
i_version directly, the integrity_inode_attrs_changed() falls back to the
original inode.i_version != cached comparison.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
caching may be behind as well as file systems that don't support/export
i_version. Thus we're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime guard when
that's not updated.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Below is a simplified test that demonstrates the issue:
_fragment.config_
CONFIG_XFS_FS=y
CONFIG_OVERLAY_FS=y
CONFIG_IMA=y
CONFIG_IMA_WRITE_POLICY=y
CONFIG_IMA_READ_POLICY=y
_./test.sh_
IMA_POLICY="/sys/kernel/security/ima/policy"
TEST_BIN="/bin/date"
MNT_BASE="/tmp/ima_test_root"
mkdir -p "$MNT_BASE"
mount -t tmpfs tmpfs "$MNT_BASE"
mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
mkfs.xfs -q "$MNT_BASE/xfs.img"
mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
mount -t overlay overlay -o \
"lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
"$MNT_BASE/ovl"
echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
target_prog="$MNT_BASE/ovl/test_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
if [[ "$audit_count" -eq 1 ]]; then
echo "PASS: Found exactly 1 audit event."
else
echo "FAIL: Expected 1 audit event, but found $audit_count."
exit 1
fi
---
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
Changes in v2:
- Updated commit description + message to clarify the problem.
- compare struct timespec64 to avoid collision possibility [Roberto].
- Don't check inode_attr_changed() in ima_check_last_writer()
- Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
---
include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
security/integrity/evm/evm_crypto.c | 4 +++-
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 20 +++++++++++++------
security/integrity/ima/ima_main.c | 18 ++++++++++-------
5 files changed, 65 insertions(+), 22 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -9,6 +9,8 @@
#include <linux/fs.h>
#include <linux/iversion.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
/* An inode's attributes for detection of changes */
struct integrity_inode_attributes {
+ struct timespec64 ctime;
u64 version; /* track inode changes */
unsigned long ino;
dev_t dev;
@@ -42,8 +45,10 @@ struct integrity_inode_attributes {
*/
static inline void
integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
- u64 i_version, const struct inode *inode)
+ u64 i_version, struct timespec64 ctime,
+ const struct inode *inode)
{
+ attrs->ctime = ctime;
attrs->version = i_version;
attrs->dev = inode->i_sb->s_dev;
attrs->ino = inode->i_ino;
@@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
/*
* On stacked filesystems detect whether the inode or its content has changed.
+ *
+ * Must be called in process context.
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ might_sleep();
+
+ if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
+ return true;
+
+ /*
+ * EVM currently relies on backing inode i_version. While IS_I_VERSION
+ * is not a good indicator of i_version support, this still retains
+ * the logic such that a re-evaluation should still occur for EVM, and
+ * only for IMA if vfs_getattr_nosec() fails.
+ */
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return !IS_I_VERSION(inode) ||
+ !inode_eq_iversion(inode, attrs->version);
+
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ return stat.change_cookie != attrs->version;
+
+ if (stat.result_mask & STATX_CTIME)
+ return !timespec64_equal(&stat.ctime, &attrs->ctime);
+
+ return true;
}
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
hmac_add_misc(desc, inode, type, data->digest);
if (inode != d_backing_inode(dentry) && iint) {
+ struct timespec64 ctime = {0};
+
if (IS_I_VERSION(inode))
i_version = inode_query_iversion(inode);
integrity_inode_attrs_store(&iint->metadata_inode, i_version,
- inode);
+ ctime, inode);
}
/* Portable EVM signatures must include an IMA hash */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
bool ret = false;
if (iint) {
- ret = (!IS_I_VERSION(metadata_inode) ||
- integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ ret = integrity_inode_attrs_changed(&iint->metadata_inode,
+ NULL, metadata_inode);
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
int length;
void *tmpbuf;
u64 i_version = 0;
+ struct timespec64 ctime = {0};
/*
* Always collect the modsig, because IMA might have already collected
@@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ if (stat.result_mask & STATX_CTIME)
+ ctime = stat.ctime;
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
@@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
iint->ima_hash = tmpbuf;
memcpy(iint->ima_hash, &hash, length);
- if (real_inode == inode)
+ if (real_inode == inode) {
iint->real_inode.version = i_version;
- else
+ iint->real_inode.ctime = ctime;
+ } else {
integrity_inode_attrs_store(&iint->real_inode, i_version,
- real_inode);
+ ctime, real_inode);
+ }
/* Possibly temporary failure due to type of read (eg. O_DIRECT) */
if (!result)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,12 +22,14 @@
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
+#include <linux/stat.h>
#include <linux/xattr.h>
#include <linux/ima.h>
#include <linux/fs.h>
#include <linux/iversion.h>
#include <linux/evm.h>
#include <linux/crash_dump.h>
+#include <linux/time64.h>
#include "ima.h"
@@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT) ||
+ ((stat.result_mask & STATX_CHANGE_COOKIE) ?
+ stat.change_cookie != iint->real_inode.version :
+ (!(stat.result_mask & STATX_CTIME) ||
+ !timespec64_equal(&stat.ctime,
+ &iint->real_inode.ctime)))) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
real_inode = d_real_inode(file_dentry(file));
if (real_inode != inode &&
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
- if (!IS_I_VERSION(real_inode) ||
- integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ if (integrity_inode_attrs_changed(&iint->real_inode,
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Jeff Layton @ 2026-01-20 21:22 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260120-xfs-ima-fixup-v2-1-f332ead8b043@cloudflare.com>
On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> introduced a means to track change detection for an inode
> via ctime updates, opposed to setting kstat.change_cookie to
> an i_version when calling into xfs_vn_getattr().
>
> This introduced a regression for IMA such that an action
> performed on a LOWER inode on a stacked file systems always
> requires a re-evaluation if the LOWER file system does not
> leverage kstat.change_cookie to track inode i_version or lacks
> i_version support all together.
>
> In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> leverages generic_fillattr() to update the change cookie.
>
> This is because IMA caches kstat.change_cookie to compare against an
> inode's i_version directly in integrity_inode_attrs_changed(), and thus
> could be out of date depending on how file systems set
> kstat.change_cookie.
>
> To address this, require integrity_inode_attrs_changed() to query
> vfs_getattr_nosec() to compare the cached version against
> kstat.change_cookie directly. This ensures that when updates occur,
> we're accessing the same changed inode version on changes, and fallback
> to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> result mask.
>
> Lastly, because EVM still relies on querying and caching a inode's
> i_version directly, the integrity_inode_attrs_changed() falls back to the
> original inode.i_version != cached comparison.
>
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> using multigrain ctime (as well as other file systems) for
> change detection in commit 1cf7e834a6fb ("xfs: switch to
> multigrain timestamps").
>
> Because file systems may implement i_version as they see fit, IMA
> caching may be behind as well as file systems that don't support/export
> i_version. Thus we're proposing to compare against the kstat.change_cookie
> directly to the cached version, and fall back to a ctime guard when
> that's not updated.
>
> EVM is largely left alone since there's no trivial way to query a file
> directly in the LSM call paths to obtain kstat.change_cookie &
> kstat.ctime to cache. Thus retains accessing i_version directly.
>
> Regression tests will be added to the Linux Test Project instead of
> selftest to help catch future file system changes that may impact
> future evaluation of IMA.
>
> I'd like this to be backported to at least 6.18 if possible.
>
> Below is a simplified test that demonstrates the issue:
>
> _fragment.config_
> CONFIG_XFS_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_IMA=y
> CONFIG_IMA_WRITE_POLICY=y
> CONFIG_IMA_READ_POLICY=y
>
> _./test.sh_
>
> IMA_POLICY="/sys/kernel/security/ima/policy"
> TEST_BIN="/bin/date"
> MNT_BASE="/tmp/ima_test_root"
>
> mkdir -p "$MNT_BASE"
> mount -t tmpfs tmpfs "$MNT_BASE"
> mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
>
> dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> mkfs.xfs -q "$MNT_BASE/xfs.img"
> mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
>
> mount -t overlay overlay -o \
> "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> "$MNT_BASE/ovl"
>
> echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
>
> target_prog="$MNT_BASE/ovl/test_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
> setpriv --reuid nobody "$target_prog"
>
> audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
>
> if [[ "$audit_count" -eq 1 ]]; then
> echo "PASS: Found exactly 1 audit event."
> else
> echo "FAIL: Expected 1 audit event, but found $audit_count."
> exit 1
> fi
> ---
> Changes since RFC:
> - Remove calls to I_IS_VERSION()
> - Function documentation/comments
> - Abide IMA/EVM change detection fallback invariants
> - Combined ctime guard into version for attributes struct
> - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> ---
> Changes in v2:
> - Updated commit description + message to clarify the problem.
> - compare struct timespec64 to avoid collision possibility [Roberto].
> - Don't check inode_attr_changed() in ima_check_last_writer()
> - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> ---
> include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> security/integrity/evm/evm_crypto.c | 4 +++-
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_api.c | 20 +++++++++++++------
> security/integrity/ima/ima_main.c | 18 ++++++++++-------
> 5 files changed, 65 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,8 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
>
> /* An inode's attributes for detection of changes */
> struct integrity_inode_attributes {
> + struct timespec64 ctime;
> u64 version; /* track inode changes */
> unsigned long ino;
> dev_t dev;
> @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> */
> static inline void
> integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> - u64 i_version, const struct inode *inode)
> + u64 i_version, struct timespec64 ctime,
> + const struct inode *inode)
> {
> + attrs->ctime = ctime;
> attrs->version = i_version;
> attrs->dev = inode->i_sb->s_dev;
> attrs->ino = inode->i_ino;
> @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
>
> /*
> * On stacked filesystems detect whether the inode or its content has changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
Seems like a reasonable way to handle getattr failure, given the
limitations on EVM.
> +
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + return stat.change_cookie != attrs->version;
> +
> + if (stat.result_mask & STATX_CTIME)
> + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> +
> + return true;
> }
>
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> hmac_add_misc(desc, inode, type, data->digest);
>
> if (inode != d_backing_inode(dentry) && iint) {
> + struct timespec64 ctime = {0};
> +
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
It would be nice if you could change this codepath as well, but that
looks like a much bigger project. FWIW, I took a quick look:
The problematic codepath is get/setxattr, which can call into this but
currently only receives a dentry and not a struct path. That would
probably have to be changed. I think it's doable but it's a fair bit of
yak shaving.
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> - inode);
> + ctime, inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> int length;
> void *tmpbuf;
> u64 i_version = 0;
> + struct timespec64 ctime = {0};
>
> /*
> * Always collect the modsig, because IMA might have already collected
> @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + if (stat.result_mask & STATX_CTIME)
> + ctime = stat.ctime;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>
> iint->ima_hash = tmpbuf;
> memcpy(iint->ima_hash, &hash, length);
> - if (real_inode == inode)
> + if (real_inode == inode) {
> iint->real_inode.version = i_version;
> - else
> + iint->real_inode.ctime = ctime;
> + } else {
> integrity_inode_attrs_store(&iint->real_inode, i_version,
> - real_inode);
> + ctime, real_inode);
> + }
>
> /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> if (!result)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -22,12 +22,14 @@
> #include <linux/mount.h>
> #include <linux/mman.h>
> #include <linux/slab.h>
> +#include <linux/stat.h>
> #include <linux/xattr.h>
> #include <linux/ima.h>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include <linux/time64.h>
>
> #include "ima.h"
>
> @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> - AT_STATX_SYNC_AS_STAT) ||
> - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> - stat.change_cookie != iint->real_inode.version) {
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> + AT_STATX_SYNC_AS_STAT) ||
> + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> + stat.change_cookie != iint->real_inode.version :
> + (!(stat.result_mask & STATX_CTIME) ||
> + !timespec64_equal(&stat.ctime,
> + &iint->real_inode.ctime)))) {
> iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> iint->measured_pcrs = 0;
> if (update)
> @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251212-xfs-ima-fixup-931780a62c2c
>
> Best regards,
Looks like a good change overall. Thanks for fixing this up. I don't
know IMA or EVM very well, so FWIW:
Acked-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Frederick Lawler @ 2026-01-20 21:49 UTC (permalink / raw)
To: Jeff Layton
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <1a7985c825cf4af3b2a87e89793ae0daf2bb9c49.camel@kernel.org>
On Tue, Jan 20, 2026 at 04:22:55PM -0500, Jeff Layton wrote:
> On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> > Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > introduced a means to track change detection for an inode
> > via ctime updates, opposed to setting kstat.change_cookie to
> > an i_version when calling into xfs_vn_getattr().
> >
> > This introduced a regression for IMA such that an action
> > performed on a LOWER inode on a stacked file systems always
> > requires a re-evaluation if the LOWER file system does not
> > leverage kstat.change_cookie to track inode i_version or lacks
> > i_version support all together.
> >
> > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > leverages generic_fillattr() to update the change cookie.
> >
> > This is because IMA caches kstat.change_cookie to compare against an
> > inode's i_version directly in integrity_inode_attrs_changed(), and thus
> > could be out of date depending on how file systems set
> > kstat.change_cookie.
> >
> > To address this, require integrity_inode_attrs_changed() to query
> > vfs_getattr_nosec() to compare the cached version against
> > kstat.change_cookie directly. This ensures that when updates occur,
> > we're accessing the same changed inode version on changes, and fallback
> > to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> > result mask.
> >
> > Lastly, because EVM still relies on querying and caching a inode's
> > i_version directly, the integrity_inode_attrs_changed() falls back to the
> > original inode.i_version != cached comparison.
> >
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> > struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> > using multigrain ctime (as well as other file systems) for
> > change detection in commit 1cf7e834a6fb ("xfs: switch to
> > multigrain timestamps").
> >
> > Because file systems may implement i_version as they see fit, IMA
> > caching may be behind as well as file systems that don't support/export
> > i_version. Thus we're proposing to compare against the kstat.change_cookie
> > directly to the cached version, and fall back to a ctime guard when
> > that's not updated.
> >
> > EVM is largely left alone since there's no trivial way to query a file
> > directly in the LSM call paths to obtain kstat.change_cookie &
> > kstat.ctime to cache. Thus retains accessing i_version directly.
> >
> > Regression tests will be added to the Linux Test Project instead of
> > selftest to help catch future file system changes that may impact
> > future evaluation of IMA.
> >
> > I'd like this to be backported to at least 6.18 if possible.
> >
> > Below is a simplified test that demonstrates the issue:
> >
> > _fragment.config_
> > CONFIG_XFS_FS=y
> > CONFIG_OVERLAY_FS=y
> > CONFIG_IMA=y
> > CONFIG_IMA_WRITE_POLICY=y
> > CONFIG_IMA_READ_POLICY=y
> >
> > _./test.sh_
> >
> > IMA_POLICY="/sys/kernel/security/ima/policy"
> > TEST_BIN="/bin/date"
> > MNT_BASE="/tmp/ima_test_root"
> >
> > mkdir -p "$MNT_BASE"
> > mount -t tmpfs tmpfs "$MNT_BASE"
> > mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
> >
> > dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> > mkfs.xfs -q "$MNT_BASE/xfs.img"
> > mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> > cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
> >
> > mount -t overlay overlay -o \
> > "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> > "$MNT_BASE/ovl"
> >
> > echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
> >
> > target_prog="$MNT_BASE/ovl/test_prog"
> > setpriv --reuid nobody "$target_prog"
> > setpriv --reuid nobody "$target_prog"
> > setpriv --reuid nobody "$target_prog"
> >
> > audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
> >
> > if [[ "$audit_count" -eq 1 ]]; then
> > echo "PASS: Found exactly 1 audit event."
> > else
> > echo "FAIL: Expected 1 audit event, but found $audit_count."
> > exit 1
> > fi
> > ---
> > Changes since RFC:
> > - Remove calls to I_IS_VERSION()
> > - Function documentation/comments
> > - Abide IMA/EVM change detection fallback invariants
> > - Combined ctime guard into version for attributes struct
> > - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> > ---
> > Changes in v2:
> > - Updated commit description + message to clarify the problem.
> > - compare struct timespec64 to avoid collision possibility [Roberto].
> > - Don't check inode_attr_changed() in ima_check_last_writer()
> > - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> > ---
> > include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> > security/integrity/evm/evm_crypto.c | 4 +++-
> > security/integrity/evm/evm_main.c | 5 ++---
> > security/integrity/ima/ima_api.c | 20 +++++++++++++------
> > security/integrity/ima/ima_main.c | 18 ++++++++++-------
> > 5 files changed, 65 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -9,6 +9,8 @@
> >
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > +#include <linux/kernel.h>
> > +#include <linux/time64.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
> >
> > /* An inode's attributes for detection of changes */
> > struct integrity_inode_attributes {
> > + struct timespec64 ctime;
> > u64 version; /* track inode changes */
> > unsigned long ino;
> > dev_t dev;
> > @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> > */
> > static inline void
> > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > - u64 i_version, const struct inode *inode)
> > + u64 i_version, struct timespec64 ctime,
> > + const struct inode *inode)
> > {
> > + attrs->ctime = ctime;
> > attrs->version = i_version;
> > attrs->dev = inode->i_sb->s_dev;
> > attrs->ino = inode->i_ino;
> > @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> >
> > /*
> > * On stacked filesystems detect whether the inode or its content has changed.
> > + *
> > + * Must be called in process context.
> > */
> > static inline bool
> > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > - const struct inode *inode)
> > + struct file *file, struct inode *inode)
> > {
> > - return (inode->i_sb->s_dev != attrs->dev ||
> > - inode->i_ino != attrs->ino ||
> > - !inode_eq_iversion(inode, attrs->version));
> > + struct kstat stat;
> > +
> > + might_sleep();
> > +
> > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > + return true;
> > +
> > + /*
> > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > + * is not a good indicator of i_version support, this still retains
> > + * the logic such that a re-evaluation should still occur for EVM, and
> > + * only for IMA if vfs_getattr_nosec() fails.
> > + */
> > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT))
> > + return !IS_I_VERSION(inode) ||
> > + !inode_eq_iversion(inode, attrs->version);
>
> Seems like a reasonable way to handle getattr failure, given the
> limitations on EVM.
>
> > +
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + return stat.change_cookie != attrs->version;
> > +
> > + if (stat.result_mask & STATX_CTIME)
> > + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> > +
> > + return true;
> > }
> >
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > hmac_add_misc(desc, inode, type, data->digest);
> >
> > if (inode != d_backing_inode(dentry) && iint) {
> > + struct timespec64 ctime = {0};
> > +
> > if (IS_I_VERSION(inode))
> > i_version = inode_query_iversion(inode);
>
> It would be nice if you could change this codepath as well, but that
> looks like a much bigger project. FWIW, I took a quick look:
>
> The problematic codepath is get/setxattr, which can call into this but
> currently only receives a dentry and not a struct path. That would
> probably have to be changed. I think it's doable but it's a fair bit of
> yak shaving.
Agreed. I was looking at that a couple weeks ago to include a patch 2
for EVM, and came to the same conclusion. I did find struct iattr on the
setxattr which has struct file *, but that seemed dubious to at best.
Alternatively, create security_get/setxattr() equivalent hooks, or
have an internal mapping of sorts.
>
> > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > - inode);
> > + ctime, inode);
> > }
> >
> > /* Portable EVM signatures must include an IMA hash */
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > bool ret = false;
> >
> > if (iint) {
> > - ret = (!IS_I_VERSION(metadata_inode) ||
> > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > - metadata_inode));
> > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > + NULL, metadata_inode);
> > if (ret)
> > iint->evm_status = INTEGRITY_UNKNOWN;
> > }
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > int length;
> > void *tmpbuf;
> > u64 i_version = 0;
> > + struct timespec64 ctime = {0};
> >
> > /*
> > * Always collect the modsig, because IMA might have already collected
> > @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + if (stat.result_mask & STATX_CTIME)
> > + ctime = stat.ctime;
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> >
> > iint->ima_hash = tmpbuf;
> > memcpy(iint->ima_hash, &hash, length);
> > - if (real_inode == inode)
> > + if (real_inode == inode) {
> > iint->real_inode.version = i_version;
> > - else
> > + iint->real_inode.ctime = ctime;
> > + } else {
> > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > - real_inode);
> > + ctime, real_inode);
> > + }
> >
> > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > if (!result)
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -22,12 +22,14 @@
> > #include <linux/mount.h>
> > #include <linux/mman.h>
> > #include <linux/slab.h>
> > +#include <linux/stat.h>
> > #include <linux/xattr.h>
> > #include <linux/ima.h>
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > #include <linux/evm.h>
> > #include <linux/crash_dump.h>
> > +#include <linux/time64.h>
> >
> > #include "ima.h"
> >
> > @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > &iint->atomic_flags);
> > if ((iint->flags & IMA_NEW_FILE) ||
> > vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > - AT_STATX_SYNC_AS_STAT) ||
> > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > - stat.change_cookie != iint->real_inode.version) {
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > + AT_STATX_SYNC_AS_STAT) ||
> > + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> > + stat.change_cookie != iint->real_inode.version :
> > + (!(stat.result_mask & STATX_CTIME) ||
> > + !timespec64_equal(&stat.ctime,
> > + &iint->real_inode.ctime)))) {
> > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > iint->measured_pcrs = 0;
> > if (update)
> > @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > real_inode = d_real_inode(file_dentry(file));
> > if (real_inode != inode &&
> > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > - if (!IS_I_VERSION(real_inode) ||
> > - integrity_inode_attrs_changed(&iint->real_inode,
> > - real_inode)) {
> > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > + file, real_inode)) {
> > iint->flags &= ~IMA_DONE_MASK;
> > iint->measured_pcrs = 0;
> > }
> >
> > ---
> > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> >
> > Best regards,
>
> Looks like a good change overall. Thanks for fixing this up. I don't
> know IMA or EVM very well, so FWIW:
Thanks! Fun investigation & fix.
>
> Acked-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] selftests/landlock: Move some UAPI header inclusions after libc ones
From: Mickaël Salaün @ 2026-01-20 21:46 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Shuah Khan,
Matthieu Baerts, Mat Martineau, Geliang Tang, Günther Noack,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Hao Luo, Jiri Olsa, netdev, linux-kernel, linux-api,
Arnd Bergmann, linux-kselftest, mptcp, linux-security-module, bpf,
libc-alpha, Carlos O'Donell, Adhemerval Zanella, Rich Felker,
klibc, Florian Weimer
In-Reply-To: <20260120-uapi-sockaddr-v2-2-63c319111cf6@linutronix.de>
On Tue, Jan 20, 2026 at 03:10:32PM +0100, Thomas Weißschuh wrote:
> Interleaving inclusions of UAPI headers and libc headers is problematic.
> Both sets of headers define conflicting symbols. To enable their
> coexistence a compatibility-mechanism is in place.
>
> An upcoming change will define 'struct sockaddr' from linux/socket.h.
> However sys/socket.h from libc does not yet handle this case and a
> symbol conflict will arise.
>
> Move the inclusion of all UAPI headers after the inclusion of the glibc
> ones, so the compatibility mechanism from the UAPI headers is used.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: Mickaël Salaün <mic@digikod.net>
Thanks!
> ---
> tools/testing/selftests/landlock/audit.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
> index 44eb433e9666..c12b16c690dc 100644
> --- a/tools/testing/selftests/landlock/audit.h
> +++ b/tools/testing/selftests/landlock/audit.h
> @@ -7,9 +7,6 @@
>
> #define _GNU_SOURCE
> #include <errno.h>
> -#include <linux/audit.h>
> -#include <linux/limits.h>
> -#include <linux/netlink.h>
> #include <regex.h>
> #include <stdbool.h>
> #include <stdint.h>
> @@ -20,6 +17,10 @@
> #include <sys/time.h>
> #include <unistd.h>
>
> +#include <linux/audit.h>
> +#include <linux/limits.h>
> +#include <linux/netlink.h>
> +
> #include "kselftest.h"
>
> #ifndef ARRAY_SIZE
>
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Jeff Layton @ 2026-01-20 22:24 UTC (permalink / raw)
To: Frederick Lawler
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aW_4V93kczhzFjQ3@CMGLRV3>
On Tue, 2026-01-20 at 15:49 -0600, Frederick Lawler wrote:
> On Tue, Jan 20, 2026 at 04:22:55PM -0500, Jeff Layton wrote:
> > On Tue, 2026-01-20 at 14:20 -0600, Frederick Lawler wrote:
> > > Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > > introduced a means to track change detection for an inode
> > > via ctime updates, opposed to setting kstat.change_cookie to
> > > an i_version when calling into xfs_vn_getattr().
> > >
> > > This introduced a regression for IMA such that an action
> > > performed on a LOWER inode on a stacked file systems always
> > > requires a re-evaluation if the LOWER file system does not
> > > leverage kstat.change_cookie to track inode i_version or lacks
> > > i_version support all together.
> > >
> > > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > > will require re-evaluation. Stacking TPMFS on XFS for instance, once the
> > > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > > leverages generic_fillattr() to update the change cookie.
> > >
> > > This is because IMA caches kstat.change_cookie to compare against an
> > > inode's i_version directly in integrity_inode_attrs_changed(), and thus
> > > could be out of date depending on how file systems set
> > > kstat.change_cookie.
> > >
> > > To address this, require integrity_inode_attrs_changed() to query
> > > vfs_getattr_nosec() to compare the cached version against
> > > kstat.change_cookie directly. This ensures that when updates occur,
> > > we're accessing the same changed inode version on changes, and fallback
> > > to compare against kstat.ctime when STATX_CHANGE_COOKIE is missing from
> > > result mask.
> > >
> > > Lastly, because EVM still relies on querying and caching a inode's
> > > i_version directly, the integrity_inode_attrs_changed() falls back to the
> > > original inode.i_version != cached comparison.
> > >
> > > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > ---
> > > We uncovered a case in kernels >= 6.13 where XFS is no longer updating
> > > struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
> > > using multigrain ctime (as well as other file systems) for
> > > change detection in commit 1cf7e834a6fb ("xfs: switch to
> > > multigrain timestamps").
> > >
> > > Because file systems may implement i_version as they see fit, IMA
> > > caching may be behind as well as file systems that don't support/export
> > > i_version. Thus we're proposing to compare against the kstat.change_cookie
> > > directly to the cached version, and fall back to a ctime guard when
> > > that's not updated.
> > >
> > > EVM is largely left alone since there's no trivial way to query a file
> > > directly in the LSM call paths to obtain kstat.change_cookie &
> > > kstat.ctime to cache. Thus retains accessing i_version directly.
> > >
> > > Regression tests will be added to the Linux Test Project instead of
> > > selftest to help catch future file system changes that may impact
> > > future evaluation of IMA.
> > >
> > > I'd like this to be backported to at least 6.18 if possible.
> > >
> > > Below is a simplified test that demonstrates the issue:
> > >
> > > _fragment.config_
> > > CONFIG_XFS_FS=y
> > > CONFIG_OVERLAY_FS=y
> > > CONFIG_IMA=y
> > > CONFIG_IMA_WRITE_POLICY=y
> > > CONFIG_IMA_READ_POLICY=y
> > >
> > > _./test.sh_
> > >
> > > IMA_POLICY="/sys/kernel/security/ima/policy"
> > > TEST_BIN="/bin/date"
> > > MNT_BASE="/tmp/ima_test_root"
> > >
> > > mkdir -p "$MNT_BASE"
> > > mount -t tmpfs tmpfs "$MNT_BASE"
> > > mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
> > >
> > > dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
> > > mkfs.xfs -q "$MNT_BASE/xfs.img"
> > > mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
> > > cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
> > >
> > > mount -t overlay overlay -o \
> > > "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
> > > "$MNT_BASE/ovl"
> > >
> > > echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
> > >
> > > target_prog="$MNT_BASE/ovl/test_prog"
> > > setpriv --reuid nobody "$target_prog"
> > > setpriv --reuid nobody "$target_prog"
> > > setpriv --reuid nobody "$target_prog"
> > >
> > > audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
> > >
> > > if [[ "$audit_count" -eq 1 ]]; then
> > > echo "PASS: Found exactly 1 audit event."
> > > else
> > > echo "FAIL: Expected 1 audit event, but found $audit_count."
> > > exit 1
> > > fi
> > > ---
> > > Changes since RFC:
> > > - Remove calls to I_IS_VERSION()
> > > - Function documentation/comments
> > > - Abide IMA/EVM change detection fallback invariants
> > > - Combined ctime guard into version for attributes struct
> > > - Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
> > > ---
> > > Changes in v2:
> > > - Updated commit description + message to clarify the problem.
> > > - compare struct timespec64 to avoid collision possibility [Roberto].
> > > - Don't check inode_attr_changed() in ima_check_last_writer()
> > > - Link to v1: https://lore.kernel.org/r/20260112-xfs-ima-fixup-v1-1-8d13b6001312@cloudflare.com
> > > ---
> > > include/linux/integrity.h | 40 ++++++++++++++++++++++++++++++++-----
> > > security/integrity/evm/evm_crypto.c | 4 +++-
> > > security/integrity/evm/evm_main.c | 5 ++---
> > > security/integrity/ima/ima_api.c | 20 +++++++++++++------
> > > security/integrity/ima/ima_main.c | 18 ++++++++++-------
> > > 5 files changed, 65 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > > index f5842372359be5341b6870a43b92e695e8fc78af..46f57402b790c9c275b85f0b30819fa196fc20c7 100644
> > > --- a/include/linux/integrity.h
> > > +++ b/include/linux/integrity.h
> > > @@ -9,6 +9,8 @@
> > >
> > > #include <linux/fs.h>
> > > #include <linux/iversion.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/time64.h>
> > >
> > > enum integrity_status {
> > > INTEGRITY_PASS = 0,
> > > @@ -31,6 +33,7 @@ static inline void integrity_load_keys(void)
> > >
> > > /* An inode's attributes for detection of changes */
> > > struct integrity_inode_attributes {
> > > + struct timespec64 ctime;
> > > u64 version; /* track inode changes */
> > > unsigned long ino;
> > > dev_t dev;
> > > @@ -42,8 +45,10 @@ struct integrity_inode_attributes {
> > > */
> > > static inline void
> > > integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > - u64 i_version, const struct inode *inode)
> > > + u64 i_version, struct timespec64 ctime,
> > > + const struct inode *inode)
> > > {
> > > + attrs->ctime = ctime;
> > > attrs->version = i_version;
> > > attrs->dev = inode->i_sb->s_dev;
> > > attrs->ino = inode->i_ino;
> > > @@ -51,14 +56,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > >
> > > /*
> > > * On stacked filesystems detect whether the inode or its content has changed.
> > > + *
> > > + * Must be called in process context.
> > > */
> > > static inline bool
> > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > - const struct inode *inode)
> > > + struct file *file, struct inode *inode)
> > > {
> > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > - inode->i_ino != attrs->ino ||
> > > - !inode_eq_iversion(inode, attrs->version));
> > > + struct kstat stat;
> > > +
> > > + might_sleep();
> > > +
> > > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > > + return true;
> > > +
> > > + /*
> > > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > > + * is not a good indicator of i_version support, this still retains
> > > + * the logic such that a re-evaluation should still occur for EVM, and
> > > + * only for IMA if vfs_getattr_nosec() fails.
> > > + */
> > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT))
> > > + return !IS_I_VERSION(inode) ||
> > > + !inode_eq_iversion(inode, attrs->version);
> >
> > Seems like a reasonable way to handle getattr failure, given the
> > limitations on EVM.
> >
> > > +
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + return stat.change_cookie != attrs->version;
> > > +
> > > + if (stat.result_mask & STATX_CTIME)
> > > + return !timespec64_equal(&stat.ctime, &attrs->ctime);
> > > +
> > > + return true;
> > > }
> > >
> > >
> > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > > index a5e730ffda57fbc0a91124adaa77b946a12d08b4..361ee7b216247a0d6d2f518e82fb6e92dc355afe 100644
> > > --- a/security/integrity/evm/evm_crypto.c
> > > +++ b/security/integrity/evm/evm_crypto.c
> > > @@ -297,10 +297,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > > hmac_add_misc(desc, inode, type, data->digest);
> > >
> > > if (inode != d_backing_inode(dentry) && iint) {
> > > + struct timespec64 ctime = {0};
> > > +
> > > if (IS_I_VERSION(inode))
> > > i_version = inode_query_iversion(inode);
> >
> > It would be nice if you could change this codepath as well, but that
> > looks like a much bigger project. FWIW, I took a quick look:
> >
> > The problematic codepath is get/setxattr, which can call into this but
> > currently only receives a dentry and not a struct path. That would
> > probably have to be changed. I think it's doable but it's a fair bit of
> > yak shaving.
>
> Agreed. I was looking at that a couple weeks ago to include a patch 2
> for EVM, and came to the same conclusion. I did find struct iattr on the
> setxattr which has struct file *, but that seemed dubious to at best.
>
Yeah, it's not set in all cases. setxattr is a pathname-based
operation, so an open file sometimes just doesn't exist.
> Alternatively, create security_get/setxattr() equivalent hooks, or
> have an internal mapping of sorts.
>
>
Those are possibilities too.
A dentry without a mount is somewhat ambiguous though. IMO, it would be
best to just change all of the xattr_handler functions to take struct
path instead of a dentry.
Maybe a good topic for LSF/MM...
> > > integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> > > - inode);
> > > + ctime, inode);
> > > }
> > >
> > > /* Portable EVM signatures must include an IMA hash */
> > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > > --- a/security/integrity/evm/evm_main.c
> > > +++ b/security/integrity/evm/evm_main.c
> > > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > > bool ret = false;
> > >
> > > if (iint) {
> > > - ret = (!IS_I_VERSION(metadata_inode) ||
> > > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > > - metadata_inode));
> > > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > > + NULL, metadata_inode);
> > > if (ret)
> > > iint->evm_status = INTEGRITY_UNKNOWN;
> > > }
> > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..0d8e0a3ebd34b70bb1b4cc995aae5d4adc90a585 100644
> > > --- a/security/integrity/ima/ima_api.c
> > > +++ b/security/integrity/ima/ima_api.c
> > > @@ -254,6 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > int length;
> > > void *tmpbuf;
> > > u64 i_version = 0;
> > > + struct timespec64 ctime = {0};
> > >
> > > /*
> > > * Always collect the modsig, because IMA might have already collected
> > > @@ -272,10 +273,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > * to an initial measurement/appraisal/audit, but was modified to
> > > * assume the file changed.
> > > */
> > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > AT_STATX_SYNC_AS_STAT);
> > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > - i_version = stat.change_cookie;
> > > + if (!result) {
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + i_version = stat.change_cookie;
> > > + if (stat.result_mask & STATX_CTIME)
> > > + ctime = stat.ctime;
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
> > > @@ -305,11 +311,13 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > >
> > > iint->ima_hash = tmpbuf;
> > > memcpy(iint->ima_hash, &hash, length);
> > > - if (real_inode == inode)
> > > + if (real_inode == inode) {
> > > iint->real_inode.version = i_version;
> > > - else
> > > + iint->real_inode.ctime = ctime;
> > > + } else {
> > > integrity_inode_attrs_store(&iint->real_inode, i_version,
> > > - real_inode);
> > > + ctime, real_inode);
> > > + }
> > >
> > > /* Possibly temporary failure due to type of read (eg. O_DIRECT) */
> > > if (!result)
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index 5770cf691912aa912fc65280c59f5baac35dd725..54b638663c9743d39e5fb65711dbd9698b38e39b 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -22,12 +22,14 @@
> > > #include <linux/mount.h>
> > > #include <linux/mman.h>
> > > #include <linux/slab.h>
> > > +#include <linux/stat.h>
> > > #include <linux/xattr.h>
> > > #include <linux/ima.h>
> > > #include <linux/fs.h>
> > > #include <linux/iversion.h>
> > > #include <linux/evm.h>
> > > #include <linux/crash_dump.h>
> > > +#include <linux/time64.h>
> > >
> > > #include "ima.h"
> > >
> > > @@ -199,10 +201,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > > &iint->atomic_flags);
> > > if ((iint->flags & IMA_NEW_FILE) ||
> > > vfs_getattr_nosec(&file->f_path, &stat,
> > > - STATX_CHANGE_COOKIE,
> > > - AT_STATX_SYNC_AS_STAT) ||
> > > - !(stat.result_mask & STATX_CHANGE_COOKIE) ||
> > > - stat.change_cookie != iint->real_inode.version) {
> > > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > > + AT_STATX_SYNC_AS_STAT) ||
> > > + ((stat.result_mask & STATX_CHANGE_COOKIE) ?
> > > + stat.change_cookie != iint->real_inode.version :
> > > + (!(stat.result_mask & STATX_CTIME) ||
> > > + !timespec64_equal(&stat.ctime,
> > > + &iint->real_inode.ctime)))) {
> > > iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > > iint->measured_pcrs = 0;
> > > if (update)
> > > @@ -328,9 +333,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > > real_inode = d_real_inode(file_dentry(file));
> > > if (real_inode != inode &&
> > > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > > - if (!IS_I_VERSION(real_inode) ||
> > > - integrity_inode_attrs_changed(&iint->real_inode,
> > > - real_inode)) {
> > > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > > + file, real_inode)) {
> > > iint->flags &= ~IMA_DONE_MASK;
> > > iint->measured_pcrs = 0;
> > > }
> > >
> > > ---
> > > base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> > > change-id: 20251212-xfs-ima-fixup-931780a62c2c
> > >
> > > Best regards,
> >
> > Looks like a good change overall. Thanks for fixing this up. I don't
> > know IMA or EVM very well, so FWIW:
>
> Thanks! Fun investigation & fix.
>
> >
> > Acked-by: Jeff Layton <jlayton@kernel.org>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Tingmao Wang @ 2026-01-21 0:26 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün
Cc: Justin Suess, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze
In-Reply-To: <20251230103917.10549-7-gnoack3000@gmail.com>
On 12/30/25 10:39, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operations security check. It stores one bit
> for each combination of access right and layer index. If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
>
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t. The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
>
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
>
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
>
> * We can use bitwise operations in multiple places where we previously
> looped over bits individually with macros. (Should require less
> branch speculation)
>
> * Code is ~160 lines smaller.
>
> Other noteworthy changes:
>
> * Clarify deny_mask_t and the code assembling it.
> * Document what that value looks like
> * Make writing and reading functions specific to file system rules.
> (It only worked for FS rules before as well, but going all the way
> simplifies the code logic more.)
In the original commit message that added this type [1] there was this
statement:
> Implementing deny_masks_t with a bitfield instead of a struct enables a
> generic implementation to store and extract layer levels.
At some point when looking at this I was wondering why this wasn't a
struct with 2 u8:4 fields, but rather, a u8 with bit manipulation code.
While it is possible that I might have just misunderstood it, reading the
above statement my take-away was that a struct would have forced us to
address the indices with specific names, e.g. it would need to be defined
like
struct deny_masks_t {
u8 ioctl:4;
u8 truncate:4;
}
And it would thus not be possible to manipulate the indices in a generic
way (e.g. the way it was implemented before, given
all_existing_optional_access and access_bit, read and write the right
bits).
However, since we're now removing that generic-ability, should we consider
turning it into a struct? (If later on we have different access types
that also have optional accesses, we could use a union of structs)
btw, since this causes conflicts with the quiet flag series and Mickaël
has indicated that this should be merged first, I will probably have to
make my series based on top of this. Will watch this series to see if
there are more changes.
Also, this transpose and code simplification should also simplify the
mutable domains work so thanks for the refactor!
A while ago I also made some benchmarking script which I sent a PR to
landlock-test-tools [2], and earlier I tested this patch with it, and saw
some improvement (but it was much less in terms of percentage, which may
be due to the lower directory depth, or may be due to other unknown
reason):
TestDescription(landlock=True, dir_depth=10, nb_extra_rules=10)
base.2:
c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=1718.15, min=1663.00, max=275949.00, median=1696.46, stddev=437.52
95% confidence interval: [1718.03 .. 1718.28]
Estimated landlock overhead (vs no-landlock): 226.5%
48bd90e91fe6.2:
c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=1709.60, min=1633.00, max=280608.00, median=1688.83, stddev=441.83
95% confidence interval: [1709.48 .. 1709.73]
** Improved 0.5% **
...
[1660 .. 1669]: [1660 .. 1669]: ###
[1670 .. 1679]: ## [1670 .. 1679]: ###############
[1680 .. 1689]: ###################### [1680 .. 1689]: #################################
[1690 .. 1699]: ######################################## [1690 .. 1699]: ##################################
[1700 .. 1709]: ############################ [1700 .. 1709]: #############
[1710 .. 1719]: ######### [1710 .. 1719]: ##
[1720 .. 1729]: ## [1720 .. 1729]:
...
Estimated landlock overhead (vs no-landlock): 223.0%
TestDescription(landlock=True, dir_depth=29, nb_extra_rules=10)
base.2:
c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=3869.66, min=3727.00, max=272563.00, median=3813.42, stddev=666.18
95% confidence interval: [3869.47 .. 3869.86]
Estimated landlock overhead (vs no-landlock): 427.3%
48bd90e91fe6.2:
c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=3855.61, min=3697.00, max=271690.00, median=3804.82, stddev=682.74
95% confidence interval: [3855.41 .. 3855.81]
** Improved 0.4% **
...
[3750 .. 3759]: [3750 .. 3759]: #
[3760 .. 3769]: [3760 .. 3769]: #######
[3770 .. 3779]: [3770 .. 3779]: ###############
[3780 .. 3789]: #### [3780 .. 3789]: ###################
[3790 .. 3799]: ################### [3790 .. 3799]: ###################
[3800 .. 3809]: ###################################### [3800 .. 3809]: ########################
[3810 .. 3819]: ######################################## [3810 .. 3819]: ############################
[3820 .. 3829]: ########################## [3820 .. 3829]: #####################
[3830 .. 3839]: ############# [3830 .. 3839]: #########
[3840 .. 3849]: ###### [3840 .. 3849]: ##
[3850 .. 3859]: ## [3850 .. 3859]:
[3860 .. 3869]: [3860 .. 3869]:
[3870 .. 3879]: [3870 .. 3879]:
...
[4980 .. 4989]: [4980 .. 4989]:
[4990 .. 4999]: [4990 .. 4999]:
[5000 .. 272563]: # [5000 .. 271690]: #
Estimated landlock overhead (vs no-landlock): 424.2%
Full data including test with 0 depth, or 1000 rules:
https://fileshare.maowtm.org/landlock-20251230/index.html
[1]: https://lore.kernel.org/all/20250320190717.2287696-15-mic@digikod.net/
[2]: https://github.com/landlock-lsm/landlock-test-tools/pull/17
^ permalink raw reply
* [PATCH v2] [Reminder] Landlock documentation improvements
From: Samasth Norway Ananda @ 2026-01-21 2:14 UTC (permalink / raw)
To: samasth.norway.ananda; +Cc: gnoack, linux-kernel, linux-security-module, mic
In-Reply-To: <20260103002722.1465371-1-samasth.norway.ananda@oracle.com>
Hi Mickael/Gunther,
Just a friendly ping on the landlock documentation v2 patchset. I haven't heard back yet, and I wanted to check if you had the chance to review it.
If there are any issues or further changes required, please let me know.
("landlock: Documentation improvements")
https://lore.kernel.org/linux-security-module/20260103002722.1465371-1-samasth.norway.ananda@oracle.com/
Thanks again for your time and feedback!
Best regards,
Samasth.
^ permalink raw reply
* TVでも話題。地方で儲かる、焼肉ビジネス
From: ときわ亭フランチャイズ事務局 @ 2026-01-21 4:24 UTC (permalink / raw)
To: linux-security-module
新たな事業をお考えの経営者様へ
いつもお世話になります。
TBSテレビ『坂上＆指原のつぶれない店』で取り上げられるなど、
今話題の、待ち時間0秒の飲み放題を提供する
焼肉酒場「 ときわ亭 」のフランチャイズシステム説明会をご案内いたします。
わずか2年半で全国80店舗展開した
「0秒レモンサワー ときわ亭」の
ビジネスモデル、収益性や最新の飲食市場実態を
オンラインによる説明会形式で分かりやすく、丁寧にお伝えします。
※本セミナー参加、店舗オープンの場合
研修費用50万円を本部支援！
専門家による、資金調達相談が無料！
このような方におススメ
・銀行融資を受けやすくなるコツをしりたい
・飲食店の経営に興味がある
・焼肉屋の経営実態がしりたい
こういった方は、ぜひお気軽にご視聴くださいませ。
▼ オンライン説明会のご視聴予約はこちら ▼
https://tokiwatei-fc.work/web/
━━━━━━━━━━━━━━━━━━━━━━━━━━━
■ ご説明するフランチャイズシステム
0秒レモンサワー 仙台ホルモン焼肉酒場ときわ亭
■ 開催日程
・ 1月27日（火）15:00〜16:30 ＠オンライン開催
■ 定員
・10社限定
■ 参加費
・不要
━━━━━━━━━━━━━━━━━━━━━━━━━━━━
本メールが不要の方には大変失礼しました。
今後ご案内が不要な際は、下記URLにて配信解除を承っています。
https://tokiwatei-fc.work/mail/
━━━━━━━━━━━━━━━━━━━━━━━━━━━━
主催：GOSSO株式会社（ときわ亭フランチャイズ本部）
東京都渋谷区宇田川町14-13 宇田川ビルディング6F
03-6316-8191
━━━━━━━━━━━━━━━━━━━━━━━━━━━━
^ permalink raw reply
* [PATCH] efi: Mark UEFI skip cert quirks list as __initconst
From: Thomas Weißschuh @ 2026-01-21 7:43 UTC (permalink / raw)
To: Jarkko Sakkinen, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-integrity, keyrings, linux-security-module, linux-kernel,
Thomas Weißschuh
The only user is marked as __init, so there is no reason to keep this
data around after bootup.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
security/integrity/platform_certs/load_uefi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index d1fdd113450a..26523a2b29c7 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -19,7 +19,7 @@
* a crash disabling EFI runtime services. The following quirk skips reading
* these variables.
*/
-static const struct dmi_system_id uefi_skip_cert[] = {
+static const struct dmi_system_id uefi_skip_cert[] __initconst = {
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1") },
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,2") },
{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,3") },
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20260121-uefi-certs-initconst-d5dadc5f01dd
Best regards,
--
Thomas Weißschuh <thomas.weissschuh@linutronix.de>
^ permalink raw reply related
* Re: [PATCH v2 1/3] landlock: Add missing ABI 7 case in documentation example
From: Günther Noack @ 2026-01-21 9:37 UTC (permalink / raw)
To: Samasth Norway Ananda; +Cc: mic, linux-security-module, linux-kernel
In-Reply-To: <20260103002722.1465371-2-samasth.norway.ananda@oracle.com>
Thanks for the updated documentation!
The comments below are mostly about the structure in the bigger
document context. The wording seems good.
On Fri, Jan 02, 2026 at 04:27:13PM -0800, Samasth Norway Ananda wrote:
> Add the missing case 6 and case 7 handling in the ABI version
> compatibility example to properly handle ABI < 7 kernels.
>
> Add an optional backwards compatibility section for restrict flags
> between the case analysis and landlock_restrict_self() call. The main
> tutorial example remains unchanged with
> landlock_restrict_self(ruleset_fd, 0) to keep it simple for users who
> don't need logging flags.
>
> Also fix misleading description of the /usr rule which incorrectly
> stated it "only allow[s] reading" when the code actually allows both
> reading and executing (LANDLOCK_ACCESS_FS_EXECUTE is included in
> allowed_access).
>
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
> ---
> Documentation/userspace-api/landlock.rst | 35 +++++++++++++++++++++---
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 1d0c2c15c22e..650c7b368561 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -127,6 +127,12 @@ version, and only use the available subset of access rights:
> /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> LANDLOCK_SCOPE_SIGNAL);
> + __attribute__((fallthrough));
> + case 6:
> + /* Removes logging flags for ABI < 7 */
> + __attribute__((fallthrough));
> + case 7:
> + break;
> }
This code does nothing, and the comment about removing logging flags
is unclear in that context without pointing to the later section.
IMHO we can either say that logging flags are removed in the code
further below, or just leave it out.
The "case 7" case is not necessary.
>
> This enables the creation of an inclusive ruleset that will contain our rules.
> @@ -142,8 +148,9 @@ This enables the creation of an inclusive ruleset that will contain our rules.
> }
>
> We can now add a new rule to this ruleset thanks to the returned file
> -descriptor referring to this ruleset. The rule will only allow reading the
> -file hierarchy ``/usr``. Without another rule, write actions would then be
> +descriptor referring to this ruleset. The rule will allow reading and
> +executing files in the ``/usr`` hierarchy. Without another rule, write actions
> +and other operations (make_dir, remove_file, etc.) would then be
> denied by the ruleset. To add ``/usr`` to the ruleset, we open it with the
> ``O_PATH`` flag and fill the &struct landlock_path_beneath_attr with this file
> descriptor.
> @@ -191,10 +198,30 @@ number for a specific action: HTTPS connections.
> err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
> &net_port, 0);
>
> +Backwards compatibility for restrict flags
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
If you were to add this header, then the section about prctl() and
landlock_restrict_self() would also now appear under "Backwards
compatibility for restrict flags", and that would be confusing. I
would recommend to drop the header for now and clarify in the text
what it is about. (IMHO, it is visually reasonably easy to understand
the structure of the different sections, as they always follow the
structure "text + code snippet".)
If we want to introduce a new level of headers below "Defining and
enforcing a security policy", we would have to introduce subsections
for the other steps as well, so that it is more structured. (If we
wanted to do that though, it would probably be better to do it in a
separate commit.)
> +When passing a non-zero ``flags`` argument to ``landlock_restrict_self()``, the
> +following backwards compatibility check needs to be taken into account:
> +
> +.. code-block:: c
> +
> + /*
> + * Desired restriction flags, see ABI version section above.
^^^^^^^^^^^^^^^^^^^^^^^^^
The ABI section does not talk about the restriction flags much;
I would have expected that to link to the place where the restriction
flags are documented, which would be the system call documentation
from the header?
https://docs.kernel.org/userspace-api/landlock.html#enforcing-a-ruleset
> + * This value is only an example and differs by use case.
> + */
> + int restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
> + if (abi < 7) {
> + /* Clear logging flags unsupported in ABI < 7 */
> + restrict_flags &= ~(LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
> + }
> +
> The next step is to restrict the current thread from gaining more privileges
> (e.g. through a SUID binary). We now have a ruleset with the first rule
> -allowing read access to ``/usr`` while denying all other handled accesses for
> -the filesystem, and a second rule allowing HTTPS connections.
> +allowing read and execute access to ``/usr`` while denying all other handled
> +accesses for the filesystem, and a second rule allowing HTTPS connections.
>
If you are setting a "restrict_flags" variable here, then you would
also need to use that variable in the call to landlock_restrict_self()
in the documentation below, so that the different code sections in the
documentation work together.
—Günther
^ permalink raw reply
* Re: [PATCH v2 2/3] landlock: Add comprehensive errata documentation
From: Günther Noack @ 2026-01-21 10:19 UTC (permalink / raw)
To: Samasth Norway Ananda; +Cc: mic, linux-security-module, linux-kernel
In-Reply-To: <20260103002722.1465371-3-samasth.norway.ananda@oracle.com>
Hello!
Overall, this looks very good to me, thanks for documenting that!
Some smaller remarks, mostly on structure.
On Fri, Jan 02, 2026 at 04:27:14PM -0800, Samasth Norway Ananda wrote:
> Add comprehensive documentation for the Landlock errata mechanism,
> including how to query errata using LANDLOCK_CREATE_RULESET_ERRATA
> and links to enhanced detailed descriptions in the kernel source.
>
> Also enhance existing DOC sections in security/landlock/errata/abi-*.h
> files with Impact sections, and update the code comment in syscalls.c
> to remind developers to update errata documentation when applicable.
>
> This addresses the gap where the kernel implements errata tracking
> but provides no user-facing documentation on how to use it, while
> improving the existing technical documentation in-place rather than
> duplicating it.
>
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
> ---
> Documentation/userspace-api/landlock.rst | 60 +++++++++++++++++++++++-
> security/landlock/errata/abi-1.h | 8 ++++
> security/landlock/errata/abi-4.h | 7 +++
> security/landlock/errata/abi-6.h | 10 ++++
> security/landlock/syscalls.c | 4 +-
> 5 files changed, 87 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 650c7b368561..930723fd7c1a 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -8,7 +8,7 @@ Landlock: unprivileged access control
> =====================================
>
> :Author: Mickaël Salaün
> -:Date: March 2025
> +:Date: January 2026
>
> The goal of Landlock is to enable restriction of ambient rights (e.g. global
> filesystem or network access) for a set of processes. Because Landlock
> @@ -458,6 +458,64 @@ system call:
> printf("Landlock supports LANDLOCK_ACCESS_FS_REFER.\n");
> }
>
> +Landlock Errata
> +---------------
> +
> +In addition to ABI versions, Landlock provides an errata mechanism to track
> +fixes for issues that may affect backwards compatibility or require userspace
> +awareness. The errata bitmask can be queried using:
> +
> +.. code-block:: c
> +
> + int errata;
> +
> + errata = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_ERRATA);
> + if (errata < 0) {
> + /* Landlock not available or disabled */
> + return 0;
> + }
> +
> +The returned value is a bitmask where each bit represents a specific erratum.
> +If bit N is set (``errata & (1 << (N - 1))``), then erratum N has been fixed
> +in the running kernel.
> +
> +.. warning::
> +
> + **Most applications should NOT check errata.** In 99.9% of cases, checking
> + errata is unnecessary, increases code complexity, and can potentially
> + decrease protection if misused. For example, disabling the sandbox when an
> + erratum is not fixed could leave the system less secure than using
> + Landlock's best-effort protection. When in doubt, ignore errata.
> +
> +For detailed technical descriptions of each erratum, including their impact
> +and when they affect applications, see the DOC sections in the kernel source:
> +
> +- **Erratum 1: TCP socket identification (ABI 4)** - See ``erratum_1`` in ``security/landlock/errata/abi-4.h``
> +- **Erratum 2: Scoped signal handling (ABI 6)** - See ``erratum_2`` in ``security/landlock/errata/abi-6.h``
> +- **Erratum 3: Disconnected directory handling (ABI 1)** - See ``erratum_3`` in ``security/landlock/errata/abi-1.h``
Is it not possible to include the errata descriptions here through the header?
For instance, further below in this document, we also include the
system call documentation from the UAPI header, using:
.. kernel-doc:: include/uapi/linux/landlock.h
:identifiers: fs_access net_access scope
> +
> +How to Check for Errata
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If you determine that your application needs to check for specific errata,
> +use this pattern:
> +
> +.. code-block:: c
> +
> + int errata = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_ERRATA);
> + if (errata >= 0) {
> + /* Check for specific erratum (1-indexed) */
> + if (errata & (1 << (erratum_number - 1))) {
> + /* Erratum N is fixed in this kernel */
> + } else {
> + /* Erratum N is NOT fixed - consider implications for your use case */
> + }
> + }
> +
> +**Important:** Only check errata if your application specifically relies on
> +behavior that changed due to the fix. The fixes generally make Landlock less
> +restrictive or more correct, not more restrictive.
> +
> The following kernel interfaces are implicitly supported by the first ABI
> version. Features only supported from a specific version are explicitly marked
> as such.
At the end of your added text, there is a similar issue as in the
other commit, where a section that previously belonged elsewhere is
now part of your new section by accident.
I think the paragraph "The following kernel interfaces are implicitly
supported..." is meant to belong to the "Landlock ABI versions"
section which is above the text that you added. I would recommend to
rephrase it slightly, because it also talks about the "following
kernel interfaces", which are not immediately following any more, e.g.
"All Landlock kernel interfaces are supported by the first ABI
version unless it is explicitly noted in their documentation."
Please feel free to rephrase if a different phrasing seems more
suitable.
> diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h
> index e8a2bff2e5b6..ba9895bf8ce1 100644
> --- a/security/landlock/errata/abi-1.h
> +++ b/security/landlock/errata/abi-1.h
> @@ -12,5 +12,13 @@
> * hierarchy down to its filesystem root and those from the related mount point
> * hierarchy. This prevents access right widening through rename or link
> * actions.
> + *
> + * Impact:
> + *
> + * Without this fix, it was possible to widen access rights through rename or
> + * link actions involving disconnected directories, potentially bypassing
> + * ``LANDLOCK_ACCESS_FS_REFER`` restrictions. This could allow privilege
> + * escalation in complex mount scenarios where directories become disconnected
> + * from their original mount points.
> */
> LANDLOCK_ERRATUM(3)
> diff --git a/security/landlock/errata/abi-4.h b/security/landlock/errata/abi-4.h
> index c052ee54f89f..59574759dc1e 100644
> --- a/security/landlock/errata/abi-4.h
> +++ b/security/landlock/errata/abi-4.h
> @@ -11,5 +11,12 @@
> * :manpage:`bind(2)` and :manpage:`connect(2)` operations. This change ensures
> * that only TCP sockets are subject to TCP access rights, allowing other
> * protocols to operate without unnecessary restrictions.
> + *
> + * Impact:
> + *
> + * In kernels without this fix, using ``LANDLOCK_ACCESS_NET_BIND_TCP`` or
> + * ``LANDLOCK_ACCESS_NET_CONNECT_TCP`` would incorrectly restrict non-TCP
> + * stream protocols (SMC, MPTCP, SCTP), potentially breaking applications
> + * that rely on these protocols while using Landlock network restrictions.
> */
> LANDLOCK_ERRATUM(1)
> diff --git a/security/landlock/errata/abi-6.h b/security/landlock/errata/abi-6.h
> index df7bc0e1fdf4..a3a48b2bf2db 100644
> --- a/security/landlock/errata/abi-6.h
> +++ b/security/landlock/errata/abi-6.h
> @@ -15,5 +15,15 @@
> * interaction between threads of the same process should always be allowed.
> * This change ensures that any thread is allowed to send signals to any other
> * thread within the same process, regardless of their domain.
> + *
> + * Impact:
> + *
> + * This problem only manifests when the userspace process is itself using
> + * :manpage:`libpsx(3)` or an equivalent mechanism to enforce a Landlock policy
> + * on multiple already-running threads at once. Programs which enforce a
> + * Landlock policy at startup time and only then become multithreaded are not
> + * affected. Without this fix, signal scoping could break multi-threaded
> + * applications that expect threads within the same process to freely signal
> + * each other.
> */
> LANDLOCK_ERRATUM(2)
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0116e9f93ffe..cf5ba7715916 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -157,9 +157,11 @@ static const struct file_operations ruleset_fops = {
> /*
> * The Landlock ABI version should be incremented for each new Landlock-related
> * user space visible change (e.g. Landlock syscalls). This version should
> - * only be incremented once per Linux release, and the date in
> + * only be incremented once per Linux release. When incrementing, the date in
> * Documentation/userspace-api/landlock.rst should be updated to reflect the
> * UAPI change.
> + * If the change involves a fix that requires userspace awareness, also update
> + * the errata documentation in Documentation/userspace-api/landlock.rst.
> */
> const int landlock_abi_version = 7;
>
> --
> 2.50.1
>
The texts all look very good, thank you very much for documenting this!
—Günther
^ permalink raw reply
* あたま専門のもみほぐし店 収益性等／概要資料
From: ヘッドミント @ 2026-01-21 9:38 UTC (permalink / raw)
To: linux-security-module
お世話になります。
コンパクトにスタートできて手堅く収益をあげることのできる
フランチャイズビジネスの事業概要資料をご案内申し上げます。
小資本／小スペース／少人数の
コンパクト・フランチャイズ
ドライヘッドスパ専門店
“ヘッドミント”
・収益モデル
・開業に必要な資金
・ロイヤリティ
・スケジュール etc
↓ FC事業概要資料 ↓
https://dryheadspa-hm.biz/fc/
ドライヘッドスパとは ――― 水を使わないヘッドスパです。
足つぼや耳つぼなど、専門のもみほぐし店がありますが
ご紹介するサロンは「 頭に特化したもみほぐし店 」です。
ドライヘッドスパというジャンルの認知度は、
現時点ではそれほど高くありません。
にも関わらず、私どもがフランチャイズ展開する“ヘッドミント”の
店舗には月間450人以上の新規客が来店し、満席が続いています。
これから先、認知度が高まることで
爆発的に伸びるポテンシャルを秘めています。
フランチャイズによる事業を展開していますので、
新たな収益づくりをお考えの方は、まずは概要資料をご覧ください。
ドライヘッドスパ専門店
ヘッドミント
↓ FC事業概要資料 ↓
https://dryheadspa-hm.biz/fc/
よろしくお願いします。
------------------------------------------
株式会社じむや
愛知県名古屋市中区大須3-26-41堀田ビル
TEL：052-263-4688
------------------------------------------
本情報がご不要な方にはご迷惑をおかけし申し訳ございません。
メールマガジンの解除は、下記URLにて承っております。
https://dryheadspa-hm.biz/mail/
お手数お掛けしますがよろしくお願いします。
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-21 15:29 UTC (permalink / raw)
To: Dave Hansen, Ard Biesheuvel
Cc: Coiby Xu, linux-integrity, Heiko Carstens, Roberto Sassu,
Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <79185163-bf8f-4490-9396-3fd73b7a0c73@intel.com>
Hi Dave!
On Mon, 2026-01-19 at 10:44 -0800, Dave Hansen wrote:
> On 1/18/26 10:25, Mimi Zohar wrote:
> > As not all arch's implement arch_integrity_get_secureboot, the definition in
> > include/linux/integrity.h would need to be updated. Something like:
> >
> > -#ifdef CONFIG_INTEGRITY_SECURE_BOOT
> > +#if (defined(CONFIG_INTEGRITY_SECURE_BOOT) && \
> > + (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> > + || defined(CONFIG_PPC_SECURE_BOOT))
> >
> > Then IMA_SECURE_AND_OR_TRUSTED_BOOT and EVM could select INTEGRITY_SECURE_BOOT,
> > as suggested.
>
> This seems to be going a wee bit sideways. :)
Agreed, that was my point. :) "imply" was cleaner, but Ard objected to two
imply's.
>
> This kind of CONFIG complexity really should be left to Kconfig. C
> macros really aren't a great place to do it.
>
> The other idiom we use a lot is this in generic code:
>
> #ifndef arch_foo
> static inline void arch_foo(void) {}
> #endif
>
> Then all you have to do is make sure the arch header that #defines it is
> included before the generic code. I'm not a super huge fan of these
> because it can be hard to tell (for humans at least) _if_ the
> architecture has done the #define.
>
> But it sure beats that #ifdef maze.
Sure.
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Mimi Zohar @ 2026-01-21 15:40 UTC (permalink / raw)
To: Coiby Xu, Ard Biesheuvel, Dave Hansen
Cc: linux-integrity, Heiko Carstens, Roberto Sassu, Catalin Marinas,
Will Deacon, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <aW2i3yacr5TvWU-m@Rk>
On Mon, 2026-01-19 at 12:04 +0800, Coiby Xu wrote:
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 976e75f9b9ba..5dce572192d6 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -311,6 +311,7 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> config IMA_SECURE_AND_OR_TRUSTED_BOOT
> bool
> depends on IMA_ARCH_POLICY
> + depends on INTEGRITY_SECURE_BOOT
>
>
> Another idea is make a tree-wide arch_get_secureboot i.e. to move
> current arch_ima_get_secureboot code to arch-specific secure boot
> implementation. By this way, there will no need for a new Kconfig option
> INTEGRITY_SECURE_BOOT. But I'm not sure if there is any unforeseen
> concern.
Originally basing IMA policy on the secure boot mode was an exception. As long
as making it public isn't an issue any longer, this sounds to me. Ard, Dave, do
you have any issues with replacing arch_ima_get_secureboot() with
arch_get_secureboot()?
^ permalink raw reply
* Re: [PATCH 1/3] integrity: Make arch_ima_get_secureboot integrity-wide
From: Ard Biesheuvel @ 2026-01-21 16:25 UTC (permalink / raw)
To: Mimi Zohar
Cc: Coiby Xu, Dave Hansen, linux-integrity, Heiko Carstens,
Roberto Sassu, Catalin Marinas, Will Deacon, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
H. Peter Anvin, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Jarkko Sakkinen,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), open list,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
open list:S390 ARCHITECTURE,
open list:EXTENSIBLE FIRMWARE INTERFACE (EFI),
open list:SECURITY SUBSYSTEM, open list:KEYS/KEYRINGS_INTEGRITY
In-Reply-To: <1a0b6e5601a673a81f8823de0815f92b7afbeb60.camel@linux.ibm.com>
On Wed, 21 Jan 2026 at 16:41, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2026-01-19 at 12:04 +0800, Coiby Xu wrote:
>
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 976e75f9b9ba..5dce572192d6 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -311,6 +311,7 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > bool
> > depends on IMA_ARCH_POLICY
> > + depends on INTEGRITY_SECURE_BOOT
> >
> >
> > Another idea is make a tree-wide arch_get_secureboot i.e. to move
> > current arch_ima_get_secureboot code to arch-specific secure boot
> > implementation. By this way, there will no need for a new Kconfig option
> > INTEGRITY_SECURE_BOOT. But I'm not sure if there is any unforeseen
> > concern.
>
> Originally basing IMA policy on the secure boot mode was an exception. As long
> as making it public isn't an issue any longer, this sounds to me. Ard, Dave, do
> you have any issues with replacing arch_ima_get_secureboot() with
> arch_get_secureboot()?
I don't see an issue with that. If there is a legitimate need to
determine this even if IMA is not enabled, then this makes sense.
^ permalink raw reply
* [LSF/MM/BPF TOPIC] Refactor LSM hooks for VFS mount operations
From: Song Liu @ 2026-01-21 19:54 UTC (permalink / raw)
To: bpf, Linux-Fsdevel, lsf-pc, linux-security-module
Cc: Christian Brauner, Al Viro
Current LSM hooks do not have good coverage for VFS mount operations.
Specifically, there are the following issues (and maybe more..):
1. security_sb_mount suffers from the TOCTOU bug for bind mount and
move mount [1];
2. There is not sufficient coverage for new mount syscalls (open_tree, fspick,
etc.) [2].
A key consideration of this refactor is to minimize lock contention, especially
around namespace_sem.
I also want to discuss what features in the kernel side (kfuncs,
iterators, etc.)
are needed to enable reliable monitoring of mount operations in BPF LSM.
Thanks,
Song
PS: I am not sure whether other folks are already working on it. I will prepare
some RFC patches before the conference if I don't see other proposals.
[1] https://lore.kernel.org/bpf/20251130064609.GR3538@ZenIV/
[2] https://lore.kernel.org/linux-security-module/20250711-pfirsich-worum-c408f9a14b13@brauner/
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-21 22:16 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <20260111.11c57c607174@gnoack.org>
On Sun, Jan 11, 2026 at 10:52:26PM +0100, Günther Noack wrote:
> On Tue, Dec 30, 2025 at 11:39:21AM +0100, Günther Noack wrote:
> > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > index 7961c6630a2d7..aa0efa36a37db 100644
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > [...]
> > /*
> > * Tracks domains responsible of a denied access. This is required to avoid
> > * storing in each object the full layer_masks[] required by update_request().
> > + *
> > + * Each nibble represents the layer index of the newest layer which denied a
> > + * certain access right. For file system access rights, the upper four bits are
> > + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> > + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
> > */
> > typedef u8 deny_masks_t;
>
> FYI: I left this out for now because it felt a bit out of scope (and
> transposing the layer masks was adventurous enough), but I was tempted
> to go one step further here and turn this into a struct with
> bitfields:
>
> /* A collection of layer indices denying specific access rights. */
> struct layers_denying_fs_access {
> unsigned int truncate : 4;
> unsigned int ioctl_dev : 4;
> }
>
> (Type name TBD, I am open for suggestions.)
>
> I think if we accept that this data structure is specific to FS access
> rights, we win clarity in the code. When I came across the code that
> put this together dynamically and in a more generic way, it took me a
> while to figure out what it did.
This change should help indeed (in a standalone patch). Maybe we could
make the rest of the logic more generic in a way that it would be
simpler to add this kind of access right?
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-21 22:22 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: linux-security-module, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze
In-Reply-To: <20251230103917.10549-7-gnoack3000@gmail.com>
The goal of the initial design was to minimize the amount of memory wrt
the number of different access rights because the maximum number of
layers is 16 whereas access rights could grow up to 64.
Transposing the matrix increases the memory footprint in theory but
because we still need the struct layer_access_masks matrix, it should
actually be better. See stack usage delta with audit (generated with
check-linux.sh):
landlock_unmask_layers 208 80 -128
landlock_init_layer_masks 192 96 -96
landlock_log_denial 176 80 -96
current_check_access_path 336 304 -32
current_check_refer_path 592 560 -32
hook_file_open 352 320 -32
hook_file_send_sigiotask 176 160 -16
hook_file_truncate 112 96 -16
hook_move_mount 128 112 -16
hook_ptrace_access_check 192 176 -16
hook_ptrace_traceme 160 144 -16
hook_sb_mount 128 112 -16
hook_sb_pivotroot 128 112 -16
hook_sb_remount 128 112 -16
hook_sb_umount 128 112 -16
hook_task_kill 176 160 -16
current_check_access_socket 336 352 +16
is_access_to_paths_allowed 384 400 +16
...and stack usage delta without audit:
landlock_unmask_layers 208 80 -128
landlock_init_layer_masks 192 96 -96
hook_file_open 208 192 -16
current_check_access_socket 176 208 +32
However, when we'll add the next access right, access_mask_t will be u32
instead of u16, and stack usage delta will increase:
current_check_access_socket 352 384 +32
hook_file_open 320 352 +32
current_check_access_path 304 352 +48
current_check_refer_path 560 608 +48
is_access_to_paths_allowed 400 464 +64
Even if the cumulative stack usage delta seems reasonable, the commit
message should talk about these drawbacks.
I think the improved compiled code, and the overall simplification are
worth it.
On Tue, Dec 30, 2025 at 11:39:21AM +0100, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operations security check. It stores one bit
operation?
> for each combination of access right and layer index. If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
>
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t. The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
>
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
>
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
>
> * We can use bitwise operations in multiple places where we previously
> looped over bits individually with macros. (Should require less
> branch speculation)
>
> * Code is ~160 lines smaller.
>
> Other noteworthy changes:
>
> * Clarify deny_mask_t and the code assembling it.
> * Document what that value looks like
> * Make writing and reading functions specific to file system rules.
> (It only worked for FS rules before as well, but going all the way
> simplifies the code logic more.)
> * In no_more_access(), call a new helper function may_refer(), which
> only solves the asymmetric case. Previously, the code interleaved
> the checks for the two symmetric cases in RENAME_EXCHANGE. It feels
> that the code is clearer when renames without RENAME_EXCHANGE are
> more obviously the normal case.
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 10 +-
> security/landlock/audit.c | 155 ++++++----------
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 120 +++----------
> security/landlock/domain.h | 6 +-
> security/landlock/fs.c | 350 ++++++++++++++++--------------------
> security/landlock/net.c | 10 +-
> security/landlock/ruleset.c | 78 +++-----
> security/landlock/ruleset.h | 18 +-
> 9 files changed, 290 insertions(+), 460 deletions(-)
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d7..aa0efa36a37db 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -61,14 +61,14 @@ union access_masks_all {
> static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> sizeof(typeof_member(union access_masks_all, all)));
>
> -typedef u16 layer_mask_t;
> -
> -/* Makes sure all layers can be checked. */
> -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> -
> /*
> * Tracks domains responsible of a denied access. This is required to avoid
> * storing in each object the full layer_masks[] required by update_request().
> + *
> + * Each nibble represents the layer index of the newest layer which denied a
> + * certain access right. For file system access rights, the upper four bits are
> + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
> */
> typedef u8 deny_masks_t;
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index c52d079cdb77b..650bd7f5cb6be 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -182,36 +182,18 @@ static void test_get_hierarchy(struct kunit *const test)
>
> static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> access_mask_t *const access_request,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> + const struct layer_access_masks *masks)
> {
> - const unsigned long access_req = *access_request;
> - unsigned long access_bit;
> - access_mask_t missing = 0;
> - long youngest_layer = -1;
> -
> - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> - const access_mask_t mask = (*layer_masks)[access_bit];
> - long layer;
> -
> - if (!mask)
> - continue;
> -
> - /* __fls(1) == 0 */
> - layer = __fls(mask);
> - if (layer > youngest_layer) {
> - youngest_layer = layer;
> - missing = BIT(access_bit);
> - } else if (layer == youngest_layer) {
> - missing |= BIT(access_bit);
> + for (int i = LANDLOCK_MAX_NUM_LAYERS - 1; i >= 0; i--) {
All the loop indexes should be size_t (same as before).
Instead of LANDLOCK_MAX_NUM_LAYERS, ARRAY_SIZE(masks->access) would be
better.
> + if (masks->access[i] & *access_request) {
> + *access_request &= masks->access[i];
> + return i;
> }
> }
>
> - *access_request = missing;
> - if (youngest_layer == -1)
> - return domain->num_layers - 1;
> -
> - return youngest_layer;
> + /* Not found - fall back to default values */
> + *access_request = 0;
> + return domain->num_layers - 1;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -221,94 +203,82 @@ static void test_get_denied_layer(struct kunit *const test)
> const struct landlock_ruleset dom = {
> .num_layers = 5,
> };
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
> };
> access_mask_t access;
>
> access = LANDLOCK_ACCESS_FS_EXECUTE;
> - KUNIT_EXPECT_EQ(test, 0,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
>
> access = LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_READ_FILE |
> LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> - KUNIT_EXPECT_EQ(test, 4,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, 0);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
>
> -static size_t
> -get_layer_from_deny_masks(access_mask_t *const access_request,
> - const access_mask_t all_existing_optional_access,
> - const deny_masks_t deny_masks)
> +/*
> + * get_layer_from_fs_deny_masks - get the layer which denied the access request
> + *
> + * As a side effect, stores the denied access rights from that layer(!) in
> + * *access_request.
> + */
> +static size_t get_layer_from_fs_deny_masks(access_mask_t *const access_request,
> + const deny_masks_t deny_masks)
I'm not a fan of this change. We come from a generic approach to a
specific and hardcoded one. This is simpler *for now*, but could we get
a better implementation?
Anyway, please create at least a dedicated patch for the
non-transposition changes.
> {
> - const unsigned long access_opt = all_existing_optional_access;
> - const unsigned long access_req = *access_request;
> - access_mask_t missing = 0;
> + const access_mask_t access_req = *access_request;
> size_t youngest_layer = 0;
> - size_t access_index = 0;
> - unsigned long access_bit;
> + access_mask_t missing = 0;
>
> - /* This will require change with new object types. */
> - WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
> + WARN_ON_ONCE((access_req | _LANDLOCK_ACCESS_FS_OPTIONAL) !=
> + _LANDLOCK_ACCESS_FS_OPTIONAL);
>
> - for_each_set_bit(access_bit, &access_opt,
> - BITS_PER_TYPE(access_mask_t)) {
> - if (access_req & BIT(access_bit)) {
> - const size_t layer =
> - (deny_masks >> (access_index * 4)) &
> - (LANDLOCK_MAX_NUM_LAYERS - 1);
> + if (access_req & LANDLOCK_ACCESS_FS_TRUNCATE) {
> + size_t layer = deny_masks & 0x0f;
>
> - if (layer > youngest_layer) {
> - youngest_layer = layer;
> - missing = BIT(access_bit);
> - } else if (layer == youngest_layer) {
> - missing |= BIT(access_bit);
> - }
> - }
> - access_index++;
> + missing |= LANDLOCK_ACCESS_FS_TRUNCATE;
> + youngest_layer = max(youngest_layer, layer);
> }
Add a new line.
> + if (access_req & LANDLOCK_ACCESS_FS_IOCTL_DEV) {
> + size_t layer = (deny_masks & 0xf0) >> 4;
>
> + if (layer > youngest_layer)
> + missing = 0;
> +
> + missing |= LANDLOCK_ACCESS_FS_IOCTL_DEV;
> + youngest_layer = max(youngest_layer, layer);
> + }
Add a new line.
> *access_request = missing;
> return youngest_layer;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
>
> -static void test_get_layer_from_deny_masks(struct kunit *const test)
> +static void test_get_layer_from_fs_deny_masks(struct kunit *const test)
> {
> deny_masks_t deny_mask;
> access_mask_t access;
> @@ -318,16 +288,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE;
> KUNIT_EXPECT_EQ(test, 0,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
> KUNIT_EXPECT_EQ(test, 2,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
>
> /* truncate:15 ioctl_dev:15 */
> @@ -335,16 +301,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE;
> KUNIT_EXPECT_EQ(test, 15,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
>
> access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
> KUNIT_EXPECT_EQ(test, 15,
> - get_layer_from_deny_masks(&access,
> - _LANDLOCK_ACCESS_FS_OPTIONAL,
> - deny_mask));
> + get_layer_from_fs_deny_masks(&access, deny_mask));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_TRUNCATE |
> LANDLOCK_ACCESS_FS_IOCTL_DEV);
> @@ -361,18 +323,15 @@ static bool is_valid_request(const struct landlock_request *const request)
> return false;
>
> if (request->access) {
> - if (WARN_ON_ONCE(!(!!request->layer_masks ^
> + if (WARN_ON_ONCE(!(!!request->masks ^
> !!request->all_existing_optional_access)))
> return false;
> } else {
> - if (WARN_ON_ONCE(request->layer_masks ||
> + if (WARN_ON_ONCE(request->masks ||
> request->all_existing_optional_access))
> return false;
> }
>
> - if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
> - return false;
> -
> if (request->deny_masks) {
> if (WARN_ON_ONCE(!request->all_existing_optional_access))
> return false;
> @@ -405,14 +364,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> missing = request->access;
> if (missing) {
> /* Gets the nearest domain that denies the request. */
> - if (request->layer_masks) {
> + if (request->masks) {
> youngest_layer = get_denied_layer(
> - subject->domain, &missing, request->layer_masks,
> - request->layer_masks_size);
> + subject->domain, &missing, request->masks);
> } else {
> - youngest_layer = get_layer_from_deny_masks(
> - &missing, request->all_existing_optional_access,
> - request->deny_masks);
> + youngest_layer = get_layer_from_fs_deny_masks(
> + &missing, request->deny_masks);
> }
> youngest_denied =
> get_hierarchy(subject->domain, youngest_layer);
> @@ -507,7 +464,7 @@ static struct kunit_case test_cases[] = {
> /* clang-format off */
> KUNIT_CASE(test_get_hierarchy),
> KUNIT_CASE(test_get_denied_layer),
> - KUNIT_CASE(test_get_layer_from_deny_masks),
> + KUNIT_CASE(test_get_layer_from_fs_deny_masks),
> {}
> /* clang-format on */
> };
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d80..104472060ef5e 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -43,8 +43,7 @@ struct landlock_request {
> access_mask_t access;
>
> /* Required fields for requests with layer masks. */
> - const layer_mask_t (*layer_masks)[];
> - size_t layer_masks_size;
> + const struct layer_access_masks *masks;
>
> /* Required fields for requests with deny masks. */
> const access_mask_t all_existing_optional_access;
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index a647b68e8d060..e8e4ae5d075fe 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -23,6 +23,7 @@
> #include "common.h"
> #include "domain.h"
> #include "id.h"
> +#include "limits.h"
>
> #ifdef CONFIG_AUDIT
>
> @@ -133,111 +134,47 @@ int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
> return 0;
> }
>
> -static deny_masks_t
> -get_layer_deny_mask(const access_mask_t all_existing_optional_access,
> - const unsigned long access_bit, const size_t layer)
> -{
> - unsigned long access_weight;
> -
> - /* This may require change with new object types. */
> - WARN_ON_ONCE(all_existing_optional_access !=
> - _LANDLOCK_ACCESS_FS_OPTIONAL);
> -
> - if (WARN_ON_ONCE(layer >= LANDLOCK_MAX_NUM_LAYERS))
> - return 0;
> -
> - access_weight = hweight_long(all_existing_optional_access &
> - GENMASK(access_bit, 0));
> - if (WARN_ON_ONCE(access_weight < 1))
> - return 0;
> -
> - return layer
> - << ((access_weight - 1) * HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1));
> -}
> -
> -#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> -
> -static void test_get_layer_deny_mask(struct kunit *const test)
> -{
> - const unsigned long truncate = BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE);
> - const unsigned long ioctl_dev = BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV);
> -
> - KUNIT_EXPECT_EQ(test, 0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - truncate, 0));
> - KUNIT_EXPECT_EQ(test, 0x3,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - truncate, 3));
> -
> - KUNIT_EXPECT_EQ(test, 0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - ioctl_dev, 0));
> - KUNIT_EXPECT_EQ(test, 0xf0,
> - get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - ioctl_dev, 15));
> -}
> -
> -#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> -
> deny_masks_t
> -landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> - const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> +landlock_get_fs_deny_masks(const access_mask_t optional_access,
> + const struct layer_access_masks *layer_masks)
> {
> - const unsigned long access_opt = optional_access;
> - unsigned long access_bit;
> - deny_masks_t deny_masks = 0;
> + u8 truncate_layer = 0;
> + u8 ioctl_dev_layer = 0;
>
> - /* This may require change with new object types. */
> - WARN_ON_ONCE(access_opt !=
> - (optional_access & all_existing_optional_access));
> -
> - if (WARN_ON_ONCE(!layer_masks))
> - return 0;
> -
> - if (WARN_ON_ONCE(!access_opt))
> - return 0;
> -
> - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> - const layer_mask_t mask = (*layer_masks)[access_bit];
> -
> - if (!mask)
> - continue;
> -
> - /* __fls(1) == 0 */
> - deny_masks |= get_layer_deny_mask(all_existing_optional_access,
> - access_bit, __fls(mask));
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
size_t i
ARRAY_SIZE(masks->access)
> + if (layer_masks->access[i] & optional_access &
> + LANDLOCK_ACCESS_FS_TRUNCATE)
> + truncate_layer = i;
> + if (layer_masks->access[i] & optional_access &
> + LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + ioctl_dev_layer = i;
> }
> - return deny_masks;
> + return ((ioctl_dev_layer << 4) & 0xf0) | (truncate_layer & 0x0f);
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
>
> -static void test_landlock_get_deny_masks(struct kunit *const test)
> +static void test_landlock_get_fs_deny_masks(struct kunit *const test)
> {
> - const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(9),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
> - BIT_ULL(0),
> + const struct layer_access_masks layers1 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
> + .access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
>
> KUNIT_EXPECT_EQ(test, 0x1,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_TRUNCATE,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE,
> + &layers1));
> KUNIT_EXPECT_EQ(test, 0x20,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + &layers1));
> KUNIT_EXPECT_EQ(
> test, 0x21,
> - landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> - LANDLOCK_ACCESS_FS_TRUNCATE |
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE |
> + LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + &layers1));
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -246,8 +183,7 @@ static void test_landlock_get_deny_masks(struct kunit *const test)
>
> static struct kunit_case test_cases[] = {
> /* clang-format off */
> - KUNIT_CASE(test_get_layer_deny_mask),
> - KUNIT_CASE(test_landlock_get_deny_masks),
> + KUNIT_CASE(test_landlock_get_fs_deny_masks),
> {}
> /* clang-format on */
> };
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 7fb70b25f85a1..39600acb63897 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -120,10 +120,8 @@ struct landlock_hierarchy {
> #ifdef CONFIG_AUDIT
>
> deny_masks_t
> -landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> - const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - size_t layer_masks_size);
> +landlock_get_fs_deny_masks(const access_mask_t optional_access,
> + const struct layer_access_masks *layer_masks);
>
> int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index b4ce03bef4b8e..1e765d22d8d49 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -407,57 +407,55 @@ static bool access_mask_subset(access_mask_t a, access_mask_t b)
> return (a | b) == b;
> }
>
> +/*
> + * Returns true iff the child file with the given src_child access rights under
> + * src_parent would result in having the same or fewer access rights if it were
> + * moved under new_parent.
> + */
> +static bool may_refer(const struct layer_access_masks *const src_parent,
> + const struct layer_access_masks *const src_child,
> + const struct layer_access_masks *const new_parent,
> + const bool child_is_dir)
> +{
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
size_t i
ARRAY_SIZE(src_child->access)
> + access_mask_t child_access = src_parent->access[i] &
> + src_child->access[i];
> + access_mask_t parent_access = new_parent->access[i];
> +
> + if (!child_is_dir) {
> + child_access &= ACCESS_FILE;
> + parent_access &= ACCESS_FILE;
> + }
> +
> + if (!access_mask_subset(child_access, parent_access))
> + return false;
> + }
> + return true;
> +}
> +
> /*
> * Check that a destination file hierarchy has more restrictions than a source
> * file hierarchy. This is only used for link and rename actions.
> *
> - * @layer_masks_child2: Optional child masks.
> + * Returns: true if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights. If child2 is set, an additional condition is
> + * that child2 may be used from parent2 to parent1 without increasing its access
> + * rights.
> */
> -static bool no_more_access(
> - const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child1_is_directory,
> - const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child2_is_directory)
> +static bool no_more_access(const struct layer_access_masks *const parent1,
> + const struct layer_access_masks *const child1,
> + const bool child1_is_dir,
> + const struct layer_access_masks *const parent2,
> + const struct layer_access_masks *const child2,
> + const bool child2_is_dir)
> {
> - unsigned long access_bit;
> + if (!may_refer(parent1, child1, parent2, child1_is_dir))
> + return false;
>
> - for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
> - access_bit++) {
> - /* Ignores accesses that only make sense for directories. */
> - const bool is_file_access =
> - !!(BIT_ULL(access_bit) & ACCESS_FILE);
> + if (!child2)
> + return true;
>
> - if (child1_is_directory || is_file_access) {
> - /*
> - * Checks if the destination restrictions are a
> - * superset of the source ones (i.e. inherited access
> - * rights without child exceptions):
> - * restrictions(parent2) >= restrictions(child1)
> - */
> - if ((((*layer_masks_parent1)[access_bit] &
> - (*layer_masks_child1)[access_bit]) |
> - (*layer_masks_parent2)[access_bit]) !=
> - (*layer_masks_parent2)[access_bit])
> - return false;
> - }
> -
> - if (!layer_masks_child2)
> - continue;
> - if (child2_is_directory || is_file_access) {
> - /*
> - * Checks inverted restrictions for RENAME_EXCHANGE:
> - * restrictions(parent1) >= restrictions(child2)
> - */
> - if ((((*layer_masks_parent2)[access_bit] &
> - (*layer_masks_child2)[access_bit]) |
> - (*layer_masks_parent1)[access_bit]) !=
> - (*layer_masks_parent1)[access_bit])
> - return false;
> - }
> - }
> - return true;
> + return may_refer(parent2, child2, parent1, child2_is_dir);
> }
>
> #define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
> @@ -467,25 +465,25 @@ static bool no_more_access(
>
> static void test_no_more_access(struct kunit *const test)
> {
> - const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
> + const struct layer_access_masks rx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
> + const struct layer_access_masks mx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_MAKE_REG,
> };
> - const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> + const struct layer_access_masks x0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
> + const struct layer_access_masks x1 = {
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(1),
> + const struct layer_access_masks x01 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
> + const struct layer_access_masks allows_all = {};
>
> /* Checks without restriction. */
> NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
> @@ -573,31 +571,30 @@ static void test_no_more_access(struct kunit *const test)
> #undef NMA_TRUE
> #undef NMA_FALSE
>
> -static bool is_layer_masks_allowed(
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
> {
> - return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
> + return !memchr_inv(&masks->access, 0, sizeof(masks->access));
> }
>
> /*
> - * Removes @layer_masks accesses that are not requested.
> + * Removes @masks accesses that are not requested.
> *
> * Returns true if the request is allowed, false otherwise.
> */
> -static bool
> -scope_to_request(const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool scope_to_request(const access_mask_t access_request,
> + struct layer_access_masks *masks)
> {
> - const unsigned long access_req = access_request;
> - unsigned long access_bit;
> + bool saw_unfulfilled_access = false;
>
> - if (WARN_ON_ONCE(!layer_masks))
> + if (WARN_ON_ONCE(!masks))
> return true;
>
> - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> - (*layer_masks)[access_bit] = 0;
> -
> - return is_layer_masks_allowed(layer_masks);
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
size_t i
ARRAY_SIZE(masks->access)
> + masks->access[i] &= access_request;
> + if (masks->access[i])
> + saw_unfulfilled_access = true;
> + }
> + return !saw_unfulfilled_access;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -605,48 +602,41 @@ scope_to_request(const access_mask_t access_request,
> static void test_scope_to_request_with_exec_none(struct kunit *const test)
> {
> /* Allows everything. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks masks = {};
>
> /* Checks and scopes with execute. */
> - KUNIT_EXPECT_TRUE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
> - &layer_masks));
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + KUNIT_EXPECT_TRUE(test,
> + scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE, &masks));
> + KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
> }
>
> static void test_scope_to_request_with_exec_some(struct kunit *const test)
> {
> /* Denies execute and write. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
> + struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> /* Checks and scopes with execute. */
> KUNIT_EXPECT_FALSE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
> - &layer_masks));
> - KUNIT_EXPECT_EQ(test, BIT_ULL(0),
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + &masks));
> + KUNIT_EXPECT_EQ(test, LANDLOCK_ACCESS_FS_EXECUTE, masks.access[0]);
> + KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
> }
>
> static void test_scope_to_request_without_access(struct kunit *const test)
> {
> /* Denies execute and write. */
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
> + struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> /* Checks and scopes without access request. */
> - KUNIT_EXPECT_TRUE(test, scope_to_request(0, &layer_masks));
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
> - KUNIT_EXPECT_EQ(test, 0,
> - layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
> + KUNIT_EXPECT_TRUE(test, scope_to_request(0, &masks));
> + KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
> + KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -655,20 +645,16 @@ static void test_scope_to_request_without_access(struct kunit *const test)
> * Returns true if there is at least one access right different than
> * LANDLOCK_ACCESS_FS_REFER.
> */
> -static bool
> -is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
> - const access_mask_t access_request)
> +static bool is_eacces(const struct layer_access_masks *masks,
> + const access_mask_t access_request)
> {
> - unsigned long access_bit;
> - /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
> - const unsigned long access_check = access_request &
> - ~LANDLOCK_ACCESS_FS_REFER;
> -
> - if (!layer_masks)
> + if (!masks)
> return false;
>
> - for_each_set_bit(access_bit, &access_check, ARRAY_SIZE(*layer_masks)) {
> - if ((*layer_masks)[access_bit])
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
size_t i
ARRAY_SIZE(masks->access)
> + /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
> + if (masks->access[i] & access_request &
> + ~LANDLOCK_ACCESS_FS_REFER)
> return true;
> }
> return false;
> @@ -681,37 +667,37 @@ is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
>
> static void test_is_eacces_with_none(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + const struct layer_access_masks masks = {};
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> static void test_is_eacces_with_refer(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = BIT_ULL(0),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_REFER,
> };
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> static void test_is_eacces_with_write(struct kunit *const test)
> {
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(0),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_WRITE_FILE,
> };
>
> - IE_FALSE(&layer_masks, 0);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
> - IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
> + IE_FALSE(&masks, 0);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
> + IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
>
> - IE_TRUE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> + IE_TRUE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -761,26 +747,25 @@ static void test_is_eacces_with_write(struct kunit *const test)
> * - true if the access request is granted;
> * - false otherwise.
> */
> -static bool is_access_to_paths_allowed(
> - const struct landlock_ruleset *const domain,
> - const struct path *const path,
> - const access_mask_t access_request_parent1,
> - layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> - struct landlock_request *const log_request_parent1,
> - struct dentry *const dentry_child1,
> - const access_mask_t access_request_parent2,
> - layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> - struct landlock_request *const log_request_parent2,
> - struct dentry *const dentry_child2)
> +static bool
> +is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> + const struct path *const path,
> + const access_mask_t access_request_parent1,
> + struct layer_access_masks *layer_masks_parent1,
> + struct landlock_request *const log_request_parent1,
> + struct dentry *const dentry_child1,
> + const access_mask_t access_request_parent2,
> + struct layer_access_masks *layer_masks_parent2,
> + struct landlock_request *const log_request_parent2,
> + struct dentry *const dentry_child2)
> {
> bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
> child1_is_directory = true, child2_is_directory = true;
> struct path walker_path;
> access_mask_t access_masked_parent1, access_masked_parent2;
> - layer_mask_t _layer_masks_child1[LANDLOCK_NUM_ACCESS_FS],
> - _layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
> - layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
> - (*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
> + struct layer_access_masks _layer_masks_child1, _layer_masks_child2;
> + struct layer_access_masks *layer_masks_child1 = NULL,
> + *layer_masks_child2 = NULL;
>
> if (!access_request_parent1 && !access_request_parent2)
> return true;
> @@ -820,22 +805,20 @@ static bool is_access_to_paths_allowed(
> }
>
> if (unlikely(dentry_child1)) {
> - landlock_unmask_layers(
> - find_rule(domain, dentry_child1),
> - landlock_init_layer_masks(
> - domain, LANDLOCK_MASK_ACCESS_FS,
> - &_layer_masks_child1, LANDLOCK_KEY_INODE),
> - &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
> + if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
> + &_layer_masks_child1,
> + LANDLOCK_KEY_INODE))
> + landlock_unmask_layers(find_rule(domain, dentry_child1),
> + &_layer_masks_child1);
> layer_masks_child1 = &_layer_masks_child1;
> child1_is_directory = d_is_dir(dentry_child1);
> }
> if (unlikely(dentry_child2)) {
> - landlock_unmask_layers(
> - find_rule(domain, dentry_child2),
> - landlock_init_layer_masks(
> - domain, LANDLOCK_MASK_ACCESS_FS,
> - &_layer_masks_child2, LANDLOCK_KEY_INODE),
> - &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
> + if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
> + &_layer_masks_child2,
> + LANDLOCK_KEY_INODE))
> + landlock_unmask_layers(find_rule(domain, dentry_child2),
> + &_layer_masks_child2);
> layer_masks_child2 = &_layer_masks_child2;
> child2_is_directory = d_is_dir(dentry_child2);
> }
> @@ -890,16 +873,12 @@ static bool is_access_to_paths_allowed(
> }
>
> rule = find_rule(domain, walker_path.dentry);
> - allowed_parent1 = allowed_parent1 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent1,
> - layer_masks_parent1,
> - ARRAY_SIZE(*layer_masks_parent1));
> - allowed_parent2 = allowed_parent2 ||
> - landlock_unmask_layers(
> - rule, access_masked_parent2,
> - layer_masks_parent2,
> - ARRAY_SIZE(*layer_masks_parent2));
> + allowed_parent1 =
> + allowed_parent1 ||
> + landlock_unmask_layers(rule, layer_masks_parent1);
> + allowed_parent2 =
> + allowed_parent2 ||
> + landlock_unmask_layers(rule, layer_masks_parent2);
>
> /* Stops when a rule from each layer grants access. */
> if (allowed_parent1 && allowed_parent2)
> @@ -953,9 +932,7 @@ static bool is_access_to_paths_allowed(
> log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent1->audit.u.path = *path;
> log_request_parent1->access = access_masked_parent1;
> - log_request_parent1->layer_masks = layer_masks_parent1;
> - log_request_parent1->layer_masks_size =
> - ARRAY_SIZE(*layer_masks_parent1);
> + log_request_parent1->masks = layer_masks_parent1;
> }
>
> if (!allowed_parent2) {
> @@ -963,9 +940,7 @@ static bool is_access_to_paths_allowed(
> log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
> log_request_parent2->audit.u.path = *path;
> log_request_parent2->access = access_masked_parent2;
> - log_request_parent2->layer_masks = layer_masks_parent2;
> - log_request_parent2->layer_masks_size =
> - ARRAY_SIZE(*layer_masks_parent2);
> + log_request_parent2->masks = layer_masks_parent2;
> }
> return allowed_parent1 && allowed_parent2;
> }
> @@ -978,7 +953,7 @@ static int current_check_access_path(const struct path *const path,
> };
> const struct landlock_cred_security *const subject =
> landlock_get_applicable_subject(current_cred(), masks, NULL);
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks;
> struct landlock_request request = {};
>
> if (!subject)
> @@ -1053,10 +1028,10 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> * - true if all the domain access rights are allowed for @dir;
> * - false if the walk reached @mnt_root.
> */
> -static bool collect_domain_accesses(
> - const struct landlock_ruleset *const domain,
> - const struct dentry *const mnt_root, struct dentry *dir,
> - layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
> +static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> + const struct dentry *const mnt_root,
> + struct dentry *dir,
> + struct layer_access_masks *layer_masks_dom)
> {
> unsigned long access_dom;
> bool ret = false;
> @@ -1075,9 +1050,8 @@ static bool collect_domain_accesses(
> struct dentry *parent_dentry;
>
> /* Gets all layers allowing all domain accesses. */
> - if (landlock_unmask_layers(find_rule(domain, dir), access_dom,
> - layer_masks_dom,
> - ARRAY_SIZE(*layer_masks_dom))) {
> + if (landlock_unmask_layers(find_rule(domain, dir),
> + layer_masks_dom)) {
> /*
> * Stops when all handled accesses are allowed by at
> * least one rule in each layer.
> @@ -1165,8 +1139,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> access_mask_t access_request_parent1, access_request_parent2;
> struct path mnt_dir;
> struct dentry *old_parent;
> - layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
> - layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks_parent1 = {},
> + layer_masks_parent2 = {};
> struct landlock_request request1 = {}, request2 = {};
>
> if (!subject)
> @@ -1323,7 +1297,8 @@ static void hook_sb_delete(struct super_block *const sb)
> * second call to iput() for the same Landlock object. Also
> * checks I_NEW because such inode cannot be tied to an object.
> */
> - if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) {
> + if (inode_state_read(inode) &
> + (I_FREEING | I_WILL_FREE | I_NEW)) {
> spin_unlock(&inode->i_lock);
> continue;
> }
> @@ -1641,7 +1616,7 @@ static bool is_device(const struct file *const file)
>
> static int hook_file_open(struct file *const file)
> {
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct layer_access_masks layer_masks = {};
> access_mask_t open_access_request, full_access_request, allowed_access,
> optional_access;
> const struct landlock_cred_security *const subject =
> @@ -1676,20 +1651,14 @@ static int hook_file_open(struct file *const file)
> &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
> allowed_access = full_access_request;
> } else {
> - unsigned long access_bit;
> - const unsigned long access_req = full_access_request;
> -
> /*
> * Calculate the actual allowed access rights from layer_masks.
> - * Add each access right to allowed_access which has not been
> - * vetoed by any layer.
> + * Remove the access rights from the full access request which
> + * are still unfulfilled in any of the layers.
> */
> - allowed_access = 0;
> - for_each_set_bit(access_bit, &access_req,
> - ARRAY_SIZE(layer_masks)) {
> - if (!layer_masks[access_bit])
> - allowed_access |= BIT_ULL(access_bit);
> - }
> + allowed_access = full_access_request;
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++)
size_t i
ARRAY_SIZE(layer_masks.access)
> + allowed_access &= ~layer_masks.access[i];
> }
>
> /*
> @@ -1700,9 +1669,8 @@ static int hook_file_open(struct file *const file)
> */
> landlock_file(file)->allowed_access = allowed_access;
> #ifdef CONFIG_AUDIT
> - landlock_file(file)->deny_masks = landlock_get_deny_masks(
> - _LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
> - ARRAY_SIZE(layer_masks));
> + landlock_file(file)->deny_masks =
> + landlock_get_fs_deny_masks(optional_access, &layer_masks);
> #endif /* CONFIG_AUDIT */
>
> if (access_mask_subset(open_access_request, allowed_access))
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 1f3915a90a808..2a5456f4f017e 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -47,7 +47,7 @@ static int current_check_access_socket(struct socket *const sock,
> access_mask_t access_request)
> {
> __be16 port;
> - layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
> + struct layer_access_masks layer_masks = {};
> const struct landlock_rule *rule;
> struct landlock_id id = {
> .type = LANDLOCK_KEY_NET_PORT,
> @@ -178,8 +178,9 @@ static int current_check_access_socket(struct socket *const sock,
> access_request = landlock_init_layer_masks(subject->domain,
> access_request, &layer_masks,
> LANDLOCK_KEY_NET_PORT);
> - if (landlock_unmask_layers(rule, access_request, &layer_masks,
> - ARRAY_SIZE(layer_masks)))
> + if (!access_request)
> + return 0;
Add a new line.
> + if (landlock_unmask_layers(rule, &layer_masks))
> return 0;
>
> audit_net.family = address->sa_family;
> @@ -189,8 +190,7 @@ static int current_check_access_socket(struct socket *const sock,
> .audit.type = LSM_AUDIT_DATA_NET,
> .audit.u.net = &audit_net,
> .access = access_request,
> - .layer_masks = &layer_masks,
> - .layer_masks_size = ARRAY_SIZE(layer_masks),
> + .masks = &layer_masks,
> });
> return -EACCES;
> }
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index dfcdc19ea2683..d20e28d38e9c9 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -622,49 +622,24 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
> * request are empty).
> */
The above doc should be updated with the new arguments. You can also
convert it to a proper function docstring (which would have detected
this issue).
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> - const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> - const size_t masks_array_size)
> + struct layer_access_masks *masks)
> {
> - size_t layer_level;
> -
> - if (!access_request || !layer_masks)
> + if (!masks)
> return true;
> if (!rule)
> return false;
>
> - /*
> - * An access is granted if, for each policy layer, at least one rule
> - * encountered on the pathwalk grants the requested access,
> - * regardless of its position in the layer stack. We must then check
> - * the remaining layers for each inode, from the first added layer to
> - * the last one. When there is multiple requested accesses, for each
> - * policy layer, the full set of requested accesses may not be granted
> - * by only one rule, but by the union (binary OR) of multiple rules.
> - * E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
> - */
I'd like to keep most of this comment, even if some parts are specific
to FS access, I think it helps understand the logic.
> - for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
> - const struct landlock_layer *const layer =
> - &rule->layers[layer_level];
> - const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> - const unsigned long access_req = access_request;
> - unsigned long access_bit;
> - bool is_empty;
> + for (int i = 0; i < rule->num_layers; i++) {
size_t i
> + const struct landlock_layer *l = &rule->layers[i];
... *const layer = ...
>
> - /*
> - * Records in @layer_masks which layer grants access to each requested
> - * access: bit cleared if the related layer grants access.
> - */
At least the "bit cleared if the related layer grants access" comment
should be kept.
> - is_empty = true;
> - for_each_set_bit(access_bit, &access_req, masks_array_size) {
> - if (layer->access & BIT_ULL(access_bit))
> - (*layer_masks)[access_bit] &= ~layer_bit;
> - is_empty = is_empty && !(*layer_masks)[access_bit];
> - }
> - if (is_empty)
> - return true;
> + masks->access[l->level - 1] &= ~l->access;
It's indeed better to get rid of the requested access in this helper.
> }
> - return false;
> +
> + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
size_t i
ARRAY_SIZE(masks->access)
> + if (masks->access[i])
> + return false;
> + }
> + return true;
> }
>
> typedef access_mask_t
> @@ -679,8 +654,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> *
> * @domain: The domain that defines the current restrictions.
> * @access_request: The requested access rights to check.
> - * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_FS or
There is another mention @layer_masks above.
> - * %LANDLOCK_NUM_ACCESS_NET elements according to @key_type.
> + * @masks: Layer access masks to populate.
> * @key_type: The key type to switch between access masks of different types.
> *
> * Returns: An access mask where each access right bit is set which is handled
> @@ -689,23 +663,20 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> access_mask_t
> landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> + struct layer_access_masks *masks,
*const masks
> const enum landlock_key_type key_type)
> {
> access_mask_t handled_accesses = 0;
> - size_t layer_level, num_access;
> get_access_mask_t *get_access_mask;
>
> switch (key_type) {
> case LANDLOCK_KEY_INODE:
> get_access_mask = landlock_get_fs_access_mask;
> - num_access = LANDLOCK_NUM_ACCESS_FS;
> break;
>
> #if IS_ENABLED(CONFIG_INET)
> case LANDLOCK_KEY_NET_PORT:
> get_access_mask = landlock_get_net_access_mask;
> - num_access = LANDLOCK_NUM_ACCESS_NET;
> break;
> #endif /* IS_ENABLED(CONFIG_INET) */
>
> @@ -714,27 +685,18 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> return 0;
> }
>
> - memset(layer_masks, 0,
> - array_size(sizeof((*layer_masks)[0]), num_access));
> -
> /* An empty access request can happen because of O_WRONLY | O_RDWR. */
> if (!access_request)
> return 0;
>
> - /* Saves all handled accesses per layer. */
> - for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
> - const unsigned long access_req = access_request;
> - const access_mask_t access_mask =
> - get_access_mask(domain, layer_level);
> - unsigned long access_bit;
> + for (int i = 0; i < domain->num_layers; i++) {
size_t i
> + const access_mask_t handled = get_access_mask(domain, i);
>
> - for_each_set_bit(access_bit, &access_req, num_access) {
> - if (BIT_ULL(access_bit) & access_mask) {
> - (*layer_masks)[access_bit] |=
> - BIT_ULL(layer_level);
> - handled_accesses |= BIT_ULL(access_bit);
> - }
> - }
> + masks->access[i] = access_request & handled;
> + handled_accesses |= masks->access[i];
> }
> + for (int i = domain->num_layers; i < LANDLOCK_MAX_NUM_LAYERS; i++)
size_t i
ARRAY_SIZE(masks->access)
> + masks->access[i] = 0;
> +
> return handled_accesses;
> }
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 1a78cba662b24..f7b80b18c2a70 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -301,15 +301,25 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
> return ruleset->access_masks[layer_level].scope;
> }
>
> +/**
> + * struct layer_accesses - A boolean matrix of layers and access rights
> + *
> + * This has a bit for each combination of layer numbers and access rights.
> + * During access checks, it is used to represent the access rights for each
> + * layer which still need to be fulfilled. When all bits are 0, the access
> + * request is considered to be fulfilled.
> + */
> +struct layer_access_masks {
> + access_mask_t access[LANDLOCK_MAX_NUM_LAYERS];
> +};
> +
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> - const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> - const size_t masks_array_size);
> + struct layer_access_masks *masks);
>
> access_mask_t
> landlock_init_layer_masks(const struct landlock_ruleset *const domain,
> const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[],
> + struct layer_access_masks *masks,
> const enum landlock_key_type key_type);
>
> #endif /* _SECURITY_LANDLOCK_RULESET_H */
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-21 22:27 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <6a789aa9-c479-43f9-ac24-bc227f8388c6@maowtm.org>
On Wed, Jan 21, 2026 at 12:26:52AM +0000, Tingmao Wang wrote:
> On 12/30/25 10:39, Günther Noack wrote:
> > The layer masks data structure tracks the requested but unfulfilled
> > access rights during an operations security check. It stores one bit
> > for each combination of access right and layer index. If the bit is
> > set, that access right is not granted (yet) in the given layer and we
> > have to traverse the path further upwards to grant it.
> >
> > Previously, the layer masks were stored as arrays mapping from access
> > right indices to layer_mask_t. The layer_mask_t value then indicates
> > all layers in which the given access right is still (tentatively)
> > denied.
> >
> > This patch introduces struct layer_access_masks instead: This struct
> > contains an array with the access_mask_t of each (tentatively) denied
> > access right in that layer.
> >
> > The hypothesis of this patch is that this simplifies the code enough
> > so that the resulting code will run faster:
> >
> > * We can use bitwise operations in multiple places where we previously
> > looped over bits individually with macros. (Should require less
> > branch speculation)
> >
> > * Code is ~160 lines smaller.
> >
> > Other noteworthy changes:
> >
> > * Clarify deny_mask_t and the code assembling it.
> > * Document what that value looks like
> > * Make writing and reading functions specific to file system rules.
> > (It only worked for FS rules before as well, but going all the way
> > simplifies the code logic more.)
>
> In the original commit message that added this type [1] there was this
> statement:
>
> > Implementing deny_masks_t with a bitfield instead of a struct enables a
> > generic implementation to store and extract layer levels.
>
> At some point when looking at this I was wondering why this wasn't a
> struct with 2 u8:4 fields, but rather, a u8 with bit manipulation code.
> While it is possible that I might have just misunderstood it, reading the
> above statement my take-away was that a struct would have forced us to
> address the indices with specific names, e.g. it would need to be defined
> like
>
> struct deny_masks_t {
> u8 ioctl:4;
> u8 truncate:4;
> }
>
> And it would thus not be possible to manipulate the indices in a generic
> way (e.g. the way it was implemented before, given
> all_existing_optional_access and access_bit, read and write the right
> bits).
>
> However, since we're now removing that generic-ability, should we consider
> turning it into a struct? (If later on we have different access types
> that also have optional accesses, we could use a union of structs)
I would prefer to have a more generic implementation, or at least to
make it easy to add this kind of access rights. Any idea how to improve
the situation?
>
>
> btw, since this causes conflicts with the quiet flag series and Mickaël
> has indicated that this should be merged first, I will probably have to
> make my series based on top of this. Will watch this series to see if
> there are more changes.
I'd like to make sure your quiet flag series is still OK with this
patch, and what would be the impact, so yes, please review and
experiment with this series.
>
> Also, this transpose and code simplification should also simplify the
> mutable domains work so thanks for the refactor!
Good :)
>
> A while ago I also made some benchmarking script which I sent a PR to
> landlock-test-tools [2], and earlier I tested this patch with it, and saw
> some improvement (but it was much less in terms of percentage, which may
> be due to the lower directory depth, or may be due to other unknown
> reason):
>
> TestDescription(landlock=True, dir_depth=10, nb_extra_rules=10)
> base.2:
> c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=1718.15, min=1663.00, max=275949.00, median=1696.46, stddev=437.52
> 95% confidence interval: [1718.03 .. 1718.28]
> Estimated landlock overhead (vs no-landlock): 226.5%
> 48bd90e91fe6.2:
> c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=1709.60, min=1633.00, max=280608.00, median=1688.83, stddev=441.83
> 95% confidence interval: [1709.48 .. 1709.73]
> ** Improved 0.5% **
> ...
> [1660 .. 1669]: [1660 .. 1669]: ###
> [1670 .. 1679]: ## [1670 .. 1679]: ###############
> [1680 .. 1689]: ###################### [1680 .. 1689]: #################################
> [1690 .. 1699]: ######################################## [1690 .. 1699]: ##################################
> [1700 .. 1709]: ############################ [1700 .. 1709]: #############
> [1710 .. 1719]: ######### [1710 .. 1719]: ##
> [1720 .. 1729]: ## [1720 .. 1729]:
> ...
> Estimated landlock overhead (vs no-landlock): 223.0%
>
> TestDescription(landlock=True, dir_depth=29, nb_extra_rules=10)
> base.2:
> c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=3869.66, min=3727.00, max=272563.00, median=3813.42, stddev=666.18
> 95% confidence interval: [3869.47 .. 3869.86]
> Estimated landlock overhead (vs no-landlock): 427.3%
> 48bd90e91fe6.2:
> c_measured_syscall_time_ns: 45000000 samples (3 trials), avg=3855.61, min=3697.00, max=271690.00, median=3804.82, stddev=682.74
> 95% confidence interval: [3855.41 .. 3855.81]
> ** Improved 0.4% **
> ...
> [3750 .. 3759]: [3750 .. 3759]: #
> [3760 .. 3769]: [3760 .. 3769]: #######
> [3770 .. 3779]: [3770 .. 3779]: ###############
> [3780 .. 3789]: #### [3780 .. 3789]: ###################
> [3790 .. 3799]: ################### [3790 .. 3799]: ###################
> [3800 .. 3809]: ###################################### [3800 .. 3809]: ########################
> [3810 .. 3819]: ######################################## [3810 .. 3819]: ############################
> [3820 .. 3829]: ########################## [3820 .. 3829]: #####################
> [3830 .. 3839]: ############# [3830 .. 3839]: #########
> [3840 .. 3849]: ###### [3840 .. 3849]: ##
> [3850 .. 3859]: ## [3850 .. 3859]:
> [3860 .. 3869]: [3860 .. 3869]:
> [3870 .. 3879]: [3870 .. 3879]:
> ...
> [4980 .. 4989]: [4980 .. 4989]:
> [4990 .. 4999]: [4990 .. 4999]:
> [5000 .. 272563]: # [5000 .. 271690]: #
> Estimated landlock overhead (vs no-landlock): 424.2%
Thanks for the benchmark.
>
> Full data including test with 0 depth, or 1000 rules:
> https://fileshare.maowtm.org/landlock-20251230/index.html
>
>
> [1]: https://lore.kernel.org/all/20250320190717.2287696-15-mic@digikod.net/
> [2]: https://github.com/landlock-lsm/landlock-test-tools/pull/17
>
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Justin Suess @ 2026-01-21 23:08 UTC (permalink / raw)
To: Tingmao Wang, Günther Noack, Mickaël Salaün
Cc: linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze
In-Reply-To: <6a789aa9-c479-43f9-ac24-bc227f8388c6@maowtm.org>
On 1/20/26 19:26, Tingmao Wang wrote:
> On 12/30/25 10:39, Günther Noack wrote:
>> The layer masks data structure tracks the requested but unfulfilled
>> access rights during an operations security check. It stores one bit
>> for each combination of access right and layer index. If the bit is
>> set, that access right is not granted (yet) in the given layer and we
>> have to traverse the path further upwards to grant it.
>>
>> Previously, the layer masks were stored as arrays mapping from access
>> right indices to layer_mask_t. The layer_mask_t value then indicates
>> all layers in which the given access right is still (tentatively)
>> denied.
>>
>> This patch introduces struct layer_access_masks instead: This struct
>> contains an array with the access_mask_t of each (tentatively) denied
>> access right in that layer.
>>
>> The hypothesis of this patch is that this simplifies the code enough
>> so that the resulting code will run faster:
>>
>> * We can use bitwise operations in multiple places where we previously
>> looped over bits individually with macros. (Should require less
>> branch speculation)
>>
>> * Code is ~160 lines smaller.
>>
>> Other noteworthy changes:
>>
>> * Clarify deny_mask_t and the code assembling it.
>> * Document what that value looks like
>> * Make writing and reading functions specific to file system rules.
>> (It only worked for FS rules before as well, but going all the way
>> simplifies the code logic more.)
> In the original commit message that added this type [1] there was this
> statement:
>
>> Implementing deny_masks_t with a bitfield instead of a struct enables a
>> generic implementation to store and extract layer levels.
> At some point when looking at this I was wondering why this wasn't a
> struct with 2 u8:4 fields, but rather, a u8 with bit manipulation code.
> While it is possible that I might have just misunderstood it, reading the
> above statement my take-away was that a struct would have forced us to
> address the indices with specific names, e.g. it would need to be defined
> like
>
> struct deny_masks_t {
> u8 ioctl:4;
> u8 truncate:4;
> }
>
> And it would thus not be possible to manipulate the indices in a generic
> way (e.g. the way it was implemented before, given
> all_existing_optional_access and access_bit, read and write the right
> bits).
>
> However, since we're now removing that generic-ability, should we consider
> turning it into a struct? (If later on we have different access types
> that also have optional accesses, we could use a union of structs)
>
>
> btw, since this causes conflicts with the quiet flag series and Mickaël
> has indicated that this should be merged first, I will probably have to
> make my series based on top of this. Will watch this series to see if
> there are more changes.
Likewise for my NO_INHERIT series, which will need some rebase work as
well. (my series is built on the quiet flag series, to reuse the similar "bubble up"
flag collection logic).
I'll keep an eye on your tree Tingmao and start rebasing my NO_INHERIT
on your patches if you put your work there. (Otherwise I'll do it when you
send it on the mailing list)
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC] Refactor LSM hooks for VFS mount operations
From: Paul Moore @ 2026-01-22 0:14 UTC (permalink / raw)
To: Song Liu
Cc: bpf, Linux-Fsdevel, lsf-pc, linux-security-module,
Christian Brauner, Al Viro
In-Reply-To: <CAPhsuW4=heDwYEkmRzSnLHDdW=da71qDd1KqUj9sYUOT5uOx3w@mail.gmail.com>
On Wed, Jan 21, 2026 at 4:18 PM Song Liu <song@kernel.org> wrote:
>
> Current LSM hooks do not have good coverage for VFS mount operations.
> Specifically, there are the following issues (and maybe more..):
I don't recall LSM folks normally being invited to LSFMMBPF so it
seems like this would be a poor forum to discuss LSM hooks.
> PS: I am not sure whether other folks are already working on it. I will prepare
> some RFC patches before the conference if I don't see other proposals.
FWIW, I'm not aware of anyone currently working on revising the mount
hooks, but it's possible. Posting a patchset, even an early RFC
draft, is always a good way to find out who might be working in the
same space :)
Posting to the mailing list also has the advantage of reaching
everyone who might be interested, whereas discussing this at a
conference, especially one that is invite-only, is limiting.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] cipso: harden use of skb_cow() in cipso_v4_skbuff_setattr()
From: Paul Moore @ 2026-01-22 0:48 UTC (permalink / raw)
To: Will Rosenberg
Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netdev, linux-security-module,
linux-kernel
In-Reply-To: <20260120155738.982771-1-whrosenb@asu.edu>
On Tue, Jan 20, 2026 at 10:57 AM Will Rosenberg <whrosenb@asu.edu> wrote:
>
> If skb_cow() is passed a headroom <= -NET_SKB_PAD, it will trigger a
> BUG. As a result, use cases should avoid calling with a headroom that
> is negative to prevent triggering this issue.
>
> This is the same code pattern fixed in Commit 58fc7342b529 ("ipv6:
> BUG() in pskb_expand_head() as part of calipso_skbuff_setattr()").
>
> In cipso_v4_skbuff_setattr(), len_delta can become negative, leading to
> a negative headroom passed to skb_cow(). However, the BUG is not
> triggerable because the condition headroom <= -NET_SKB_PAD cannot be
> satisfied due to limits on the IPv4 options header size.
>
> Avoid potential problems in the future by only using skb_cow() to grow
> the skb headroom.
>
> Signed-off-by: Will Rosenberg <whrosenb@asu.edu>
> ---
>
> Notes:
> Given that IPv4 option length should not change,
> this may not be a worthwhile patch.
>
> Apologies in advance if this ends up being a waste
> of time.
>
> net/ipv4/cipso_ipv4.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
I think it's a reasonable thing to do in an effort to avoid future
problems. Thanks Will.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 709021197e1c..32b951ebc0c2 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -2196,7 +2196,8 @@ int cipso_v4_skbuff_setattr(struct sk_buff *skb,
> /* if we don't ensure enough headroom we could panic on the skb_push()
> * call below so make sure we have enough, we are also "mangling" the
> * packet so we should probably do a copy-on-write call anyway */
> - ret_val = skb_cow(skb, skb_headroom(skb) + len_delta);
> + ret_val = skb_cow(skb,
> + skb_headroom(skb) + (len_delta > 0 ? len_delta : 0));
> if (ret_val < 0)
> return ret_val;
>
>
> base-commit: 58bae918d73e3b6cd57d1e39fcf7c75c7dd1a8fe
> --
> 2.34.1
--
paul-moore.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox