* Re: [PATCH 1/2] LSM: Exclusive secmark usage
From: Casey Schaufler @ 2025-11-04 16:58 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAHC9VhTr_0XtFfp017vXmaQVm77kkN+4ZqxNqNnBOW6MpFQqkg@mail.gmail.com>
On 10/13/2025 3:11 PM, Paul Moore wrote:
> On Fri, Oct 10, 2025 at 11:03 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 10/9/2025 11:49 AM, Stephen Smalley wrote:
>>> On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> The network secmark can only be used by one security module
>>>> at a time. Establish mechanism to identify to security modules
>>> a mechanism to inform security modules?
>>>
>>>> whether they have access to the secmark. SELinux already
>>>> incorparates mechanism, but it has to be added to Smack and
>>> incorporates
>>>
>>>> AppArmor.
>>>>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>> include/linux/lsm_hooks.h | 1 +
>>>> security/apparmor/include/net.h | 5 +++++
>>>> security/apparmor/lsm.c | 7 ++++---
>>>> security/security.c | 6 ++++++
>>>> security/selinux/hooks.c | 4 +++-
>>>> security/smack/smack.h | 5 +++++
>>>> security/smack/smack_lsm.c | 3 ++-
>>>> security/smack/smack_netfilter.c | 10 ++++++++--
>>>> 8 files changed, 34 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/security/security.c b/security/security.c
>>>> index ad163f06bf7a..e59e3d403de6 100644
>>>> --- a/security/security.c
>>>> +++ b/security/security.c
>>>> @@ -283,6 +283,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>>>> lsm_set_blob_size(&needed->lbs_xattr_count,
>>>> &blob_sizes.lbs_xattr_count);
>>>> lsm_set_blob_size(&needed->lbs_bdev, &blob_sizes.lbs_bdev);
>>>> + if (needed->lbs_secmark) {
>>>> + if (blob_sizes.lbs_secmark)
>>>> + needed->lbs_secmark = false;
>>>> + else
>>>> + blob_sizes.lbs_secmark = true;
>>>> + }
>>> So if I understand correctly, the first LSM to register with
>>> lbs_secmark set wins.
>>> Not sure that's a great idea - seemingly some LSMs may want to insist
>>> that they get to use secmark regardless of registration order?
>> But what if two LSMs insist on getting the secmark? The whole point
>> is to make it possible to use multiple LSMs that what the feature at
>> the same time.
> My current thinking is that if two LSMs *insist* on access to the
> secmark, one of them has to fail to load/initialize, even if that
> means a panic on boot (we should flag that as an invalid config in
> Kconfig).
That's sensible, but why should an LSM be allowed to insist on access
to the secmark? Best I can tell, SELinux rarely uses it in real life.
Smack currently always uses it, but that's fixed in this patch set.
I would be perplexed by a "dog in the manger" attitude on the part of
any maintainers.
>
> Perhaps the solution is to have lbs_secmark as a tristate value: don't
> use secmarks, would like access to secmarks, must have access to
> secmarks. Upon registration a LSM that requested "would like" could
> check to see if they have been granted access and could adjust
> accordingly. A LSM that requested "must have" would fail to register
> if the secmarks were already taken by a prior LSM.
I would be unhappy if any existing LSM decided it "must have" secmarks.
I can imagine a LSM that really required the secmark, but it would have
a tough time getting accepted.
>> The limitation on a secmark being a u32 is a huge problem,
>> and Paul has battled with the netdev people over it for years.
> I suspect the only way forward at this point is to convert the secmark
> field into an IDR* that we could use to point to a LSM security blob
> that could store LSM specific structs for both secmarks and general
> LSM state associated with a skb. This would also allow us to do some
> cool things in the forward path that we can't properly do now and
> would make it much easier to handle a wider variety of packet level
> access control scenarios.
>
> It's on my todo list for <hand_waving>someday</hand_waving>, but if
> somebody wanted to do it that would be awesome. Just a word of
> warning, this is not a quick task and it is probably only suited for
> someone who already has a few netdev inflicted scars.
I expect to be dead, or at least suffering serious memory loss,
by the time that work can be done. :(
>
> *I see that IDR is now deprecated in favor of XArray, I haven't looked
> that closely at XArray but it looks workable too.
>
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Casey Schaufler @ 2025-11-04 17:07 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAHC9VhSBxhiTvxPpHHqZJygDTTuMWOPFpQcoMSsvZD6Bueg0ZQ@mail.gmail.com>
On 10/13/2025 3:21 PM, Paul Moore wrote:
> On Fri, Oct 10, 2025 at 5:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 10/10/2025 12:53 PM, Stephen Smalley wrote:
>>> On Fri, Oct 10, 2025 at 11:09 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> On 10/9/2025 11:53 AM, Stephen Smalley wrote:
>>>>> On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> ..
>
>>> But some security modules may not function correctly (or at all) if
>>> secmark and/or netlabel are silently disabled on them, and the end
>>> user needs a better way to express intent.
> This is the point I was trying to make in patch 1/2 with secmarks, but
> Stephen has captured the idea much better in the sentence above. To
> be clear, the argument applies to both secmarks and NetLabel.
>
>> I'm open to suggestions. Would boot options lsm.secmark and lsm.netlabel
>> be sufficient to address your concern?
> No. Please no. We already have two LSM initialization related
> command line parameters, and one of them is pretty broken and very
> confusing in the new world of multiple LSMs (as an aside, does someone
> want to kick off the work to deprecate "security=?"). Maybe we have
> to go this route eventually, but let's keep it simple for right now; I
> don't want to add a lot of user-visible APIs for something that is
> pretty niche.
>
> If you absolutely can't live with the "first one gets it" approach,
> look at the no/wants/must idea in my patch 1/2 comments. It would
> require work in the individual LSMs to support it, but I'd rather try
> that route first.
I'm fine (for now, at least) with the "first LSM" approach, which is
what I have implemented. What I *am* afraid of is SELinux deciding that
it can only ever possibly work if it is the "first LSM". Best I can tell,
there's no reason for it beyond "configuration is hard". Which it is,
but we're already there.
^ permalink raw reply
* Re: [PATCH 2/2] LSM: Allow reservation of netlabel
From: Casey Schaufler @ 2025-11-04 17:01 UTC (permalink / raw)
To: Stephen Smalley
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAEjxPJ48PiZ5ZOZbZjka5YeiBxaWFsCufoGcY_jEztM+wtEUCA@mail.gmail.com>
On 10/9/2025 11:53 AM, Stephen Smalley wrote:
> On Wed, Oct 1, 2025 at 5:56 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Allow LSMs to request exclusive access to the netlabel facility.
>> Provide mechanism for LSMs to determine if they have access to
>> netlabel. Update the current users of netlabel, SELinux and Smack,
>> to use and respect the exclusive use of netlabel.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> diff --git a/security/security.c b/security/security.c
>> index e59e3d403de6..9eca10844b56 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -289,6 +289,12 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>> else
>> blob_sizes.lbs_secmark = true;
>> }
>> + if (needed->lbs_netlabel) {
>> + if (blob_sizes.lbs_netlabel)
>> + needed->lbs_netlabel = false;
>> + else
>> + blob_sizes.lbs_netlabel = true;
>> +
> Same principle here - if a LSM wants to use netlabel, it may want to
> guarantee that it truly has exclusive access to it no matter what the
> LSM order is.
Again, SELinux doesn't actually use this very often. Declaring that SELinux
always wants it to the exclusion of others would be obstructionist.
^ permalink raw reply
* Re: [PATCH v2 2/2] LSM: Infrastructure management of the mnt_opts security blob
From: Casey Schaufler @ 2025-11-04 17:46 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley
Cc: Ondrej Mosnacek, eparis, linux-security-module, jmorris, serge,
keescook, john.johansen, penguin-kernel, linux-kernel, selinux,
Casey Schaufler
In-Reply-To: <CAHC9VhSRGyMuTYxP0nDpXv_MwvNqVsrBXcak84AGHj7ycDtu3A@mail.gmail.com>
On 10/13/2025 1:55 PM, Paul Moore wrote:
> On Thu, Oct 9, 2025 at 2:38 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>> On Thu, Sep 25, 2025 at 1:12 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> Move management of the mnt_opts->security blob out of the individual
>>> security modules and into the security infrastructure. The modules
>>> tell the infrastructure how much space is required, and the space is
>>> allocated as required in the interfaces that use the blob.
>>>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> ---
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index 4bba9d119713..1ccf880e4894 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -656,19 +651,13 @@ static int selinux_set_mnt_opts(struct super_block *sb,
>>> mutex_lock(&sbsec->lock);
>>>
>>> if (!selinux_initialized()) {
>>> - if (!opts) {
>>> - /* Defer initialization until selinux_complete_init,
>>> - after the initial policy is loaded and the security
>>> - server is ready to handle calls. */
>>> - if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
>>> - sbsec->flags |= SE_SBNATIVE;
>>> - *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
>>> - }
>>> - goto out;
>>> + /* Defer initialization until selinux_complete_init,
>>> + after the initial policy is loaded and the security
>>> + server is ready to handle calls. */
>>> + if (kern_flags & SECURITY_LSM_NATIVE_LABELS) {
>>> + sbsec->flags |= SE_SBNATIVE;
>>> + *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS;
>> This seemingly would produce a change in behavior for SELinux.
Except that it doesn't, at least from the tests I've been able to find.
If multiple LSMs use mount options you can't use the !opts test, because
there may be options for another LSM. Deferring initialization is harmless
when there are options, as it's all checked again later.
>> Previously we would only do this if there were no SELinux mount
>> options specified.
> What Stephen said. I think this is good work that needs to be done
> (thank you for doing it!), but we have to preserve existing behaviors.
>
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Coiby Xu @ 2025-11-05 0:18 UTC (permalink / raw)
To: Paul Moore, Mimi Zohar
Cc: linux-integrity, linux-security-module, Karel Srot, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
open list, open list:MODULE SUPPORT
In-Reply-To: <CAHC9VhToe-VNqbh6TY2iYnRvqTHRfQjnHYSRWYgt8K7NcLKMdg@mail.gmail.com>
On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
>On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
>> > On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
>> > >
>> > > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
>> > > is enabled, IMA has no way to verify the appended module signature as it
>> > > can't decompress the module.
>> > >
>> > > Define a new LSM hook security_kernel_module_read_file which will be
>> > > called after kernel module decompression is done so IMA can access the
>> > > decompressed kernel module to verify the appended signature.
>> > >
>> > > Since IMA can access both xattr and appended kernel module signature
>> > > with the new LSM hook, it no longer uses the security_kernel_post_read_file
>> > > LSM hook for kernel module loading.
>> > >
>> > > Before enabling in-kernel module decompression, a kernel module in
>> > > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
>> > > kernel module rule in secure_boot policy to allow either an IMA
>> > > signature OR an appended signature i.e. to use
>> > > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>> > >
>> > > Reported-by: Karel Srot <ksrot@redhat.com>
>> > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > ---
>> > > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
>> > >
>> > > include/linux/lsm_hook_defs.h | 2 ++
>> > > include/linux/security.h | 7 +++++++
>> > > kernel/module/main.c | 10 +++++++++-
>> > > security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
>> > > security/integrity/ima/ima_policy.c | 2 +-
>> > > security/security.c | 17 +++++++++++++++++
>> > > 6 files changed, 62 insertions(+), 2 deletions(-)
>> >
>> > We don't really need a new LSM hook for this do we? Can't we just
>> > define a new file read type, e.g. READING_MODULE_DECOMPRESS, and do
>> > another call to security_kernel_post_read_file() after the module is
>> > unpacked? Something like the snippet below ...
>>
>> Yes, this is similar to my suggestion based on defining multiple enumerations:
>> READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE.
>> With this solution, IMA would need to make an exception in the post kernel
>> module read for the READING_COMPRESSED_MODULE case, since the kernel module has
>> not yet been decompressed.
>>
>> Coiby suggested further simplification by moving the call later. At which point
>> either there is or isn't an appended signature for non-compressed and
>> decompressed kernel modules.
>>
>> As long as you don't have a problem calling the security_kernel_post_read_file()
>> hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?
>
>It isn't clear from these comments if you are talking about moving
>only the second security_kernel_post_read_file() call that was
>proposed for init_module_from_file() to later in the function, leaving
>the call in kernel_read_file() intact, or something else?
Hi Paul and Mimi,
Thanks for sharing your feedback! Yes, you are right, there is no need
for a new LSM hook. Actually by not introducing a new LSM hook, we can
have a much simpler solution!
>
>I think we want to leave the hook calls in kernel_read_file() intact,
>in which case I'm not certain what advantage there is in moving the
>security_kernel_post_read_file() call to a location where it is called
>in init_module_from_file() regardless of if the module is compressed
>or not. In the uncompressed case you are calling the hook twice for
>no real benefit? It may be helpful to submit a patch with your
>proposal as a patch can be worth a thousand words ;)
>
>
>> > diff --git a/kernel/module/main.c b/kernel/module/main.c
>> > index c66b26184936..f127000d2e0a 100644
>> > --- a/kernel/module/main.c
>> > +++ b/kernel/module/main.c
>> > @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
>> > ar __user * uargs, int
>> > mod_stat_add_long(len, &invalid_decompress_bytes);
>> > return err;
>> > }
>> > +
>> > + err = security_kernel_post_read_file(f,
>> > + (char *)info.hdr, info.len,
>> > + READING_MODULE_DECOMPRESS);
>> > + if (err) {
>> > + mod_stat_inc(&failed_kreads);
>> > + return err;
>> > + }
>> > } else {
>> > info.hdr = buf;
>> > info.len = len;
>>
>> == defer security_kernel_post_read_file() call to here ==
By moving security_kernel_post_read_file, I think what Mimi means is to
move security_kernel_post_read_file in init_module_from_file() to later
in the function,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b261849362a..66725e53fef0c1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
struct load_info info = { };
void *buf = NULL;
int len;
+ int err;
len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
if (len < 0) {
@@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
}
if (flags & MODULE_INIT_COMPRESSED_FILE) {
- int err = module_decompress(&info, buf, len);
+ err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
mod_stat_inc(&failed_decompress);
@@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
info.len = len;
}
+ err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ free_copy(&info, flags);
+ return err;
+ }
+
return load_module(&info, uargs, flags);
}
If we only call security_kernel_post_read_file the 2nd time for a
decompressed kernel module, IMA won't be sure what to do when
security_kernel_post_read_file is called for the 1st time because it
can't distinguish between a compressed module with appended signature or
a uncompressed module without appended signature. If it permits 1st
calling security_kernel_post_read_file, a uncompressed module without
appended signature can be loaded. If it doesn't permit 1st calling
security_kernel_post_read_file, there is no change to call
security_kernel_post_read_file again for decompressed module.
And you are right, there is no need to call
security_kernel_post_read_file twice. And from the perspective of IMA,
it simplifies reasoning if it is guaranteed that IMA will always access
uncompressed kernel module regardless regardless of its original
compression state.
So I think a better solution is to stop calling
security_kernel_post_read_file in kernel_read_file for READING_MODULE.
This can also avoiding introducing an unnecessary
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
can make the solution even simpler,
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index de32c95d823dbd..7c78e84def6ec7 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
goto out_free;
}
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ /*
+ * security_kernel_post_read_file will be called later after
+ * a read kernel module is truly decompressed
+ */
+ if (id != READING_MODULE)
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
}
Btw, I notice IMA is the only user of security_kernel_post_read_file so
this change won't affect other LSMs. For a full patch, please visit
https://github.com/coiby/linux/commit/558d85779ab5d794874749ecfae0e48b890bf3e0.patch
If there are concerns that I'm unaware of and a new
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration is
necessary, here's another patch
https://github.com/coiby/linux/commit/cdd40317b6070f48ec871c6a89428084f38ca083.patch
>
>--
>paul-moore.com
>
--
Best regards,
Coiby
^ permalink raw reply related
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Paul Moore @ 2025-11-05 2:47 UTC (permalink / raw)
To: Coiby Xu
Cc: Mimi Zohar, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <fftfj4o3kqxmfu3hb655xczqcddoeqjv55llsnwkrdu5isdm4z@6sqe3k24a6kk>
On Tue, Nov 4, 2025 at 7:19 PM Coiby Xu <coxu@redhat.com> wrote:
> On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
> >On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >> On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
> >> > On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
> >> > >
> >> > > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> >> > > is enabled, IMA has no way to verify the appended module signature as it
> >> > > can't decompress the module.
> >> > >
> >> > > Define a new LSM hook security_kernel_module_read_file which will be
> >> > > called after kernel module decompression is done so IMA can access the
> >> > > decompressed kernel module to verify the appended signature.
> >> > >
> >> > > Since IMA can access both xattr and appended kernel module signature
> >> > > with the new LSM hook, it no longer uses the security_kernel_post_read_file
> >> > > LSM hook for kernel module loading.
> >> > >
> >> > > Before enabling in-kernel module decompression, a kernel module in
> >> > > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> >> > > kernel module rule in secure_boot policy to allow either an IMA
> >> > > signature OR an appended signature i.e. to use
> >> > > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
> >> > >
> >> > > Reported-by: Karel Srot <ksrot@redhat.com>
> >> > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> >> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> >> > > ---
> >> > > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
> >> > >
> >> > > include/linux/lsm_hook_defs.h | 2 ++
> >> > > include/linux/security.h | 7 +++++++
> >> > > kernel/module/main.c | 10 +++++++++-
> >> > > security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
> >> > > security/integrity/ima/ima_policy.c | 2 +-
> >> > > security/security.c | 17 +++++++++++++++++
> >> > > 6 files changed, 62 insertions(+), 2 deletions(-)
> >> >
> >> > We don't really need a new LSM hook for this do we? Can't we just
> >> > define a new file read type, e.g. READING_MODULE_DECOMPRESS, and do
> >> > another call to security_kernel_post_read_file() after the module is
> >> > unpacked? Something like the snippet below ...
> >>
> >> Yes, this is similar to my suggestion based on defining multiple enumerations:
> >> READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE.
> >> With this solution, IMA would need to make an exception in the post kernel
> >> module read for the READING_COMPRESSED_MODULE case, since the kernel module has
> >> not yet been decompressed.
> >>
> >> Coiby suggested further simplification by moving the call later. At which point
> >> either there is or isn't an appended signature for non-compressed and
> >> decompressed kernel modules.
> >>
> >> As long as you don't have a problem calling the security_kernel_post_read_file()
> >> hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?
> >
> >It isn't clear from these comments if you are talking about moving
> >only the second security_kernel_post_read_file() call that was
> >proposed for init_module_from_file() to later in the function, leaving
> >the call in kernel_read_file() intact, or something else?
>
> Hi Paul and Mimi,
>
> Thanks for sharing your feedback! Yes, you are right, there is no need
> for a new LSM hook. Actually by not introducing a new LSM hook, we can
> have a much simpler solution!
>
> >
> >I think we want to leave the hook calls in kernel_read_file() intact,
> >in which case I'm not certain what advantage there is in moving the
> >security_kernel_post_read_file() call to a location where it is called
> >in init_module_from_file() regardless of if the module is compressed
> >or not. In the uncompressed case you are calling the hook twice for
> >no real benefit? It may be helpful to submit a patch with your
> >proposal as a patch can be worth a thousand words ;)
> >
> >
> >> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> >> > index c66b26184936..f127000d2e0a 100644
> >> > --- a/kernel/module/main.c
> >> > +++ b/kernel/module/main.c
> >> > @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
> >> > ar __user * uargs, int
> >> > mod_stat_add_long(len, &invalid_decompress_bytes);
> >> > return err;
> >> > }
> >> > +
> >> > + err = security_kernel_post_read_file(f,
> >> > + (char *)info.hdr, info.len,
> >> > + READING_MODULE_DECOMPRESS);
> >> > + if (err) {
> >> > + mod_stat_inc(&failed_kreads);
> >> > + return err;
> >> > + }
> >> > } else {
> >> > info.hdr = buf;
> >> > info.len = len;
> >>
> >> == defer security_kernel_post_read_file() call to here ==
>
> By moving security_kernel_post_read_file, I think what Mimi means is to
> move security_kernel_post_read_file in init_module_from_file() to later
> in the function,
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362a..66725e53fef0c1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> struct load_info info = { };
> void *buf = NULL;
> int len;
> + int err;
>
> len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> if (len < 0) {
> @@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> }
>
> if (flags & MODULE_INIT_COMPRESSED_FILE) {
> - int err = module_decompress(&info, buf, len);
> + err = module_decompress(&info, buf, len);
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> mod_stat_inc(&failed_decompress);
> @@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> info.len = len;
> }
>
> + err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
> + READING_MODULE);
> + if (err) {
> + mod_stat_inc(&failed_kreads);
> + free_copy(&info, flags);
> + return err;
> + }
> +
> return load_module(&info, uargs, flags);
> }
>
> If we only call security_kernel_post_read_file the 2nd time for a
> decompressed kernel module, IMA won't be sure what to do when
> security_kernel_post_read_file is called for the 1st time because it
> can't distinguish between a compressed module with appended signature or
> a uncompressed module without appended signature. If it permits 1st
> calling security_kernel_post_read_file, a uncompressed module without
> appended signature can be loaded. If it doesn't permit 1st calling
> security_kernel_post_read_file, there is no change to call
> security_kernel_post_read_file again for decompressed module.
>
> And you are right, there is no need to call
> security_kernel_post_read_file twice. And from the perspective of IMA,
> it simplifies reasoning if it is guaranteed that IMA will always access
> uncompressed kernel module regardless regardless of its original
> compression state.
>
> So I think a better solution is to stop calling
> security_kernel_post_read_file in kernel_read_file for READING_MODULE.
> This can also avoiding introducing an unnecessary
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
> can make the solution even simpler,
>
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index de32c95d823dbd..7c78e84def6ec7 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
> goto out_free;
> }
>
> - ret = security_kernel_post_read_file(file, *buf, i_size, id);
> + /*
> + * security_kernel_post_read_file will be called later after
> + * a read kernel module is truly decompressed
> + */
> + if (id != READING_MODULE)
> + ret = security_kernel_post_read_file(file, *buf, i_size, id);
> }
Assuming I'm understanding the problem correctly, I think you're
making this harder than it needs to be. I believe something like this
should solve the problem without having to add more conditionals
around the hooks in kernel_read_file(), and limiting the multiple
security_kernel_post_read_file() calls to just the compressed case ...
and honestly in each of the _post_read_file() calls in the compressed
case, the buffer contents have changed so it somewhat makes sense.
Given the code below, IMA could simply ignore the
READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
in that case) and focus on the READING_MODULE case as it does today.
I expect the associated IMA patch would be both trivial and small.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..b435c498ec01 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
ent *u)
static int init_module_from_file(struct file *f, const char __user * uargs, int
flags)
{
+ bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
struct load_info info = { };
void *buf = NULL;
int len;
- len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+ len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+ compressed ? READING_MODULE_COMPRESSED : READING_
MODULE);
if (len < 0) {
mod_stat_inc(&failed_kreads);
return len;
}
- if (flags & MODULE_INIT_COMPRESSED_FILE) {
+ if (compressed) {
int err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
@@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
ar __user * uargs, int
mod_stat_add_long(len, &invalid_decompress_bytes);
return err;
}
+
+ err = security_kernel_post_read_file(f,
+ (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ return err;
+ }
} else {
info.hdr = buf;
info.len = len;
--
paul-moore.com
^ permalink raw reply related
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-05 14:07 UTC (permalink / raw)
To: Paul Moore, Coiby Xu
Cc: linux-integrity, linux-security-module, Karel Srot, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
open list, open list:MODULE SUPPORT
In-Reply-To: <CAHC9VhRGwXvhU64Nk5jdmtPfrt9bbkzpLVqS0LRbtN3Q3HhnCw@mail.gmail.com>
On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> Assuming I'm understanding the problem correctly, I think you're
> making this harder than it needs to be. I believe something like this
> should solve the problem without having to add more conditionals
> around the hooks in kernel_read_file(), and limiting the multiple
> security_kernel_post_read_file() calls to just the compressed case ...
> and honestly in each of the _post_read_file() calls in the compressed
> case, the buffer contents have changed so it somewhat makes sense.
> Given the code below, IMA could simply ignore the
> READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> in that case) and focus on the READING_MODULE case as it does today.
> I expect the associated IMA patch would be both trivial and small.
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..b435c498ec01 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> ent *u)
>
> static int init_module_from_file(struct file *f, const char __user * uargs, int
> flags)
> {
> + bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> struct load_info info = { };
> void *buf = NULL;
> int len;
>
> - len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> + len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> + compressed ? READING_MODULE_COMPRESSED : READING_
> MODULE);
> if (len < 0) {
> mod_stat_inc(&failed_kreads);
> return len;
> }
>
> - if (flags & MODULE_INIT_COMPRESSED_FILE) {
> + if (compressed) {
> int err = module_decompress(&info, buf, len);
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> @@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
> ar __user * uargs, int
> mod_stat_add_long(len, &invalid_decompress_bytes);
> return err;
> }
> +
> + err = security_kernel_post_read_file(f,
> + (char *)info.hdr, info.len,
> + READING_MODULE);
Without changing the enumeration here, IMA would not be able to differentiate
the first call to security_kernel_post_read_file() and this one. The first call
would result in unnecessary error messages.
Adding an additional call to security_kernel_post_read_file() here, would
require defining 2 additional enumerations: READING_MODULE_COMPRESSED,
READING_MODULE_DECOMPRESSED.
> + if (err) {
> + mod_stat_inc(&failed_kreads);
> + return err;
> + }
> } else {
> info.hdr = buf;
> info.len = len;
Deferring the security_kernel_post_read_file() call to here, eliminates the need
for defining additional enumerations. (Coiby's first link.)
Adding an additional call to security_kernel_post_read_file() here, requires 1
additional enumeration. (Coiby's 2nd link.)
Mimi
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Oleg Nesterov @ 2025-11-05 14:32 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE9EF78827D73D3D7212F7E432A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
I am still thinking about another approach, will write another email.
But let me take a closer look at your patch.
First of all, can you split it? See below.
On 08/21, Bernd Edlinger wrote:
>
> -static int de_thread(struct task_struct *tsk)
> +static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
> {
> struct signal_struct *sig = tsk->signal;
> struct sighand_struct *oldsighand = tsk->sighand;
> spinlock_t *lock = &oldsighand->siglock;
> + struct task_struct *t;
> + bool unsafe_execve_in_progress = false;
>
> if (thread_group_empty(tsk))
> goto no_thread_group;
> @@ -932,6 +934,19 @@ static int de_thread(struct task_struct *tsk)
> if (!thread_group_leader(tsk))
> sig->notify_count--;
>
> + for_other_threads(tsk, t) {
> + if (unlikely(t->ptrace)
> + && (t != tsk->group_leader || !t->exit_state))
> + unsafe_execve_in_progress = true;
you can add "break" into the "if ()" block...
But this is minor. Why do we need "bool unsafe_execve_in_progress" ?
If this patch is correct, de_thread() can drop/reacquire cred_guard_mutex
unconditionally.
If you really think it makes sense, please make another patch with the
changelog.
I'd certainly prefer to avoid this boolean at least for the start. If nothing
else to catch the potential problems earlier.
> + if (unlikely(unsafe_execve_in_progress)) {
> + spin_unlock_irq(lock);
> + sig->exec_bprm = bprm;
> + mutex_unlock(&sig->cred_guard_mutex);
> + spin_lock_irq(lock);
I don't think spin_unlock_irq() + spin_lock_irq() makes any sense...
> @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
> */
> trace_sched_prepare_exec(current, bprm);
>
> + /* If the binary is not readable then enforce mm->dumpable=0 */
> + would_dump(bprm, bprm->file);
> + if (bprm->have_execfd)
> + would_dump(bprm, bprm->executable);
> +
> + /*
> + * Figure out dumpability. Note that this checking only of current
> + * is wrong, but userspace depends on it. This should be testing
> + * bprm->secureexec instead.
> + */
> + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> + is_dumpability_changed(current_cred(), bprm->cred) ||
> + !(uid_eq(current_euid(), current_uid()) &&
> + gid_eq(current_egid(), current_gid())))
> + set_dumpable(bprm->mm, suid_dumpable);
> + else
> + set_dumpable(bprm->mm, SUID_DUMP_USER);
> +
OK, we need to do this before de_thread() drops cred_guard_mutex.
But imo this too should be done in a separate patch, the changelog should
explain this change.
> @@ -1361,6 +1387,11 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> return -ERESTARTNOINTR;
>
> + if (unlikely(current->signal->exec_bprm)) {
> + mutex_unlock(¤t->signal->cred_guard_mutex);
> + return -ERESTARTNOINTR;
> + }
OK, if signal->exec_bprm != NULL, then current is already killed. But
proc_pid_attr_write() and ptrace_traceme() do the same. So how about
something like
int lock_current_cgm(void)
{
if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
return -ERESTARTNOINTR;
if (!current->signal->group_exec_task)
return 0;
WARN_ON(!fatal_signal_pending(current));
mutex_unlock(¤t->signal->cred_guard_mutex);
return -ERESTARTNOINTR;
}
?
Note that it checks ->group_exec_task, not ->exec_bprm. So this change can
come in a separate patch too, but I won't insist.
> @@ -453,6 +454,28 @@ static int ptrace_attach(struct task_struct *task, long request,
> return retval;
> }
>
> + if (unlikely(task == task->signal->group_exec_task)) {
> + retval = down_write_killable(&task->signal->exec_update_lock);
> + if (retval)
> + return retval;
> +
> + scoped_guard (task_lock, task) {
> + struct linux_binprm *bprm = task->signal->exec_bprm;
> + const struct cred __rcu *old_cred = task->real_cred;
> + struct mm_struct *old_mm = task->mm;
> +
> + rcu_assign_pointer(task->real_cred, bprm->cred);
> + task->mm = bprm->mm;
> + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + rcu_assign_pointer(task->real_cred, old_cred);
> + task->mm = old_mm;
> + }
This is the most problematic change which I can't review...
Firstly, it changes task->mm/real_cred for __ptrace_may_access() and this
looks dangerous to me.
Say, current_is_single_threaded() called by another CLONE_VM process can
miss group_exec_task and falsely return true. Probably not that bad, in
this case old_mm should go away soon, but still...
And I don't know if this can fool the users of task_cred_xxx/__task_cred
somehow.
Or. check_unsafe_exec() sets LSM_UNSAFE_PTRACE if ptrace. Is it safe to
ptrace the execing task after that? I have no idea what the security hooks
can do...
Again, can't review this part.
Oleg.
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Paul Moore @ 2025-11-05 15:42 UTC (permalink / raw)
To: Mimi Zohar
Cc: Coiby Xu, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <0c7e94a436a3742003e5e1155a48480d8307a9c7.camel@linux.ibm.com>
On Wed, Nov 5, 2025 at 9:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> > Assuming I'm understanding the problem correctly, I think you're
> > making this harder than it needs to be. I believe something like this
> > should solve the problem without having to add more conditionals
> > around the hooks in kernel_read_file(), and limiting the multiple
> > security_kernel_post_read_file() calls to just the compressed case ...
> > and honestly in each of the _post_read_file() calls in the compressed
> > case, the buffer contents have changed so it somewhat makes sense.
>
> > Given the code below, IMA could simply ignore the
> > READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> > in that case) and focus on the READING_MODULE case as it does today.
> > I expect the associated IMA patch would be both trivial and small.
> >
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index c66b26184936..b435c498ec01 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> > ent *u)
> >
> > static int init_module_from_file(struct file *f, const char __user * uargs, int
> > flags)
> > {
> > + bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> > struct load_info info = { };
> > void *buf = NULL;
> > int len;
> >
> > - len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> > + len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> > + compressed ? READING_MODULE_COMPRESSED : READING_
> > MODULE);
> > if (len < 0) {
> > mod_stat_inc(&failed_kreads);
> > return len;
> > }
> >
> > - if (flags & MODULE_INIT_COMPRESSED_FILE) {
> > + if (compressed) {
> > int err = module_decompress(&info, buf, len);
> > vfree(buf); /* compressed data is no longer needed */
> > if (err) {
> > @@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
> > ar __user * uargs, int
> > mod_stat_add_long(len, &invalid_decompress_bytes);
> > return err;
> > }
> > +
> > + err = security_kernel_post_read_file(f,
> > + (char *)info.hdr, info.len,
> > + READING_MODULE);
>
> Without changing the enumeration here, IMA would not be able to differentiate
> the first call to security_kernel_post_read_file() and this one. The first call
> would result in unnecessary error messages.
Given the patch snippet above, in the case where an uncompressed
module is passed into init_module_from_file() there would be the
following checks, in this order:
* kernel_read_file()
-> security_kernel_read_file(READING_MODULE)
-> security_kernel_post_read_file(READING_MODULE)
* init_module_from_file()
-> NONE
... this should be the same as the current behavior.
In the case where a compressed module is passed into
init_module_from_file() there would be the following checks, in this
order:
* kernel_read_file()
-> security_kernel_read_file(READING_MODULE_COMPRESSED)
-> security_kernel_post_read_file(READING_MODULE_COMPRESSED)
* init_module_from_file()
-> security_kernel_post_read_file(READING_MODULE)
... the two differences being that the hook calls in
kernel_read_file() use the READING_MODULE_COMPRESSED id, which seems
appropriate as the data passed to the hook is the compressed
representation, and the additional _post_read_file() hook call in
init_module_from_file() using the READING_MODULE id, as the data
passed to the hook is now uncompressed. Not only should IMA be able
to easily differentiate between the two _post_read_file() calls, but
it should have access to both the compressed and uncompressed data.
--
paul-moore.com
^ permalink raw reply
* [PATCH 0/3] Implement LANDLOCK_ADD_RULE_NO_INHERIT
From: Justin Suess @ 2025-11-05 18:00 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Justin Suess
Hi,
This patch builds on version 3 of the the "quiet flag" series by Tingmao Wang.
v3: https://lore.kernel.org/linux-security-module/cover.1761511023.git.m@maowtm.org/
It implements a new flag that prevents inheriting access rights from parent
objects within a single landlock layer. This is useful for policies
where a parent directory requires looser access grants that its
children.
For example, within a single ruleset / layer, given:
/a = rw
/a/b = ro
Under the current featureset, /a/b recieves rw permissions because it
inherits the w permission from /a
To solve this, I add a new flag LANDLOCK_RULE_ADD_NO_INHERIT which
suppresses parent permissions.
For example:
/a = rw
/a/b = ro + LANDLOCK_RULE_ADD_NO_INHERIT
This grants /a/b only read permissions.
Design:
- When this flag is added to a rule, the landlock_unmask_layers function will
track encounters of this flag in the rule_flags as it traverses up the fs tree.
When this flag is encountered, the access grants of the current rule will be allowed,
but further access grants by rules within that layer will be suppressed.
- Access grants made by rules in other layers will continue until the access requirements are satisfied.
Demo:
~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch fi
Executing the sandboxed command...
~ # ls
bin dev etc fi init proc root sbin sys tmp usr
~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch /tmp/fi
Executing the sandboxed command...
touch: /tmp/fi: Permission denied
~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer sh
Executing the sandboxed command...
sh: can't access tty; job control turned off
~ # cd tmp
/tmp # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer ls
Executing the sandboxed command...
/tmp # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch fifi
Executing the sandboxed command...
touch: fifi: Permission denied
/tmp # LL_FS_RW="/" LL_FS_RO="" landlock-sandboxer touch fifi
Executing the sandboxed command...
touch: fifi: Permission denied
/tmp #
This is my first patch/contribution to the LSM subsystem (and the linux
kernel as a whole), so any feedback and corrections on mailing list
ettiquite would be appreciated.
Very Respectfully,
Justin Suess
Justin Suess (3):
landlock: Add flag to supress access rule inheritence within a layer
samples/landlock: Add no inherit support to sandboxer
selftests/landlock: Add test for new no inherit flag
include/uapi/linux/landlock.h | 9 ++++
samples/landlock/sandboxer.c | 39 ++++++++++++----
security/landlock/ruleset.c | 8 ++++
security/landlock/ruleset.h | 10 ++++
security/landlock/syscalls.c | 3 +-
tools/testing/selftests/landlock/fs_test.c | 53 +++++++++++++++++-----
6 files changed, 99 insertions(+), 23 deletions(-)
base-commit: 77903de728f2a1ef40a31a3babf861b8fbf9530f
--
2.51.0
^ permalink raw reply
* [PATCH 1/3] landlock: Add flag to supress access rule inheritence within a layer
From: Justin Suess @ 2025-11-05 18:00 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Justin Suess
In-Reply-To: <20251105180019.1432367-1-utilityemal77@gmail.com>
Creates a new flag that prevents inheriting access rights from parent
objects within a single landlock layer. For example /a/b = ro + no inherit
and /a = rw results in /a/b recieving ro and not rw permissions.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
include/uapi/linux/landlock.h | 9 +++++++++
security/landlock/ruleset.c | 8 ++++++++
security/landlock/ruleset.h | 10 ++++++++++
security/landlock/syscalls.c | 3 ++-
4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 50f0806b7e33..d9daef551d96 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,19 @@ struct landlock_ruleset_attr {
* allowed_access in the passed in rule_attr. When this flag is
* present, the caller is also allowed to pass in an empty
* allowed_access.
+ * %LANDLOCK_ADD_RULE_NO_INHERIT
+ * When this flag is set while adding a rule to a ruleset, the rule
+ * will not inherit allowed accesses from rules on parent objects
+ * within the same layer. (currently only applies to filesystem objects)
+ * By default, Landlock rules added to a ruleset inherit allowed accesses
+ * from parent objects, meaning that if a parent directory has been granted
+ * certain access rights, those rights will also apply to its child objects.
+ * This flag prevents such inheritance for the specific rule being added.
*/
/* clang-format off */
#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT (1U << 1)
/* clang-format on */
/**
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index e0bb7e795574..8fab8222fc30 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -315,6 +315,7 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
.level = 0,
.flags = {
.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
+ .no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
}
} };
@@ -660,9 +661,16 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Skip layers that already have no inherit flags. */
+ if (rule_flags &&
+ (rule_flags->no_inherit_masks & layer_bit))
+ continue;
+
/* Collect rule flags for each layer. */
if (rule_flags && layer->flags.quiet)
rule_flags->quiet_masks |= layer_bit;
+ if (rule_flags && layer->flags.no_inherit)
+ rule_flags->no_inherit_masks |= layer_bit;
/*
* Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index cd0434d8dc63..7759151ce727 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,12 @@ struct landlock_layer {
* down the file hierarchy.
*/
bool quiet:1;
+ /**
+ * @no_inherit: When set, this layer's rule does not inherit
+ * allowed accesses from parent objects within the same layer.
+ * (currently only applies to filesystem objects)
+ */
+ bool no_inherit:1;
} flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
@@ -56,6 +62,10 @@ struct collected_rule_flags {
* @quiet_masks: Layers for which the quiet flag is effective.
*/
layer_mask_t quiet_masks;
+ /**
+ * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+ */
+ layer_mask_t no_inherit_masks;
};
/**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 93396bfc1500..ed7304d53894 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -463,7 +463,8 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
if (!is_initialized())
return -EOPNOTSUPP;
- if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+ if (flags && flags & ~(LANDLOCK_ADD_RULE_QUIET | \
+ LANDLOCK_ADD_RULE_NO_INHERIT))
return -EINVAL;
/* Gets and checks the ruleset. */
--
2.51.0
^ permalink raw reply related
* [PATCH 2/3] samples/landlock: Add no inherit support to sandboxer
From: Justin Suess @ 2025-11-05 18:00 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Justin Suess
In-Reply-To: <20251105180019.1432367-1-utilityemal77@gmail.com>
Adds two new environment variables LL_FS_RO_NO_INHERIT and LL_FS_RW_NO_INHERIT
to test the new LANDLOCK_RULE_ADD_NO_INHERIT flag
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
samples/landlock/sandboxer.c | 39 +++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 2d8e3e94b77b..2b40b2df83b4 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,8 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RO_NAME "LL_FS_RO"
#define ENV_FS_RW_NAME "LL_FS_RW"
+#define ENV_FS_RO_NO_INHERIT_NAME "LL_FS_RO_NO_INHERIT"
+#define ENV_FS_RW_NO_INHERIT_NAME "LL_FS_RW_NO_INHERIT"
#define ENV_FS_QUIET_NAME "LL_FS_QUIET"
#define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
@@ -121,7 +123,8 @@ static int parse_path(char *env_path, const char ***const path_list)
/* clang-format on */
static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
- const __u64 allowed_access, bool quiet)
+ const __u64 allowed_access,
+ __u32 add_rule_flags, bool mandatory)
{
int num_paths, i, ret = 1;
char *env_path_name;
@@ -132,9 +135,13 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
env_path_name = getenv(env_var);
if (!env_path_name) {
- /* Prevents users to forget a setting. */
- fprintf(stderr, "Missing environment variable %s\n", env_var);
- return 1;
+ if (mandatory) {
+ /* Prevents users to forget a setting. */
+ fprintf(stderr, "Missing environment variable %s\n",
+ env_var);
+ return 1;
+ }
+ return 0;
}
env_path_name = strdup(env_path_name);
unsetenv(env_var);
@@ -171,8 +178,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
if (!S_ISDIR(statbuf.st_mode))
path_beneath.allowed_access &= ACCESS_FILE;
if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
- &path_beneath,
- quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
+ &path_beneath, add_rule_flags)) {
fprintf(stderr,
"Failed to update the ruleset with \"%s\": %s\n",
path_list[i], strerror(errno));
@@ -375,6 +381,8 @@ static const char help[] =
"Optional settings (when not set, their associated access check "
"is always allowed, which is different from an empty string which "
"means an empty list):\n"
+ "* " ENV_FS_RO_NO_INHERIT_NAME ": read-only paths without rule inheritance\n"
+ "* " ENV_FS_RW_NO_INHERIT_NAME ": read-write paths without rule inheritance\n"
"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
@@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
}
if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
- false)) {
+ 0, true)) {
goto err_close_ruleset;
}
if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
- false)) {
+ 0, true)) {
+ goto err_close_ruleset;
+ }
+ /* Optional no-inherit rules mirror the regular read-only/read-write sets. */
+ if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
+ access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
+ false)) {
+ goto err_close_ruleset;
+ }
+ if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
+ access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,
+ false)) {
goto err_close_ruleset;
}
/* Don't require this env to be present. */
- if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
+ if (quiet_supported) {
if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
- true)) {
+ LANDLOCK_ADD_RULE_QUIET, false)) {
goto err_close_ruleset;
}
}
--
2.51.0
^ permalink raw reply related
* [PATCH 3/3] selftests/landlock: Add test for new no inherit flag
From: Justin Suess @ 2025-11-05 18:00 UTC (permalink / raw)
To: linux-security-module
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
Justin Suess
In-Reply-To: <20251105180019.1432367-1-utilityemal77@gmail.com>
Adds tests for the new no inherit flag, validating the expected
supression of permission inheritence within a layer. Also updates
the add_path_beneath function to take a __u32 (instead of bool quiet)
for the flags, since LANDLOCK_ADD_RULE_QUIET isn't the only
flag anymore.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
tools/testing/selftests/landlock/fs_test.c | 53 +++++++++++++++++-----
1 file changed, 41 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index d4819ff44230..a7ebe4ec1b5a 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -717,16 +717,12 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
}
static void add_path_beneath(struct __test_metadata *const _metadata,
- const int ruleset_fd, const __u64 allowed_access,
- const char *const path, bool quiet)
+ const int ruleset_fd, const __u64 allowed_access,
+ const char *const path, __u32 flags)
{
struct landlock_path_beneath_attr path_beneath = {
.allowed_access = allowed_access,
};
- __u32 flags = 0;
-
- if (quiet)
- flags |= LANDLOCK_ADD_RULE_QUIET;
path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
ASSERT_LE(0, path_beneath.parent_fd)
@@ -790,7 +786,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
continue;
add_path_beneath(_metadata, ruleset_fd, rules[i].access,
- rules[i].path, false);
+ rules[i].path, 0);
}
return ruleset_fd;
}
@@ -1368,7 +1364,7 @@ TEST_F_FORK(layout1, inherit_subset)
* ANDed with the previous ones.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d2, false);
+ dir_s1d2, 0);
/*
* According to ruleset_fd, dir_s1d2 should now have the
* LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
@@ -1400,7 +1396,7 @@ TEST_F_FORK(layout1, inherit_subset)
* Try to get more privileges by adding new access rights to the parent
* directory: dir_s1d1.
*/
- add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, false);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
enforce_ruleset(_metadata, ruleset_fd);
/* Same tests and results as above. */
@@ -1423,7 +1419,7 @@ TEST_F_FORK(layout1, inherit_subset)
* that there was no rule tied to it before.
*/
add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
- dir_s1d3, false);
+ dir_s1d3, 0);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -1476,7 +1472,7 @@ TEST_F_FORK(layout1, inherit_superset)
add_path_beneath(_metadata, ruleset_fd,
LANDLOCK_ACCESS_FS_READ_FILE |
LANDLOCK_ACCESS_FS_READ_DIR,
- dir_s1d2, false);
+ dir_s1d2, 0);
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
@@ -1488,6 +1484,39 @@ TEST_F_FORK(layout1, inherit_superset)
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
}
+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+ struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_fs = ACCESS_RW,
+ };
+ int ruleset_fd;
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+ add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+ LANDLOCK_ADD_RULE_NO_INHERIT);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Parent directory still grants write access to its direct children. */
+ EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+ /* dir_s1d2 gets only its explicit read-only access rights. */
+ EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+ /* Descendants of dir_s1d2 inherit the reduced access mask. */
+ EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
TEST_F_FORK(layout0, max_layers)
{
int i, err;
@@ -7647,7 +7676,7 @@ static int apply_a_layer(struct __test_metadata *const _metadata,
continue;
add_path_beneath(_metadata, rs_fd, r->access, r->path,
- r->quiet);
+ r->quiet ? LANDLOCK_ADD_RULE_QUIET : 0);
}
ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v1] fs: Move might_sleep() annotation to iput_final()
From: Mateusz Guzik @ 2025-11-05 19:50 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, linux-security-module, linux-fsdevel, Al Viro,
Edward Adam Davis, Günther Noack, Hillf Danton, Jan Kara,
Jann Horn, Max Kellermann, Tingmao Wang,
syzbot+12479ae15958fc3f54ec
In-Reply-To: <20251105193800.2340868-1-mic@digikod.net>
On Wed, Nov 5, 2025 at 8:38 PM Mickaël Salaün <mic@digikod.net> wrote:
>
> iput() don't directly call any sleepable code but mostly checks flags
> and decrement a reference counter before calling iput_final() and then
> evict().
>
> Some code might call iput() with guarantees that iput_final() will not
> be called. This is the case for Landlock's hook_sb_delete() where the
> inode counter must de decremented while holding it with another
> reference, see comment above the first iput() call.
>
> Move the new might_sleep() call from iput() to iput_final(). The
> alternative would be to manually decrement the counter without calling
> iput(), but it doesn't seem right.
>
This would mostly defeat the point of the original change.
Instead, if you have a consumer which *guarantees* this is not the
last reference, the vfs layer can provide a helper which acts
accordingly.
Something like this (untested):
diff --git a/fs/inode.c b/fs/inode.c
index 84f539497857..a3ece9b4b6ef 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2011,6 +2011,15 @@ void iput(struct inode *inode)
}
EXPORT_SYMBOL(iput);
+void iput_not_last(struct inode *inode)
+{
+ VFS_BUG_ON_INODE(inode_state_read_once(inode) & I_CLEAR, inode);
+ VFS_BUG_ON_INODE(atomic_read(&inode->i_count) < 2, inode);
+
+ WARN_ON(atomic_sub_return(1, &inode->i_count) == 0);
+}
+EXPORT_SYMBOL(iput_not_last);
+
#ifdef CONFIG_BLOCK
/**
* bmap - find a block number in a file
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 41c855ef0594..8181a0d0e2ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2909,6 +2909,7 @@ extern int current_umask(void);
extern void ihold(struct inode * inode);
extern void iput(struct inode *);
+void iput_not_last(struct inode *);
int inode_update_timestamps(struct inode *inode, int flags);
int generic_update_time(struct inode *, int);
^ permalink raw reply related
* [PATCH v1] fs: Move might_sleep() annotation to iput_final()
From: Mickaël Salaün @ 2025-11-05 19:37 UTC (permalink / raw)
To: Christian Brauner
Cc: Mickaël Salaün, linux-security-module, linux-fsdevel,
Al Viro, Edward Adam Davis, Günther Noack, Hillf Danton,
Jan Kara, Jann Horn, Mateusz Guzik, Max Kellermann, Tingmao Wang,
syzbot+12479ae15958fc3f54ec
iput() don't directly call any sleepable code but mostly checks flags
and decrement a reference counter before calling iput_final() and then
evict().
Some code might call iput() with guarantees that iput_final() will not
be called. This is the case for Landlock's hook_sb_delete() where the
inode counter must de decremented while holding it with another
reference, see comment above the first iput() call.
Move the new might_sleep() call from iput() to iput_final(). The
alternative would be to manually decrement the counter without calling
iput(), but it doesn't seem right.
Cc: Christian Brauner <brauner@kernel.org>
Cc: Edward Adam Davis <eadavis@qq.com>
Cc: Günther Noack <gnoack@google.com>
Cc: Hillf Danton <hdanton@sina.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Max Kellermann <max.kellermann@ionos.com>
Cc: Tingmao Wang <m@maowtm.org>
Reported-by: syzbot+12479ae15958fc3f54ec@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68d32659.a70a0220.4f78.0012.GAE@google.com/
Fixes: 2ef435a872ab ("fs: add might_sleep() annotation to iput() and more")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
From hook_sb_delete():
/*
* At this point, we own the ihold() reference that was
* originally set up by get_inode_object() and the
* __iget() reference that we just set in this loop
* walk. Therefore the following call to iput() will
* not sleep nor drop the inode because there is now at
* least two references to it.
*/
iput(inode);
#syz test
---
fs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index 84f539497857..64120cb21e8b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1917,6 +1917,7 @@ static void iput_final(struct inode *inode)
const struct super_operations *op = inode->i_sb->s_op;
int drop;
+ might_sleep();
WARN_ON(inode_state_read(inode) & I_NEW);
VFS_BUG_ON_INODE(atomic_read(&inode->i_count) != 0, inode);
@@ -1969,7 +1970,6 @@ static void iput_final(struct inode *inode)
*/
void iput(struct inode *inode)
{
- might_sleep();
if (unlikely(!inode))
return;
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-05 20:25 UTC (permalink / raw)
To: Paul Moore
Cc: Coiby Xu, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <CAHC9VhS6xWvu5TjjS4MRGFEWxdAhg-Xsf6L+=K0k8U+fgiAtTQ@mail.gmail.com>
On Wed, 2025-11-05 at 10:42 -0500, Paul Moore wrote:
> On Wed, Nov 5, 2025 at 9:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> > > Assuming I'm understanding the problem correctly, I think you're
> > > making this harder than it needs to be. I believe something like this
> > > should solve the problem without having to add more conditionals
> > > around the hooks in kernel_read_file(), and limiting the multiple
> > > security_kernel_post_read_file() calls to just the compressed case ...
> > > and honestly in each of the _post_read_file() calls in the compressed
> > > case, the buffer contents have changed so it somewhat makes sense.
> >
> > > Given the code below, IMA could simply ignore the
> > > READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> > > in that case) and focus on the READING_MODULE case as it does today.
> > > I expect the associated IMA patch would be both trivial and small.
> > >
> > > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > > index c66b26184936..b435c498ec01 100644
> > > --- a/kernel/module/main.c
> > > +++ b/kernel/module/main.c
> > > @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> > > ent *u)
> > >
> > > static int init_module_from_file(struct file *f, const char __user * uargs, int
> > > flags)
> > > {
> > > + bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> > > struct load_info info = { };
> > > void *buf = NULL;
> > > int len;
> > >
> > > - len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> > > + len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> > > + compressed ? READING_MODULE_COMPRESSED : READING_
> > > MODULE);
> > > if (len < 0) {
> > > mod_stat_inc(&failed_kreads);
> > > return len;
> > > }
> > >
> > > - if (flags & MODULE_INIT_COMPRESSED_FILE) {
> > > + if (compressed) {
> > > int err = module_decompress(&info, buf, len);
> > > vfree(buf); /* compressed data is no longer needed */
> > > if (err) {
> > > @@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
> > > ar __user * uargs, int
> > > mod_stat_add_long(len, &invalid_decompress_bytes);
> > > return err;
> > > }
> > > +
> > > + err = security_kernel_post_read_file(f,
> > > + (char *)info.hdr, info.len,
> > > + READING_MODULE);
> >
> > Without changing the enumeration here, IMA would not be able to differentiate
> > the first call to security_kernel_post_read_file() and this one. The first call
> > would result in unnecessary error messages.
>
> Given the patch snippet above, in the case where an uncompressed
> module is passed into init_module_from_file() there would be the
> following checks, in this order:
>
> * kernel_read_file()
> -> security_kernel_read_file(READING_MODULE)
> -> security_kernel_post_read_file(READING_MODULE)
> * init_module_from_file()
> -> NONE
>
> ... this should be the same as the current behavior.
>
> In the case where a compressed module is passed into
> init_module_from_file() there would be the following checks, in this
> order:
>
> * kernel_read_file()
> -> security_kernel_read_file(READING_MODULE_COMPRESSED)
> -> security_kernel_post_read_file(READING_MODULE_COMPRESSED)
> * init_module_from_file()
> -> security_kernel_post_read_file(READING_MODULE)
>
> ... the two differences being that the hook calls in
> kernel_read_file() use the READING_MODULE_COMPRESSED id, which seems
> appropriate as the data passed to the hook is the compressed
> representation, and the additional _post_read_file() hook call in
> init_module_from_file() using the READING_MODULE id, as the data
> passed to the hook is now uncompressed. Not only should IMA be able
> to easily differentiate between the two _post_read_file() calls, but
> it should have access to both the compressed and uncompressed data.
Thanks, Paul. Yes, a single additional enumeration is enough.
Mimi
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-05 20:47 UTC (permalink / raw)
To: Coiby Xu, Paul Moore
Cc: linux-integrity, linux-security-module, Karel Srot, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
open list, open list:MODULE SUPPORT
In-Reply-To: <fftfj4o3kqxmfu3hb655xczqcddoeqjv55llsnwkrdu5isdm4z@6sqe3k24a6kk>
On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
> > On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
> > > > On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
> > > > >
> > > > > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> > > > > is enabled, IMA has no way to verify the appended module signature as it
> > > > > can't decompress the module.
> > > > >
> > > > > Define a new LSM hook security_kernel_module_read_file which will be
> > > > > called after kernel module decompression is done so IMA can access the
> > > > > decompressed kernel module to verify the appended signature.
> > > > >
> > > > > Since IMA can access both xattr and appended kernel module signature
> > > > > with the new LSM hook, it no longer uses the security_kernel_post_read_file
> > > > > LSM hook for kernel module loading.
> > > > >
> > > > > Before enabling in-kernel module decompression, a kernel module in
> > > > > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> > > > > kernel module rule in secure_boot policy to allow either an IMA
> > > > > signature OR an appended signature i.e. to use
> > > > > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
> > > > >
> > > > > Reported-by: Karel Srot <ksrot@redhat.com>
> > > > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > > ---
> > > > > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
> > > > >
> > > > > include/linux/lsm_hook_defs.h | 2 ++
> > > > > include/linux/security.h | 7 +++++++
> > > > > kernel/module/main.c | 10 +++++++++-
> > > > > security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
> > > > > security/integrity/ima/ima_policy.c | 2 +-
> > > > > security/security.c | 17 +++++++++++++++++
> > > > > 6 files changed, 62 insertions(+), 2 deletions(-)
> > > >
> > > > We don't really need a new LSM hook for this do we? Can't we just
> > > > define a new file read type, e.g. READING_MODULE_DECOMPRESS, and do
> > > > another call to security_kernel_post_read_file() after the module is
> > > > unpacked? Something like the snippet below ...
> > >
> > > Yes, this is similar to my suggestion based on defining multiple enumerations:
> > > READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE.
> > > With this solution, IMA would need to make an exception in the post kernel
> > > module read for the READING_COMPRESSED_MODULE case, since the kernel module has
> > > not yet been decompressed.
> > >
> > > Coiby suggested further simplification by moving the call later. At which point
> > > either there is or isn't an appended signature for non-compressed and
> > > decompressed kernel modules.
> > >
> > > As long as you don't have a problem calling the security_kernel_post_read_file()
> > > hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?
> >
> > It isn't clear from these comments if you are talking about moving
> > only the second security_kernel_post_read_file() call that was
> > proposed for init_module_from_file() to later in the function, leaving
> > the call in kernel_read_file() intact, or something else?
>
> Hi Paul and Mimi,
>
> Thanks for sharing your feedback! Yes, you are right, there is no need
> for a new LSM hook. Actually by not introducing a new LSM hook, we can
> have a much simpler solution!
>
> >
> > I think we want to leave the hook calls in kernel_read_file() intact,
> > in which case I'm not certain what advantage there is in moving the
> > security_kernel_post_read_file() call to a location where it is called
> > in init_module_from_file() regardless of if the module is compressed
> > or not. In the uncompressed case you are calling the hook twice for
> > no real benefit? It may be helpful to submit a patch with your
> > proposal as a patch can be worth a thousand words ;)
> >
> >
> > > > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > > > index c66b26184936..f127000d2e0a 100644
> > > > --- a/kernel/module/main.c
> > > > +++ b/kernel/module/main.c
> > > > @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
> > > > ar __user * uargs, int
> > > > mod_stat_add_long(len, &invalid_decompress_bytes);
> > > > return err;
> > > > }
> > > > +
> > > > + err = security_kernel_post_read_file(f,
> > > > + (char *)info.hdr, info.len,
> > > > + READING_MODULE_DECOMPRESS);
> > > > + if (err) {
> > > > + mod_stat_inc(&failed_kreads);
> > > > + return err;
> > > > + }
> > > > } else {
> > > > info.hdr = buf;
> > > > info.len = len;
> > >
> > > == defer security_kernel_post_read_file() call to here ==
>
> By moving security_kernel_post_read_file, I think what Mimi means is to
> move security_kernel_post_read_file in init_module_from_file() to later
> in the function,
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362a..66725e53fef0c1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> struct load_info info = { };
> void *buf = NULL;
> int len;
> + int err;
>
> len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> if (len < 0) {
> @@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> }
>
> if (flags & MODULE_INIT_COMPRESSED_FILE) {
> - int err = module_decompress(&info, buf, len);
> + err = module_decompress(&info, buf, len);
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> mod_stat_inc(&failed_decompress);
> @@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> info.len = len;
> }
>
> + err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
> + READING_MODULE);
> + if (err) {
> + mod_stat_inc(&failed_kreads);
> + free_copy(&info, flags);
> + return err;
> + }
> +
> return load_module(&info, uargs, flags);
> }
>
> If we only call security_kernel_post_read_file the 2nd time for a
> decompressed kernel module, IMA won't be sure what to do when
> security_kernel_post_read_file is called for the 1st time because it
> can't distinguish between a compressed module with appended signature or
> a uncompressed module without appended signature. If it permits 1st
> calling security_kernel_post_read_file, a uncompressed module without
> appended signature can be loaded. If it doesn't permit 1st calling
> security_kernel_post_read_file, there is no change to call
> security_kernel_post_read_file again for decompressed module.
>
> And you are right, there is no need to call
> security_kernel_post_read_file twice. And from the perspective of IMA,
> it simplifies reasoning if it is guaranteed that IMA will always access
> uncompressed kernel module regardless regardless of its original
> compression state.
>
> So I think a better solution is to stop calling
> security_kernel_post_read_file in kernel_read_file for READING_MODULE.
> This can also avoiding introducing an unnecessary
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
> can make the solution even simpler,
>
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index de32c95d823dbd..7c78e84def6ec7 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
> goto out_free;
> }
>
> - ret = security_kernel_post_read_file(file, *buf, i_size, id);
> + /*
> + * security_kernel_post_read_file will be called later after
> + * a read kernel module is truly decompressed
> + */
> + if (id != READING_MODULE)
> + ret = security_kernel_post_read_file(file, *buf, i_size, id);
> }
>
> Btw, I notice IMA is the only user of security_kernel_post_read_file so
> this change won't affect other LSMs. For a full patch, please visit
> https://github.com/coiby/linux/commit/558d85779ab5d794874749ecfae0e48b890bf3e0.patch
>
> If there are concerns that I'm unaware of and a new
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration is
> necessary, here's another patch
> https://github.com/coiby/linux/commit/cdd40317b6070f48ec871c6a89428084f38ca083.patch
Hi Coiby,
Based on the conversation with Paul, there is no reason to remove the existing
security_kernel_post_read_file() call.
The changes are similar to the 2nd link, but a bit different.
- Define a single enumeration named READING_MODULE_COMPRESSED.
- In module/main.c add a new security_kernel_post_read_file() call immediately
after decompressing the kernel module. Like a previous version of this patch,
call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
based on MODULE_INIT_COMPRESSED_FILE.
- In ima_post_read_file() defer verifying the signature when the enumeration is
READING_MODULE_COMPRESSED. (No need for a new function ima_read_kernel_module.)
thanks,
Mimi
^ permalink raw reply
* Re: [syzbot] [fs?] BUG: sleeping function called from invalid context in hook_sb_delete
From: syzbot @ 2025-11-05 20:56 UTC (permalink / raw)
To: brauner, eadavis, gnoack, hdanton, jack, jannh, linux-fsdevel,
linux-kernel, linux-security-module, m, max.kellermann, mic,
mjguzik, syzkaller-bugs, viro
In-Reply-To: <20251105193800.2340868-1-mic@digikod.net>
Hello,
syzbot tried to test the proposed patch but the build/boot failed:
f
[ 104.167925][ T5820] ? clear_bhb_loop+0x60/0xb0
[ 104.167948][ T5820] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 104.167967][ T5820] RIP: 0033:0x7f9a9fef16c5
[ 104.167983][ T5820] Code: Unable to access opcode bytes at 0x7f9a9fef169b.
[ 104.167993][ T5820] RSP: 002b:00007fff0fc3e0f8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 104.168021][ T5820] RAX: ffffffffffffffda RBX: 00005583ec761b10 RCX: 00007f9a9fef16c5
[ 104.168036][ T5820] RDX: 00000000000000e7 RSI: fffffffffffffe68 RDI: 0000000000000000
[ 104.168048][ T5820] RBP: 00005583ec738910 R08: 0000000000000000 R09: 0000000000000000
[ 104.168060][ T5820] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 104.168071][ T5820] R13: 00007fff0fc3e140 R14: 0000000000000000 R15: 0000000000000000
[ 104.168101][ T5820] </TASK>
2025/11/05 20:54:56 parsed 1 programs
[ 105.509351][ T5829] BUG: sleeping function called from invalid context at fs/inode.c:1920
[ 105.518601][ T5829] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 5829, name: syz-execprog
[ 105.528439][ T5829] preempt_count: 1, expected: 0
[ 105.533521][ T5829] RCU nest depth: 0, expected: 0
[ 105.538811][ T5829] 1 lock held by syz-execprog/5829:
[ 105.544194][ T5829] #0: ffff88807e6f68d8 (&sb->s_type->i_lock_key#9){+.+.}-{3:3}, at: iput+0x2db/0x1050
[ 105.554222][ T5829] Preemption disabled at:
[ 105.554232][ T5829] [<0000000000000000>] 0x0
[ 105.564065][ T5829] CPU: 0 UID: 0 PID: 5829 Comm: syz-execprog Tainted: G W syzkaller #0 PREEMPT(full)
[ 105.564091][ T5829] Tainted: [W]=WARN
[ 105.564096][ T5829] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
[ 105.564105][ T5829] Call Trace:
[ 105.564114][ T5829] <TASK>
[ 105.564121][ T5829] dump_stack_lvl+0x189/0x250
[ 105.564144][ T5829] ? __pfx_dump_stack_lvl+0x10/0x10
[ 105.564160][ T5829] ? __pfx__printk+0x10/0x10
[ 105.564176][ T5829] ? call_rcu+0x6ff/0x9c0
[ 105.564197][ T5829] ? print_lock_name+0xde/0x100
[ 105.564218][ T5829] __might_resched+0x495/0x610
[ 105.564241][ T5829] ? __pfx___might_resched+0x10/0x10
[ 105.564258][ T5829] ? do_raw_spin_lock+0x121/0x290
[ 105.564286][ T5829] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 105.564320][ T5829] iput+0x741/0x1050
[ 105.564352][ T5829] __dentry_kill+0x209/0x660
[ 105.564371][ T5829] ? dput+0x37/0x2b0
[ 105.564389][ T5829] dput+0x19f/0x2b0
[ 105.564406][ T5829] __fput+0x68e/0xa70
[ 105.564431][ T5829] fput_close_sync+0x113/0x220
[ 105.564450][ T5829] ? __pfx_fput_close_sync+0x10/0x10
[ 105.564470][ T5829] ? do_raw_spin_unlock+0x122/0x240
[ 105.564495][ T5829] __x64_sys_close+0x7f/0x110
[ 105.564517][ T5829] do_syscall_64+0xfa/0xfa0
[ 105.564541][ T5829] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 105.564558][ T5829] ? clear_bhb_loop+0x60/0xb0
[ 105.564577][ T5829] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 105.564593][ T5829] RIP: 0033:0x40dd0e
[ 105.564610][ T5829] Code: 24 28 44 8b 44 24 2c e9 70 ff ff ff cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 49 89 f2 48 89 fa 48 89 ce 48 89 df 0f 05 <48> 3d 01 f0 ff ff 76 15 48 f7 d8 48 89 c1 48 c7 c0 ff ff ff ff 48
[ 105.564625][ T5829] RSP: 002b:000000c002db1760 EFLAGS: 00000212 ORIG_RAX: 0000000000000003
[ 105.564688][ T5829] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000040dd0e
[ 105.564701][ T5829] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
[ 105.564711][ T5829] RBP: 000000c002db17a0 R08: 0000000000000000 R09: 0000000000000000
[ 105.564723][ T5829] R10: 0000000000000000 R11: 0000000000000212 R12: 000000c002db18c0
[ 105.564735][ T5829] R13: 00000000000007ff R14: 000000c000002380 R15: 000000c0008937c0
[ 105.564765][ T5829] </TASK>
[ 107.337546][ T5837] BUG: sleeping function called from invalid context at fs/inode.c:1920
[ 107.347227][ T5837] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 5837, name: dhcpcd
[ 107.356267][ T5837] preempt_count: 1, expected: 0
[ 107.361915][ T5837] RCU nest depth: 0, expected: 0
[ 107.367125][ T5837] 1 lock held by dhcpcd/5837:
[ 107.373083][ T5837] #0: ffff88807e6fa3d8 (&sb->s_type->i_lock_key#9){+.+.}-{3:3}, at: iput+0x2db/0x1050
[ 107.384068][ T5837] Preemption disabled at:
[ 107.384082][ T5837] [<0000000000000000>] 0x0
[ 107.393377][ T5837] CPU: 1 UID: 0 PID: 5837 Comm: dhcpcd Tainted: G W syzkaller #0 PREEMPT(full)
[ 107.393403][ T5837] Tainted: [W]=WARN
[ 107.393408][ T5837] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
[ 107.393417][ T5837] Call Trace:
[ 107.393424][ T5837] <TASK>
[ 107.393430][ T5837] dump_stack_lvl+0x189/0x250
[ 107.393456][ T5837] ? __pfx_dump_stack_lvl+0x10/0x10
[ 107.393480][ T5837] ? __pfx__printk+0x10/0x10
[ 107.393503][ T5837] ? print_lock_name+0xde/0x100
[ 107.393526][ T5837] __might_resched+0x495/0x610
[ 107.393553][ T5837] ? __pfx___might_resched+0x10/0x10
[ 107.393570][ T5837] ? do_raw_spin_lock+0x121/0x290
[ 107.393597][ T5837] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 107.393632][ T5837] iput+0x741/0x1050
[ 107.393662][ T5837] __dentry_kill+0x209/0x660
[ 107.393681][ T5837] ? dput+0x37/0x2b0
[ 107.393699][ T5837] dput+0x19f/0x2b0
[ 107.393717][ T5837] __fput+0x68e/0xa70
[ 107.393749][ T5837] fput_close_sync+0x113/0x220
[ 107.393768][ T5837] ? __pfx_fput_close_sync+0x10/0x10
[ 107.393790][ T5837] ? do_raw_spin_unlock+0x122/0x240
[ 107.393819][ T5837] __x64_sys_close+0x7f/0x110
[ 107.393843][ T5837] do_syscall_64+0xfa/0xfa0
[ 107.393871][ T5837] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 107.393890][ T5837] ? clear_bhb_loop+0x60/0xb0
[ 107.393915][ T5837] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 107.393933][ T5837] RIP: 0033:0x7fc58c16c407
[ 107.393950][ T5837] Code: 48 89 fa 4c 89 df e8 38 aa 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
[ 107.393968][ T5837] RSP: 002b:00007ffc1197cc80 EFLAGS: 00000202 ORIG_RAX: 0000000000000003
[ 107.393988][ T5837] RAX: ffffffffffffffda RBX: 00007fc58c0e2740 RCX: 00007fc58c16c407
[ 107.394002][ T5837] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
[ 107.394014][ T5837] RBP: 000055fc7ab074b0 R08: 0000000000000000 R09: 0000000000000000
[ 107.394026][ T5837] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
[ 107.394038][ T5837] R13: 000055fc83902290 R14: 0000000000000000 R15: 000055fc7ab1cac0
[ 107.394071][ T5837] </TASK>
[ 108.750656][ T5835] cgroup: Unknown subsys name 'net'
[ 108.759273][ T5835] BUG: sleeping function called from invalid context at fs/inode.c:1920
[ 108.769492][ T5835] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 5835, name: syz-executor
[ 108.782190][ T5835] preempt_count: 1, expected: 0
[ 108.787683][ T5835] RCU nest depth: 0, expected: 0
[ 108.792976][ T5835] 2 locks held by syz-executor/5835:
[ 108.799370][ T5835] #0: ffff8880340e80e0 (&type->s_umount_key#44){+.+.}-{4:4}, at: deactivate_super+0xa9/0xe0
[ 108.811870][ T5835] #1: ffff888077e41970 (&sb->s_type->i_lock_key#33){+.+.}-{3:3}, at: iput+0x2db/0x1050
[ 108.822811][ T5835] Preemption disabled at:
[ 108.822824][ T5835] [<0000000000000000>] 0x0
[ 108.833460][ T5835] CPU: 1 UID: 0 PID: 5835 Comm: syz-executor Tainted: G W syzkaller #0 PREEMPT(full)
[ 108.833488][ T5835] Tainted: [W]=WARN
[ 108.833493][ T5835] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
[ 108.833502][ T5835] Call Trace:
[ 108.833508][ T5835] <TASK>
[ 108.833515][ T5835] dump_stack_lvl+0x189/0x250
[ 108.833542][ T5835] ? __pfx_dump_stack_lvl+0x10/0x10
[ 108.833561][ T5835] ? __pfx__printk+0x10/0x10
[ 108.833580][ T5835] ? print_lock_name+0xde/0x100
[ 108.833601][ T5835] __might_resched+0x495/0x610
[ 108.833625][ T5835] ? __pfx___might_resched+0x10/0x10
[ 108.833642][ T5835] ? do_raw_spin_lock+0x121/0x290
[ 108.833666][ T5835] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 108.833699][ T5835] iput+0x741/0x1050
[ 108.833731][ T5835] __dentry_kill+0x209/0x660
[ 108.833751][ T5835] ? dput+0x37/0x2b0
[ 108.833770][ T5835] dput+0x19f/0x2b0
[ 108.833789][ T5835] shrink_dcache_for_umount+0xa0/0x170
[ 108.833815][ T5835] generic_shutdown_super+0x67/0x2c0
[ 108.833843][ T5835] kill_anon_super+0x3b/0x70
[ 108.833868][ T5835] kernfs_kill_sb+0x161/0x180
[ 108.833895][ T5835] deactivate_locked_super+0xbc/0x130
[ 108.833920][ T5835] cleanup_mnt+0x425/0x4c0
[ 108.833943][ T5835] ? lockdep_hardirqs_on+0x9c/0x150
[ 108.833970][ T5835] task_work_run+0x1d4/0x260
[ 108.833998][ T5835] ? __pfx_task_work_run+0x10/0x10
[ 108.834027][ T5835] ? exit_to_user_mode_loop+0x55/0x4f0
[ 108.834058][ T5835] exit_to_user_mode_loop+0xff/0x4f0
[ 108.834084][ T5835] ? rcu_is_watching+0x15/0xb0
[ 108.834109][ T5835] do_syscall_64+0x2e9/0xfa0
[ 108.834135][ T5835] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 108.834152][ T5835] ? clear_bhb_loop+0x60/0xb0
[ 108.834175][ T5835] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 108.834192][ T5835] RIP: 0033:0x7f2c235901f7
[ 108.834208][ T5835] Code: a8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 c7 c2 a8 ff ff ff f7 d8 64 89 02 b8
[ 108.834224][ T5835] RSP: 002b:00007ffeee90f4f8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
[ 108.834244][ T5835] RAX: 0000000000000000 RBX: 00007ffeee90f5f0 RCX: 00007f2c235901f7
[ 108.834256][ T5835] RDX: 00007f2c23623d15 RSI: 0000000000000000 RDI: 00007f2c236125ca
[ 108.834266][ T5835] RBP: 00007f2c236125ca R08: 00007f2c236128ae R09: 0000000000000000
[ 108.834277][ T5835] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f2c23612844
[ 108.834287][ T5835] R13: 00007f2c23623d15 R14: 00007ffeee90f608 R15: 00007ffeee90f500
[ 108.834313][ T5835] </TASK>
[ 109.214905][ T5835] cgroup: Unknown subsys name 'cpuset'
[ 109.226104][ T5835] cgroup: Unknown subsys name 'rlimit'
[ 110.666383][ T5835] Adding 124996k swap on ./swap-file. Priority:0 extents:1 across:124996k
[ 110.845546][ T5195] BUG: sleeping function called from invalid context at fs/inode.c:1920
[ 110.854984][ T5195] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 5195, name: udevd
[ 110.865089][ T5195] preempt_count: 1, expected: 0
[ 110.870130][ T5195] RCU nest depth: 0, expected: 0
[ 110.875858][ T5195] 2 locks held by udevd/5195:
[ 110.881834][ T5195] #0: ffff88802feb6420 (sb_writers#5){.+.+}-{0:0}, at: mnt_want_write+0x41/0x90
[ 110.893073][ T5195] #1: ffff8880306928e8 (&sb->s_type->i_lock_key){+.+.}-{3:3}, at: iput+0x2db/0x1050
[ 110.903490][ T5195] Preemption disabled at:
[ 110.903505][ T5195] [<0000000000000000>] 0x0
[ 110.912829][ T5195] CPU: 1 UID: 0 PID: 5195 Comm: udevd Tainted: G W syzkaller #0 PREEMPT(full)
[ 110.912861][ T5195] Tainted: [W]=WARN
[ 110.912867][ T5195] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
[ 110.912878][ T5195] Call Trace:
[ 110.912886][ T5195] <TASK>
[ 110.912894][ T5195] dump_stack_lvl+0x189/0x250
[ 110.912924][ T5195] ? __pfx_dump_stack_lvl+0x10/0x10
[ 110.912946][ T5195] ? __pfx__printk+0x10/0x10
[ 110.912969][ T5195] ? print_lock_name+0xde/0x100
[ 110.912994][ T5195] __might_resched+0x495/0x610
[ 110.913020][ T5195] ? __pfx___might_resched+0x10/0x10
[ 110.913036][ T5195] ? do_raw_spin_lock+0x121/0x290
[ 110.913063][ T5195] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 110.913095][ T5195] iput+0x741/0x1050
[ 110.913122][ T5195] do_unlinkat+0x39f/0x560
[ 110.913155][ T5195] ? __pfx_do_unlinkat+0x10/0x10
[ 110.913182][ T5195] ? strncpy_from_user+0x150/0x2c0
[ 110.913209][ T5195] ? getname_flags+0x1e5/0x540
[ 110.913232][ T5195] __x64_sys_unlink+0x47/0x50
[ 110.913267][ T5195] do_syscall_64+0xfa/0xfa0
[ 110.913295][ T5195] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 110.913313][ T5195] ? clear_bhb_loop+0x60/0xb0
[ 110.913335][ T5195] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 110.913354][ T5195] RIP: 0033:0x7f9a9ff15937
[ 110.913371][ T5195] Code: 00 00 e9 a9 fd ff ff 66 2e 0f 1f 84 00 00 00 00 00 66 90 b8 5f 00 00 00 0f 05 c3 0f 1f 84 00 00 00 00 00 b8 57 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 01 c3 48 8b 15 91 b4 0d 00 f7 d8 64 89 02 b8
[ 110.913387][ T5195] RSP: 002b:00007fff0fc3e2a8 EFLAGS: 00000202 ORIG_RAX: 0000000000000057
[ 110.913408][ T5195] RAX: ffffffffffffffda RBX: 0000000000000bb8 RCX: 00007f9a9ff15937
[ 110.913422][ T5195] RDX: ffffffffffffffff RSI: 000000000000000b RDI: 00005583c5bc802e
[ 110.913434][ T5195] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 110.913445][ T5195] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
[ 110.913456][ T5195] R13: 00005583c5be3100 R14: 0000000000000000 R15: 0000000000000000
[ 110.913486][ T5195] </TASK>
syzkaller build log:
go env (err=<nil>)
AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE='auto'
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/syzkaller/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/syzkaller/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2124321294=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/syzkaller/jobs/linux/gopath/src/github.com/google/syzkaller/go.mod'
GOMODCACHE='/syzkaller/jobs/linux/gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/syzkaller/jobs/linux/gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/syzkaller/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.4'
GOWORK=''
PKG_CONFIG='pkg-config'
git status (err=<nil>)
HEAD detached at 7e2882b3269
nothing to commit, working tree clean
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
go list -f '{{.Stale}}' -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=7e2882b32698b70f3149aee00c41e3d2d941dca3 -X github.com/google/syzkaller/prog.gitRevisionDate=20251007-152513" ./sys/syz-sysgen | grep -q false || go install -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=7e2882b32698b70f3149aee00c41e3d2d941dca3 -X github.com/google/syzkaller/prog.gitRevisionDate=20251007-152513" ./sys/syz-sysgen
make .descriptions
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
bin/syz-sysgen
touch .descriptions
GOOS=linux GOARCH=amd64 go build -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=7e2882b32698b70f3149aee00c41e3d2d941dca3 -X github.com/google/syzkaller/prog.gitRevisionDate=20251007-152513" -o ./bin/linux_amd64/syz-execprog github.com/google/syzkaller/tools/syz-execprog
mkdir -p ./bin/linux_amd64
g++ -o ./bin/linux_amd64/syz-executor executor/executor.cc \
-m64 -O2 -pthread -Wall -Werror -Wparentheses -Wunused-const-variable -Wframe-larger-than=16384 -Wno-stringop-overflow -Wno-array-bounds -Wno-format-overflow -Wno-unused-but-set-variable -Wno-unused-command-line-argument -static-pie -std=c++17 -I. -Iexecutor/_include -DGOOS_linux=1 -DGOARCH_amd64=1 \
-DHOSTGOOS_linux=1 -DGIT_REVISION=\"7e2882b32698b70f3149aee00c41e3d2d941dca3\"
/usr/bin/ld: /tmp/ccT2jI60.o: in function `Connection::Connect(char const*, char const*)':
executor.cc:(.text._ZN10Connection7ConnectEPKcS1_[_ZN10Connection7ConnectEPKcS1_]+0x104): warning: Using 'gethostbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
./tools/check-syzos.sh 2>/dev/null
Error text is too large and was truncated, full error text is at:
https://syzkaller.appspot.com/x/error.txt?x=11651084580000
Tested on:
commit: 84d39fb9 Add linux-next specific files for 20251105
git tree: linux-next
kernel config: https://syzkaller.appspot.com/x/.config?x=bebc0cb9c2989b81
dashboard link: https://syzkaller.appspot.com/bug?extid=12479ae15958fc3f54ec
compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch: https://syzkaller.appspot.com/x/patch.diff?x=139f532f980000
^ permalink raw reply
* [PATCH 1/2] fs: add iput_not_last()
From: Mateusz Guzik @ 2025-11-05 21:20 UTC (permalink / raw)
To: mic, brauner
Cc: linux-security-module, linux-fsdevel, viro, eadavis, gnoack, jack,
jannh, max.kellermann, m, syzbot+12479ae15958fc3f54ec,
Mateusz Guzik
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
fs/inode.c | 12 ++++++++++++
include/linux/fs.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/fs/inode.c b/fs/inode.c
index ec9339024ac3..cff1d3af0d57 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1967,6 +1967,18 @@ void iput(struct inode *inode)
}
EXPORT_SYMBOL(iput);
+/**
+ * iput_not_last - put an inode assuming this is not the last reference
+ * @inode: inode to put
+ */
+void iput_not_last(struct inode *inode)
+{
+ VFS_BUG_ON_INODE(atomic_read(&inode->i_count) < 2, inode);
+
+ WARN_ON(atomic_sub_return(1, &inode->i_count) == 0);
+}
+EXPORT_SYMBOL(iput_not_last);
+
#ifdef CONFIG_BLOCK
/**
* bmap - find a block number in a file
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c895146c1444..98fc088a461f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2823,6 +2823,7 @@ extern int current_umask(void);
extern void ihold(struct inode * inode);
extern void iput(struct inode *);
+void iput_not_last(struct inode *);
int inode_update_timestamps(struct inode *inode, int flags);
int generic_update_time(struct inode *, int);
--
2.48.1
^ permalink raw reply related
* [PATCH 2/2] landlock: fix splats from iput() after it started calling might_sleep()
From: Mateusz Guzik @ 2025-11-05 21:20 UTC (permalink / raw)
To: mic, brauner
Cc: linux-security-module, linux-fsdevel, viro, eadavis, gnoack, jack,
jannh, max.kellermann, m, syzbot+12479ae15958fc3f54ec,
Mateusz Guzik
In-Reply-To: <20251105212025.807549-1-mjguzik@gmail.com>
At this point it is guaranteed this is not the last reference.
However, a recent addition of might_sleep() at top of iput() started
generating false-positives as it was executing for all values.
Remedy the problem by using the newly introduced iput_not_last().
Reported-by: syzbot+12479ae15958fc3f54ec@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/68d32659.a70a0220.4f78.0012.GAE@google.com/
Fixes: 2ef435a872ab ("fs: add might_sleep() annotation to iput() and more")
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
security/landlock/fs.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 0bade2c5aa1d..d9c12b993fa7 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1335,11 +1335,10 @@ static void hook_sb_delete(struct super_block *const sb)
* At this point, we own the ihold() reference that was
* originally set up by get_inode_object() and the
* __iget() reference that we just set in this loop
- * walk. Therefore the following call to iput() will
- * not sleep nor drop the inode because there is now at
- * least two references to it.
+ * walk. Therefore there are at least two references
+ * on the inode.
*/
- iput(inode);
+ iput_not_last(inode);
} else {
spin_unlock(&object->lock);
rcu_read_unlock();
--
2.48.1
^ permalink raw reply related
* [PATCH v3 0/2] ipe: add script enforcement mechanism with AT_EXECVE_CHECK
From: Yanzhu Huang @ 2025-11-05 23:26 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
Indirect file execution through interpreters (e.g. python script.py, sh
script.sh) should have integrity policy enforced by IPE based on the
rules. Currently, IPE can only enforce policy on the interpreter binary
itself, but has no visibility into the scripts that the interpreter
executes.
Overview
--------
This patch series introduces script enforcement for IPE, allowing integrity
evaluation of indirectly executed scripts through the AT_EXECVE_CHECK flag.
Patch 1 adds the core implementation with ipe_bprm_creds_for_exec() hook
that integrates with the AT_EXECVE_CHECK mechanism.
Patch 2 updates admin guide documentation to explain the script enforcement
mechanism.
The IPE test suite has been updated to include script enforcement tests:
https://github.com/microsoft/ipe/pull/6
Changes since v2:
- update AT_EXECVE_CHECK reference
Changes since v1:
- update the interpreters reference
Yanzhu Huang (2):
ipe: Add AT_EXECVE_CHECK support for script enforcement
ipe: Update documentation for script enforcement
Documentation/admin-guide/LSM/ipe.rst | 17 ++++++++++++++---
security/ipe/audit.c | 1 +
security/ipe/hooks.c | 27 +++++++++++++++++++++++++++
security/ipe/hooks.h | 3 +++
security/ipe/ipe.c | 1 +
5 files changed, 46 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 1/2] ipe: Add AT_EXECVE_CHECK support for script enforcement
From: Yanzhu Huang @ 2025-11-05 23:26 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
In-Reply-To: <20251105232615.720861-1-yanzhuhuang@linux.microsoft.com>
This patch adds a new ipe_bprm_creds_for_exec() hook that integrates
with the AT_EXECVE_CHECK mechanism. To enable script enforcement,
interpreters need to incorporate the AT_EXECVE_CHECK flag when
calling execveat() on script files before execuation.
When a userspace interpreter calls execveat() with the AT_EXECVE_CHECK
flag, this hook triggers IPE policy evaluation on the script file. The
hook only triggers IPE when bprm->is_check is true, ensuring it's
being called from an AT_EXECVE_CHECK context. It then builds an
evaluation context for an IPE_OP_EXEC operation and invokes IPE policy.
The kernel returns the policy decision to the interpreter, which can
then decide whether to proceed with script execution.
This extends IPE enforcement to indirectly executed scripts, permitting
trusted scripts to execute while denying untrusted ones.
Signed-off-by: Yanzhu Huang <yanzhuhuang@linux.microsoft.com>
---
security/ipe/audit.c | 1 +
security/ipe/hooks.c | 27 +++++++++++++++++++++++++++
security/ipe/hooks.h | 3 +++
security/ipe/ipe.c | 1 +
4 files changed, 32 insertions(+)
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
index de5fed62592e..3f0deeb54912 100644
--- a/security/ipe/audit.c
+++ b/security/ipe/audit.c
@@ -46,6 +46,7 @@ static const char *const audit_op_names[__IPE_OP_MAX + 1] = {
static const char *const audit_hook_names[__IPE_HOOK_MAX] = {
"BPRM_CHECK",
+ "BPRM_CREDS_FOR_EXEC",
"MMAP",
"MPROTECT",
"KERNEL_READ",
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..32dd99abd4de 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -35,6 +35,33 @@ int ipe_bprm_check_security(struct linux_binprm *bprm)
return ipe_evaluate_event(&ctx);
}
+/**
+ * ipe_bprm_creds_for_exec() - ipe security hook function for bprm creds check.
+ * @bprm: Supplies a pointer to a linux_binprm structure to source the file
+ * being evaluated.
+ *
+ * This LSM hook is called when userspace signals the kernel to check a file
+ * for execution through the execveat syscall with the AT_EXECVE_CHECK flag.
+ * The hook triggers IPE policy evaluation on the script file and returns
+ * the policy decision to userspace. The userspace program receives the
+ * return code and can decide whether to proceed with script execution.
+ *
+ * Return:
+ * * %0 - Success
+ * * %-EACCES - Did not pass IPE policy
+ */
+int ipe_bprm_creds_for_exec(struct linux_binprm *bprm)
+{
+ struct ipe_eval_ctx ctx = IPE_EVAL_CTX_INIT;
+
+ if (!bprm->is_check)
+ return 0;
+
+ ipe_build_eval_ctx(&ctx, bprm->file, IPE_OP_EXEC,
+ IPE_HOOK_BPRM_CREDS_FOR_EXEC);
+ return ipe_evaluate_event(&ctx);
+}
+
/**
* ipe_mmap_file() - ipe security hook function for mmap check.
* @f: File being mmap'd. Can be NULL in the case of anonymous memory.
diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
index 38d4a387d039..07db37332740 100644
--- a/security/ipe/hooks.h
+++ b/security/ipe/hooks.h
@@ -13,6 +13,7 @@
enum ipe_hook_type {
IPE_HOOK_BPRM_CHECK = 0,
+ IPE_HOOK_BPRM_CREDS_FOR_EXEC,
IPE_HOOK_MMAP,
IPE_HOOK_MPROTECT,
IPE_HOOK_KERNEL_READ,
@@ -24,6 +25,8 @@ enum ipe_hook_type {
int ipe_bprm_check_security(struct linux_binprm *bprm);
+int ipe_bprm_creds_for_exec(struct linux_binprm *bprm);
+
int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
unsigned long flags);
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index 4317134cb0da..845e3fd7a345 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -47,6 +47,7 @@ struct ipe_inode *ipe_inode(const struct inode *inode)
static struct security_hook_list ipe_hooks[] __ro_after_init = {
LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
+ LSM_HOOK_INIT(bprm_creds_for_exec, ipe_bprm_creds_for_exec),
LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/2] ipe: Update documentation for script enforcement
From: Yanzhu Huang @ 2025-11-05 23:26 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
In-Reply-To: <20251105232615.720861-1-yanzhuhuang@linux.microsoft.com>
This patch adds explanation of script enforcement mechanism in admin
guide documentation. Describes how IPE supports integrity enforcement
for indirectly executed scripts through the AT_EXECVE_CHECK flag, and
how this differs from kernel enforcement for compiled executables.
Signed-off-by: Yanzhu Huang <yanzhuhuang@linux.microsoft.com>
---
Documentation/admin-guide/LSM/ipe.rst | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index dc7088451f9d..a756d8158531 100644
--- a/Documentation/admin-guide/LSM/ipe.rst
+++ b/Documentation/admin-guide/LSM/ipe.rst
@@ -95,7 +95,20 @@ languages when these scripts are invoked by passing these program files
to the interpreter. This is because the way interpreters execute these
files; the scripts themselves are not evaluated as executable code
through one of IPE's hooks, but they are merely text files that are read
-(as opposed to compiled executables) [#interpreters]_.
+(as opposed to compiled executables). However, with the introduction of the
+``AT_EXECVE_CHECK`` flag (:doc:`AT_EXECVE_CHECK </userspace-api/check_exec>`),
+interpreters can use it to signal the kernel that a script file will be executed,
+and request the kernel to perform LSM security checks on it.
+
+IPE's EXECUTE operation enforcement differs between compiled executables and
+interpreted scripts: For compiled executables, enforcement is triggered
+automatically by the kernel during ``execve()``, ``execveat()``, ``mmap()``
+and ``mprotect()`` syscalls when loading executable content. For interpreted
+scripts, enforcement requires explicit interpreter integration using
+``execveat()`` with ``AT_EXECVE_CHECK`` flag. Unlike exec syscalls that IPE
+intercepts during the execution process, this mechanism needs the interpreter
+to take the initiative, and existing interpreters won't be automatically
+supported unless the signal call is added.
Threat Model
------------
@@ -806,8 +819,6 @@ A:
.. [#digest_cache_lsm] https://lore.kernel.org/lkml/20240415142436.2545003-1-roberto.sassu@huaweicloud.com/
-.. [#interpreters] There is `some interest in solving this issue <https://lore.kernel.org/lkml/20220321161557.495388-1-mic@digikod.net/>`_.
-
.. [#devdoc] Please see :doc:`the design docs </security/ipe>` for more on
this topic.
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 0/2] ipe: add script enforcement mechanism with AT_EXECVE_CHECK
From: Fan Wu @ 2025-11-05 23:39 UTC (permalink / raw)
To: Yanzhu Huang
Cc: wufan, paul, mic, jmorris, serge, corbet, linux-security-module,
linux-doc, linux-kernel
In-Reply-To: <20251105232615.720861-1-yanzhuhuang@linux.microsoft.com>
On Wed, Nov 5, 2025 at 3:26 PM Yanzhu Huang
<yanzhuhuang@linux.microsoft.com> wrote:
>
> Indirect file execution through interpreters (e.g. python script.py, sh
> script.sh) should have integrity policy enforced by IPE based on the
> rules. Currently, IPE can only enforce policy on the interpreter binary
> itself, but has no visibility into the scripts that the interpreter
> executes.
>
> Overview
> --------
>
> This patch series introduces script enforcement for IPE, allowing integrity
> evaluation of indirectly executed scripts through the AT_EXECVE_CHECK flag.
>
> Patch 1 adds the core implementation with ipe_bprm_creds_for_exec() hook
> that integrates with the AT_EXECVE_CHECK mechanism.
>
> Patch 2 updates admin guide documentation to explain the script enforcement
> mechanism.
>
> The IPE test suite has been updated to include script enforcement tests:
> https://github.com/microsoft/ipe/pull/6
>
> Changes since v2:
> - update AT_EXECVE_CHECK reference
>
> Changes since v1:
> - update the interpreters reference
>
> Yanzhu Huang (2):
> ipe: Add AT_EXECVE_CHECK support for script enforcement
> ipe: Update documentation for script enforcement
>
> Documentation/admin-guide/LSM/ipe.rst | 17 ++++++++++++++---
> security/ipe/audit.c | 1 +
> security/ipe/hooks.c | 27 +++++++++++++++++++++++++++
> security/ipe/hooks.h | 3 +++
> security/ipe/ipe.c | 1 +
> 5 files changed, 46 insertions(+), 3 deletions(-)
>
> --
> 2.43.0
>
Thanks, applied to ipe/next.
-Fan
^ permalink raw reply
* [PATCH v5 00/14] Create and use APIs to centralise locking for directory ops.
From: NeilBrown @ 2025-11-06 0:50 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Amir Goldstein
Cc: Jan Kara, linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Miklos Szeredi, Chuck Lever,
Olga Kornievskaia, Dai Ngo, Namjae Jeon, Steve French,
Sergey Senozhatsky, Carlos Maiolino, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek,
Mateusz Guzik, Lorenzo Stoakes, Stefan Berger, Darrick J. Wong,
linux-kernel, netfs, ecryptfs, linux-nfs, linux-unionfs,
linux-cifs, linux-xfs, linux-security-module, selinux
This version contains a couple of fixes thanks to Al's review and Dan's
robot, and a couple of s-o-b and a-b tags.
It is based on vfs-all (e90fbb585e64).
Previous description:
this series is the next part of my effort to change directory-op
locking to allow multiple concurrent ops in a directory. Ultimately we
will (in my plan) lock the target dentry(s) rather than the whole
parent directory.
To help with changing the locking protocol, this series centralises
locking and lookup in some helpers. The various helpers are introduced
and then used in the same patch - roughly one patch per helper though
with various exceptions.
I haven't introduced these helpers into the various filesystems that
Al's tree-in-dcache series is changing. That series introduces and
uses similar helpers tuned to the specific needs of that set of
filesystems. Ultimately all the helpers will use the same backends
which can then be adjusted when it is time to change the locking
protocol.
One change that deserves highlighting is in patch 13 where vfs_mkdir()
is changed to unlock the parent on failure, as well as the current
behaviour of dput()ing the dentry on failure. Once this change is in
place, the final step of both create and an remove sequences only
requires the target dentry, not the parent. So e.g. end_creating() is
only given the dentry (which may be IS_ERR() after vfs_mkdir()). This
helps establish the pattern that it is the dentry that is being locked
and unlocked (the lock is currently held on dentry->d_parent->d_inode,
but that can change).
Please review the changes I've made to your respective code areas and
let us know of any problems.
Thanks,
NeilBrown
[PATCH v5 01/14] debugfs: rename end_creating() to
[PATCH v5 02/14] VFS: introduce start_dirop() and end_dirop()
[PATCH v5 03/14] VFS: tidy up do_unlinkat()
[PATCH v5 04/14] VFS/nfsd/cachefiles/ovl: add start_creating() and
[PATCH v5 05/14] VFS/nfsd/cachefiles/ovl: introduce start_removing()
[PATCH v5 06/14] VFS: introduce start_creating_noperm() and
[PATCH v5 07/14] VFS: introduce start_removing_dentry()
[PATCH v5 08/14] VFS: add start_creating_killable() and
[PATCH v5 09/14] VFS/nfsd/ovl: introduce start_renaming() and
[PATCH v5 10/14] VFS/ovl/smb: introduce start_renaming_dentry()
[PATCH v5 11/14] Add start_renaming_two_dentries()
[PATCH v5 12/14] ecryptfs: use new start_creating/start_removing APIs
[PATCH v5 13/14] VFS: change vfs_mkdir() to unlock on failure.
[PATCH v5 14/14] VFS: introduce end_creating_keep()
^ 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