* [PATCH] smack: simplify write handlers of sysfs entries
From: Dmitry Antipov @ 2026-03-20 11:31 UTC (permalink / raw)
To: Casey Schaufler, Paul Moore, James Morris, Serge E. Hallyn,
Konstantin Andreev
Cc: linux-security-module, Dmitry Antipov
Use the convenient 'kstrto{u,s}32_from_user()' to simplify write
handlers of /smack/{doi,direct,mapped,logging,ptrace} sysfs entries.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
security/smack/smackfs.c | 81 +++++++++++-----------------------------
1 file changed, 22 insertions(+), 59 deletions(-)
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 6e62dcb36f74..f60d5469043e 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1598,24 +1598,17 @@ static ssize_t smk_read_doi(struct file *filp, char __user *buf,
static ssize_t smk_write_doi(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
- char temp[80];
- unsigned long u;
+ int ret;
+ u32 u;
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
- if (count >= sizeof(temp) || count == 0)
- return -EINVAL;
-
- if (copy_from_user(temp, buf, count) != 0)
- return -EFAULT;
-
- temp[count] = '\0';
+ ret = kstrtou32_from_user(buf, count, 10, &u);
+ if (unlikely(ret))
+ return ret;
- if (kstrtoul(temp, 10, &u))
- return -EINVAL;
-
- if (u == CIPSO_V4_DOI_UNKNOWN || u > U32_MAX)
+ if (u == CIPSO_V4_DOI_UNKNOWN)
return -EINVAL;
return smk_cipso_doi(u, GFP_KERNEL) ? : count;
@@ -1664,22 +1657,14 @@ static ssize_t smk_write_direct(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
struct smack_known *skp;
- char temp[80];
- int i;
+ int i, ret;
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
- if (count >= sizeof(temp) || count == 0)
- return -EINVAL;
-
- if (copy_from_user(temp, buf, count) != 0)
- return -EFAULT;
-
- temp[count] = '\0';
-
- if (sscanf(temp, "%d", &i) != 1)
- return -EINVAL;
+ ret = kstrtos32_from_user(buf, count, 10, &i);
+ if (unlikely(ret))
+ return ret;
/*
* Don't do anything if the value hasn't actually changed.
@@ -1742,22 +1727,14 @@ static ssize_t smk_write_mapped(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
struct smack_known *skp;
- char temp[80];
- int i;
+ int i, ret;
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
- if (count >= sizeof(temp) || count == 0)
- return -EINVAL;
-
- if (copy_from_user(temp, buf, count) != 0)
- return -EFAULT;
-
- temp[count] = '\0';
-
- if (sscanf(temp, "%d", &i) != 1)
- return -EINVAL;
+ ret = kstrtos32_from_user(buf, count, 10, &i);
+ if (unlikely(ret))
+ return ret;
/*
* Don't do anything if the value hasn't actually changed.
@@ -2179,22 +2156,15 @@ static ssize_t smk_read_logging(struct file *filp, char __user *buf,
static ssize_t smk_write_logging(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
- char temp[32];
- int i;
+ int i, ret;
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
- if (count >= sizeof(temp) || count == 0)
- return -EINVAL;
-
- if (copy_from_user(temp, buf, count) != 0)
- return -EFAULT;
+ ret = kstrtos32_from_user(buf, count, 10, &i);
+ if (unlikely(ret))
+ return ret;
- temp[count] = '\0';
-
- if (sscanf(temp, "%d", &i) != 1)
- return -EINVAL;
if (i < 0 || i > 3)
return -EINVAL;
log_policy = i;
@@ -2838,22 +2808,15 @@ static ssize_t smk_read_ptrace(struct file *filp, char __user *buf,
static ssize_t smk_write_ptrace(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
- char temp[32];
- int i;
+ int i, ret;
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
- if (*ppos != 0 || count >= sizeof(temp) || count == 0)
- return -EINVAL;
-
- if (copy_from_user(temp, buf, count) != 0)
- return -EFAULT;
+ ret = kstrtos32_from_user(buf, count, 10, &i);
+ if (unlikely(ret))
+ return ret;
- temp[count] = '\0';
-
- if (sscanf(temp, "%d", &i) != 1)
- return -EINVAL;
if (i < SMACK_PTRACE_DEFAULT || i > SMACK_PTRACE_MAX)
return -EINVAL;
smack_ptrace_rule = i;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v6 5/9] landlock/selftests: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
From: Günther Noack @ 2026-03-20 10:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: John Johansen, Justin Suess, Tingmao Wang, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260318.eghahf0Chiez@digikod.net>
On Wed, Mar 18, 2026 at 05:53:15PM +0100, Mickaël Salaün wrote:
> The subject's prefix is swapped, it should be "selftests/landlock".
Thanks, done!
–Günther
^ permalink raw reply
* Re: [PATCH v6 2/9] landlock: use mem_is_zero() in is_layer_masks_allowed()
From: Günther Noack @ 2026-03-20 10:50 UTC (permalink / raw)
To: Mickaël Salaün
Cc: John Johansen, linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima
In-Reply-To: <20260318.ePee0acaeng8@digikod.net>
On Wed, Mar 18, 2026 at 05:52:10PM +0100, Mickaël Salaün wrote:
> Subject should be "landlock: Use..."
Thanks, done.
–Günther
^ permalink raw reply
* Re: [PATCH RESEND] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-20 3:24 UTC (permalink / raw)
To: stephen.smalley.work, casey, jmorris, paul, serge
Cc: linux-kernel, linux-security-module, yangfeng59949
In-Reply-To: <CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@mail.gmail.com>
On Thu, 19 Mar 2026 14:22:06 -0400, Stephen Smalley wrote:
> On Thu, Mar 19, 2026 at 1:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 3/18/2026 7:22 PM, Feng Yang wrote:
> > > On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> > >> On 3/17/2026 11:19 PM, Feng Yang wrote:
> > >>> From: Feng Yang <yangfeng@kylinos.cn>
> > >>>
> > >>> After hooking the following BPF program:
> > >>> SEC("lsm/xfrm_decode_session")
> > >>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> > >>> {
> > >>> return 1; // Any non-zero value
> > >>> }
> > >>> Subsequent packet transmission triggers will cause a kernel panic:
> > >> LSM hooks that use or provide secids cannot be stacked. That is,
> > >> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> > >> correct behavior. Your proposed "fix" removes a legitimate check.
> > > I'm very sorry, I didn't quite understand what you meant.
> > >
> > > Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
> >
> > Do i understand correctly that you do not have SELinux enabled?
> >
> > > Therefore, is it the expected behavior that simply using
> > > SEC("lsm/xfrm_decode_session")
> > > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> > > return -1;
> > > }
> > > would cause a kernel panic?
> >
> > Yes. As the infrastructure is written, any return other than 0 is erroneous.
> > You will need to query the SELinux developers about why they decided to
> > implement the xfrm handling in this way.
>
> Looks like this BUG_ON() has just been preserved since first being
> introduced by:
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=beb8d13bed80f8388f1a9a107d07ddd342e627e8
Yes, there is a comment in https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/:
+static inline void security_xfrm_skb_secid(struct sk_buff *skb, u32 *secid)
{
- return security_ops->xfrm_decode_session(skb, fl);
+ BUG_ON(security_ops->xfrm_decode_session(skb, secid, 0));
BUG_ON looks wrong here, in that you don't know why the LSM returned an
error, and why should the box panic at this point at all?
But it seems no one has answered this question before.
> I think we can remove it at this point - it evidently doesn't ever
> trigger for SELinux or we would have seen it by now and it is
> definitely unsafe for BPF LSM.
Yes, the call in the SELinux module is as follows.
void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
{
int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
0);
BUG_ON(rc);
}
int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
{
if (skb == NULL) {
*sid = SECSID_NULL;
return 0;
}
return selinux_xfrm_skb_sid_ingress(skb, sid, ckall);
}
static int selinux_xfrm_skb_sid_ingress(struct sk_buff *skb,
u32 *sid, int ckall)
{
u32 sid_session = SECSID_NULL;
struct sec_path *sp = skb_sec_path(skb);
if (sp) {
int i;
for (i = sp->len - 1; i >= 0; i--) {
struct xfrm_state *x = sp->xvec[i];
if (selinux_authorizable_xfrm(x)) {
struct xfrm_sec_ctx *ctx = x->security;
if (sid_session == SECSID_NULL) {
sid_session = ctx->ctx_sid;
if (!ckall)
goto out;
} else if (sid_session != ctx->ctx_sid) {
*sid = SECSID_NULL;
return -EINVAL;
}
}
}
}
out:
*sid = sid_session;
return 0;
}
Since ckall is 0, selinux_xfrm_skb_sid_ingress will only return 0.
Therefore, the selinux_xfrm_decode_session function will only return 0 when ckall is 0, and BUG_ON will never be triggered.
However, BPF can be exploited to cause a system crash.
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-20 3:20 UTC (permalink / raw)
To: stephen.smalley.work, casey, jmorris; +Cc: linux-security-module, yangfeng59949
In-Reply-To: <CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@mail.gmail.com>
On Thu, 19 Mar 2026 14:22:06 -0400, Stephen Smalley wrote:
> On Thu, Mar 19, 2026 at 1:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 3/18/2026 7:22 PM, Feng Yang wrote:
> > > On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> > >> On 3/17/2026 11:19 PM, Feng Yang wrote:
> > >>> From: Feng Yang <yangfeng@kylinos.cn>
> > >>>
> > >>> After hooking the following BPF program:
> > >>> SEC("lsm/xfrm_decode_session")
> > >>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> > >>> {
> > >>> return 1; // Any non-zero value
> > >>> }
> > >>> Subsequent packet transmission triggers will cause a kernel panic:
> > >> LSM hooks that use or provide secids cannot be stacked. That is,
> > >> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> > >> correct behavior. Your proposed "fix" removes a legitimate check.
> > > I'm very sorry, I didn't quite understand what you meant.
> > >
> > > Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
> >
> > Do i understand correctly that you do not have SELinux enabled?
> >
> > > Therefore, is it the expected behavior that simply using
> > > SEC("lsm/xfrm_decode_session")
> > > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> > > return -1;
> > > }
> > > would cause a kernel panic?
> >
> > Yes. As the infrastructure is written, any return other than 0 is erroneous.
> > You will need to query the SELinux developers about why they decided to
> > implement the xfrm handling in this way.
>
> Looks like this BUG_ON() has just been preserved since first being
> introduced by:
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=beb8d13bed80f8388f1a9a107d07ddd342e627e8
Yes, there is a comment in https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/:
+static inline void security_xfrm_skb_secid(struct sk_buff *skb, u32 *secid)
{
- return security_ops->xfrm_decode_session(skb, fl);
+ BUG_ON(security_ops->xfrm_decode_session(skb, secid, 0));
BUG_ON looks wrong here, in that you don't know why the LSM returned an
error, and why should the box panic at this point at all?
But it seems no one has answered this question before.
> I think we can remove it at this point - it evidently doesn't ever
> trigger for SELinux or we would have seen it by now and it is
> definitely unsafe for BPF LSM.
Yes, the call in the SELinux module is as follows.
void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
{
int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
0);
BUG_ON(rc);
}
int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall)
{
if (skb == NULL) {
*sid = SECSID_NULL;
return 0;
}
return selinux_xfrm_skb_sid_ingress(skb, sid, ckall);
}
static int selinux_xfrm_skb_sid_ingress(struct sk_buff *skb,
u32 *sid, int ckall)
{
u32 sid_session = SECSID_NULL;
struct sec_path *sp = skb_sec_path(skb);
if (sp) {
int i;
for (i = sp->len - 1; i >= 0; i--) {
struct xfrm_state *x = sp->xvec[i];
if (selinux_authorizable_xfrm(x)) {
struct xfrm_sec_ctx *ctx = x->security;
if (sid_session == SECSID_NULL) {
sid_session = ctx->ctx_sid;
if (!ckall)
goto out;
} else if (sid_session != ctx->ctx_sid) {
*sid = SECSID_NULL;
return -EINVAL;
}
}
}
}
out:
*sid = sid_session;
return 0;
}
Since ckall is 0, selinux_xfrm_skb_sid_ingress will only return 0.
Therefore, the selinux_xfrm_decode_session function will only return 0 when ckall is 0, and BUG_ON will never be triggered.
However, BPF can be exploited to cause a system crash.
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-03-20 3:03 UTC (permalink / raw)
To: casey
Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
yangfeng59949
In-Reply-To: <80ff003b-51fd-4ec8-9505-dfc1c5890ced@schaufler-ca.com>
On Thu, 19 Mar 2026 10:51:24 -0700, Casey Schaufler wrote:
> On 3/18/2026 7:22 PM, Feng Yang wrote:
> > On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> >> On 3/17/2026 11:19 PM, Feng Yang wrote:
> >>> From: Feng Yang <yangfeng@kylinos.cn>
> >>>
> >>> After hooking the following BPF program:
> >>> SEC("lsm/xfrm_decode_session")
> >>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> >>> {
> >>> return 1; // Any non-zero value
> >>> }
> >>> Subsequent packet transmission triggers will cause a kernel panic:
> >> LSM hooks that use or provide secids cannot be stacked. That is,
> >> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> >> correct behavior. Your proposed "fix" removes a legitimate check.
> > I'm very sorry, I didn't quite understand what you meant.
> >
> > Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
>
> Do i understand correctly that you do not have SELinux enabled?
Yes.
>
> > Therefore, is it the expected behavior that simply using
> > SEC("lsm/xfrm_decode_session")
> > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> > return -1;
> > }
> > would cause a kernel panic?
>
> Yes. As the infrastructure is written, any return other than 0 is erroneous.
> You will need to query the SELinux developers about why they decided to
> implement the xfrm handling in this way.
>
> > If not, and if the BUG_ON check is still necessary,
> > then does it mean we need to modify the return value validation logic in the BPF
> > verifier to ensure that only BPF programs returning 0 are accepted for this hook?
> >
> > Thanks.
> >
> >
^ permalink raw reply
* Re: [PATCH v3 3/3] ima: Add support for staging measurements for deletion
From: steven chen @ 2026-03-19 21:31 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, corbet, skhan, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <587e11bf4d29552bbbfc029f716146e8ebfca1eb.camel@linux.ibm.com>
On 3/17/2026 2:03 PM, Mimi Zohar wrote:
> Hi Roberto,
>
> On Wed, 2026-03-11 at 18:19 +0100, Roberto Sassu wrote:
>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>
>> Introduce the ability of staging the IMA measurement list for deletion.
>> Staging means moving the current content of the measurement list to a
>> separate location, and allowing users to read and delete it. This causes
>> the measurement list to be atomically truncated before new measurements can
>> be added.
> I really like this design of atomically moving and subsequently deleting the
> measurement list. However this is a solution, not the motivation for the patch.
> Please include the motivation for the patch, before describing the solution.
>
>> Staging can be done only once at a time. In the event of kexec(),
>> staging is reverted and staged entries will be carried over to the new
>> kernel.
>> Staged measurements can be deleted entirely, or partially, with the
>> non-deleted ones added back to the IMA measurements list.
> This patch description is really long, which is an indication that the patch
> needs to be split up. Adding support for partially deleting the measurement
> list records, by prepending the remaining measurement records, should be a
> separate patch.
>
>> This allows the
>> remote attestation agents to easily separate the measurements that where
>> verified (staged and deleted) from those that weren't due to the race
>> between taking a TPM quote and reading the measurements list.
>>
>> User space is responsible to concatenate the staged IMA measurements list
>> portions (excluding the measurements added back to the IMA measurements
>> list) following the temporal order in which the operations were done,
>> together with the current measurement list. Then, it can send the collected
>> data to the remote verifiers.
> This belongs in a Documentation patch.
>
>> The benefit of staging and deleting is the ability to free precious kernel
>> memory,
> This is the motivation for the patch.
>
>> in exchange of delegating user space to reconstruct the full
>> measurement list from the chunks. No trust needs to be given to user space,
>> since the integrity of the measurement list is protected by the TPM.
> Agreed the measurement list, itself, is protected by the TPM. However, relying
> on userspace to reassemble the chunks is another concern. Support for staging
> and deleting the measurement list should be configurable. Defining a Kconfig
> should be part of this initial patch.
>
>> By default, staging the measurements list does not alter the hash table.
>> When staging and deleting are done, IMA is still able to detect collisions
>> on the staged and later deleted measurement entries, by keeping the entry
>> digests (only template data are freed).
>>
>> However, since during the measurements list serialization only the SHA1
>> digest is passed, and since there are no template data to recalculate the
>> other digests from, the hash table is currently not populated with digests
>> from staged/deleted entries after kexec().
>>
>> Introduce the new kernel option ima_flush_htable to decide whether or not
>> the digests of staged measurement entries are flushed from the hash table,
>> when they are deleted. Flushing the hash table is supported only when
>> deleting all the staged measurements, since in that case the old hash table
>> can be quickly swapped with a blank one (otherwise entries would have to be
>> removed one by one for partial deletion).
> Allowing the hash table to be deleted would be an example of another patch.
>
>> Then, introduce ascii_runtime_measurements_<algo>_staged and
>> binary_runtime_measurements_<algo>_staged interfaces to stage and delete
>> the measurements. Use 'echo A > <IMA interface>' and
>> 'echo D > <IMA interface>' to respectively stage and delete the entire
>> measurements list. Use 'echo N > <IMA interface>', with N between 1 and
>> ULONG_MAX - 1, to delete the selected staged portion of the measurements
>> list.
>>
>> The ima_measure_users counter (protected by the ima_measure_mutex mutex)
>> has been introduced to protect access to the measurements list and the
>> staged part. The open method of all the measurement interfaces has been
>> extended to allow only one writer at a time or, in alternative, multiple
>> readers. The write permission is used to stage and delete the measurements,
>> the read permission to read them. Write requires also the CAP_SYS_ADMIN
>> capability.
> Yes, this is part of the initial patch that adds support for staging the
> measurement list.
>
>> Finally, introduce the binary_lists enum and make binary_runtime_size
>> and ima_num_entries as arrays, to keep track of their values for the
>> current IMA measurements list (BINARY), current list plus staged
>> measurements (BINARY_STAGED) and the cumulative list since IMA
>> initialization (BINARY_FULL).
>>
>> Use BINARY in ima_show_measurements_count(), BINARY_STAGED in
>> ima_add_kexec_buffer() and BINARY_FULL in ima_measure_kexec_event().
>>
>> It should be noted that the BINARY_FULL counter is not passed through
>> kexec. Thus, the number of entries included in the kexec critical data
>> records refers to the entries since the previous kexec records.
>>
>> Note: This code derives from the Alt-IMA Huawei project, whose license is
>> GPL-2.0 OR MIT.
>>
>> Link: https://github.com/linux-integrity/linux/issues/1
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> The design looks good. As I mentioned above, this patch description is quite
> long, which is an indication that the patch needs to be split up. One method of
> breaking it up would be:
>
> - (Basic) support for staging measurements for deletion (based on a Kconfig)
> - Support for removing the hash table
Great work on performance improvement and hash table redesign.
If update the following "Trim N" method patch with your current patch
lock time performance
improvement plus the hash table change, "Trim N" method can do the same
kernel
measurement list lock time as staged method do, right?
https://lore.kernel.org/linux-integrity/20260205235849.7086-1-chenste@linux.microsoft.com/
> - Support for deleting N measurement records (and pre-pending the remaining
> measurement records)
Is there any problem to bring work of "stage" step together to the
deletion step?
"Trim N" method does everything that "staged" method can do, right?
what's the "stage"
method can do but "trim N" method can't do?
in user space, if in "staged" state, no other user space agent can
access the IMA measure list, right?
Could you explain the benefit of bringing the "stage" step?
Thanks,
Steven
> - Adding documentation
>
> thanks,
>
> Mimi
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Paul Moore @ 2026-03-19 18:44 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAOQ4uxinSLXKWjMgwyA2A_UU5e+6ZjbdsFUY-+f9DMfQcxH0qA@mail.gmail.com>
On Thu, Mar 19, 2026 at 2:30 PM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 4:55 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > > On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > > > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > > > opened with a negative dentry.
> > > > >
> > > > > This is not something that a user should be able to do, but vfs code,
> > > > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > > > before instantiating the dentry.
> > > > >
> > > > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > ---
> > > > >
> > > > > Christian,
> > > > >
> > > > > This patch fixes the syzbot report [1] that the
> > > > > backing_file_user_path_file() patch [2] introduces.
> > > > >
> > > > > This is not the only possible fix, but it is the cleanest one IMO.
> > > > > There is a small risk in introducing a state of an O_PATH file with
> > > > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > > > obvious risk in this state.
> > > > >
> > > > > Note that specifically, the user path inode is accessed via d_inode()
> > > > > and not via file_inode(), which makes this safe for file_user_inode()
> > > > > callers.
> > > > >
> > > > > BTW, I missed this regression with the original patch because I
> > > > > only ran the quick overlayfs sanity test.
> > > > >
> > > > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > > > test case is covered and that the bug is detected.
> > > > >
> > > > > WDYT?
> > > > >
> > > > > Thanks,
> > > > > Amir.
> > > > >
> > > > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > > > >
> > > > > fs/open.c | 7 ++++---
> > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/fs/open.c b/fs/open.c
> > > > > index 91f1139591abe..2004a8c0d9c97 100644
> > > > > --- a/fs/open.c
> > > > > +++ b/fs/open.c
> > > > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > > > >
> > > > > path_get(&f->f_path);
> > > > > f->f_inode = inode;
> > > > > - f->f_mapping = inode->i_mapping;
> > > > > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > > - f->f_sb_err = file_sample_sb_err(f);
> > > > >
> > > > > if (unlikely(f->f_flags & O_PATH)) {
> > > > > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > + f->f_mapping = inode->i_mapping;
> > > > > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > > + f->f_sb_err = file_sample_sb_err(f);
> > > > > +
> > > > > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > > > > i_readcount_inc(inode);
> > > > > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> > > >
> > > > I think this is really ugly and I'm really unhappy that we should adjust
> > > > initialization of generic vfs code for this. My preference is to push
> > > > the pain into the backing file stuff. And my ultimate preference is for
> > > > this backing file stuff to be removed again for a simple struct path.
> > > > We're working around some more fundamental cleanup here imho.
> > >
> > > Fair enough, we can rip the entire thing from vfs if you don't like it.
> > > The user path file can be opened and stored internally by selinux
> > > without adding all the associated risks in vfs.
> > >
> > > Paul,
> > >
> > > Please see compile tested code at:
> > > https://github.com/amir73il/linux/commits/user_path_file/
> >
> > No. Definitely no. Ignoring the fact that there is no reason we
> > should pushing this into the LSM, doing it in this way means it is
> > very likely that each LSM wanting to provide mmap/mprotect controls on
> > overlayfs will have to create a new O_PATH file. No.
> >
> > ... and let me preemptively comment that this doesn't belong in the
> > LSM framework either.
> >
> > As Christian already mentioned, this really needs to be addressed in
> > the backing file code, please do it there.
>
> OK, will give it another try.
Thanks, please let me know if there is anything I can do to help, e.g.
testing, etc.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 18:30 UTC (permalink / raw)
To: Paul Moore
Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAHC9VhQDXHh_=X-OKD9oN8fapVXBwLakNp4rEHLUL0cQ1RSteA@mail.gmail.com>
On Thu, Mar 19, 2026 at 4:55 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > > opened with a negative dentry.
> > > >
> > > > This is not something that a user should be able to do, but vfs code,
> > > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > > before instantiating the dentry.
> > > >
> > > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > >
> > > > Christian,
> > > >
> > > > This patch fixes the syzbot report [1] that the
> > > > backing_file_user_path_file() patch [2] introduces.
> > > >
> > > > This is not the only possible fix, but it is the cleanest one IMO.
> > > > There is a small risk in introducing a state of an O_PATH file with
> > > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > > obvious risk in this state.
> > > >
> > > > Note that specifically, the user path inode is accessed via d_inode()
> > > > and not via file_inode(), which makes this safe for file_user_inode()
> > > > callers.
> > > >
> > > > BTW, I missed this regression with the original patch because I
> > > > only ran the quick overlayfs sanity test.
> > > >
> > > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > > test case is covered and that the bug is detected.
> > > >
> > > > WDYT?
> > > >
> > > > Thanks,
> > > > Amir.
> > > >
> > > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > > >
> > > > fs/open.c | 7 ++++---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/fs/open.c b/fs/open.c
> > > > index 91f1139591abe..2004a8c0d9c97 100644
> > > > --- a/fs/open.c
> > > > +++ b/fs/open.c
> > > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > > >
> > > > path_get(&f->f_path);
> > > > f->f_inode = inode;
> > > > - f->f_mapping = inode->i_mapping;
> > > > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > - f->f_sb_err = file_sample_sb_err(f);
> > > >
> > > > if (unlikely(f->f_flags & O_PATH)) {
> > > > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > > > return 0;
> > > > }
> > > >
> > > > + f->f_mapping = inode->i_mapping;
> > > > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > > + f->f_sb_err = file_sample_sb_err(f);
> > > > +
> > > > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > > > i_readcount_inc(inode);
> > > > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> > >
> > > I think this is really ugly and I'm really unhappy that we should adjust
> > > initialization of generic vfs code for this. My preference is to push
> > > the pain into the backing file stuff. And my ultimate preference is for
> > > this backing file stuff to be removed again for a simple struct path.
> > > We're working around some more fundamental cleanup here imho.
> >
> > Fair enough, we can rip the entire thing from vfs if you don't like it.
> > The user path file can be opened and stored internally by selinux
> > without adding all the associated risks in vfs.
> >
> > Paul,
> >
> > Please see compile tested code at:
> > https://github.com/amir73il/linux/commits/user_path_file/
>
> No. Definitely no. Ignoring the fact that there is no reason we
> should pushing this into the LSM, doing it in this way means it is
> very likely that each LSM wanting to provide mmap/mprotect controls on
> overlayfs will have to create a new O_PATH file. No.
>
> ... and let me preemptively comment that this doesn't belong in the
> LSM framework either.
>
> As Christian already mentioned, this really needs to be addressed in
> the backing file code, please do it there.
>
OK, will give it another try.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Stephen Smalley @ 2026-03-19 18:22 UTC (permalink / raw)
To: Casey Schaufler
Cc: Feng Yang, jmorris, linux-kernel, linux-security-module, paul,
serge
In-Reply-To: <80ff003b-51fd-4ec8-9505-dfc1c5890ced@schaufler-ca.com>
On Thu, Mar 19, 2026 at 1:52 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 3/18/2026 7:22 PM, Feng Yang wrote:
> > On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
> >> On 3/17/2026 11:19 PM, Feng Yang wrote:
> >>> From: Feng Yang <yangfeng@kylinos.cn>
> >>>
> >>> After hooking the following BPF program:
> >>> SEC("lsm/xfrm_decode_session")
> >>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> >>> {
> >>> return 1; // Any non-zero value
> >>> }
> >>> Subsequent packet transmission triggers will cause a kernel panic:
> >> LSM hooks that use or provide secids cannot be stacked. That is,
> >> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
> >> correct behavior. Your proposed "fix" removes a legitimate check.
> > I'm very sorry, I didn't quite understand what you meant.
> >
> > Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
>
> Do i understand correctly that you do not have SELinux enabled?
>
> > Therefore, is it the expected behavior that simply using
> > SEC("lsm/xfrm_decode_session")
> > int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> > return -1;
> > }
> > would cause a kernel panic?
>
> Yes. As the infrastructure is written, any return other than 0 is erroneous.
> You will need to query the SELinux developers about why they decided to
> implement the xfrm handling in this way.
Looks like this BUG_ON() has just been preserved since first being
introduced by:
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=beb8d13bed80f8388f1a9a107d07ddd342e627e8
I think we can remove it at this point - it evidently doesn't ever
trigger for SELinux or we would have seen it by now and it is
definitely unsafe for BPF LSM.
^ permalink raw reply
* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Casey Schaufler @ 2026-03-19 17:51 UTC (permalink / raw)
To: Feng Yang
Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
Casey Schaufler
In-Reply-To: <20260319022208.69924-1-yangfeng59949@163.com>
On 3/18/2026 7:22 PM, Feng Yang wrote:
> On Wed, 18 Mar 2026 10:09:47 -0700, Casey Schaufler wrote:
>> On 3/17/2026 11:19 PM, Feng Yang wrote:
>>> From: Feng Yang <yangfeng@kylinos.cn>
>>>
>>> After hooking the following BPF program:
>>> SEC("lsm/xfrm_decode_session")
>>> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
>>> {
>>> return 1; // Any non-zero value
>>> }
>>> Subsequent packet transmission triggers will cause a kernel panic:
>> LSM hooks that use or provide secids cannot be stacked. That is,
>> you can't provide a BPF LSM hook and an SELinux LSM hook and expect
>> correct behavior. Your proposed "fix" removes a legitimate check.
> I'm very sorry, I didn't quite understand what you meant.
>
> Maybe my commit message wasn't clear. I only used a BPF LSM hook without SELinux stacking enabled.
Do i understand correctly that you do not have SELinux enabled?
> Therefore, is it the expected behavior that simply using
> SEC("lsm/xfrm_decode_session")
> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall) {
> return -1;
> }
> would cause a kernel panic?
Yes. As the infrastructure is written, any return other than 0 is erroneous.
You will need to query the SELinux developers about why they decided to
implement the xfrm handling in this way.
> If not, and if the BUG_ON check is still necessary,
> then does it mean we need to modify the return value validation logic in the BPF
> verifier to ensure that only BPF programs returning 0 are accepted for this hook?
>
> Thanks.
>
>
^ permalink raw reply
* Re: [PATCH v1] selftests/landlock: Test tsync interruption and cancellation paths
From: Mickaël Salaün @ 2026-03-19 17:47 UTC (permalink / raw)
To: Günther Noack
Cc: Günther Noack, linux-security-module, Justin Suess,
Tingmao Wang, Yihan Ding
In-Reply-To: <20260314.61cb1f2dc01d@gnoack.org>
On Sat, Mar 14, 2026 at 10:10:23PM +0100, Günther Noack wrote:
> Hello Mickaël!
>
> On Tue, Mar 10, 2026 at 08:04:15PM +0100, Mickaël Salaün wrote:
> > Add tsync_interrupt test to exercise the signal interruption path in
> > landlock_restrict_sibling_threads(). When a signal interrupts
> > wait_for_completion_interruptible() while the calling thread waits for
> > sibling threads to finish credential preparation, the kernel:
> >
> > 1. Sets ERESTARTNOINTR to request a transparent syscall restart.
> > 2. Calls cancel_tsync_works() to opportunistically dequeue task works
> > that have not started running yet.
> > 3. Breaks out of the preparation loop, then unblocks remaining
> > task works via complete_all() and waits for them to finish.
> > 4. Returns the error, causing abort_creds() in the syscall handler.
> >
> > Specifically, cancel_tsync_works() in its entirety, the ERESTARTNOINTR
> > error branch in landlock_restrict_sibling_threads(), and the
> > abort_creds() error branch in the landlock_restrict_self() syscall
> > handler are timing-dependent and not exercised by the existing tsync
> > tests, making code coverage measurements non-deterministic.
> >
> > The test spawns a signaler thread that rapidly sends SIGUSR1 to the
> > calling thread while it performs landlock_restrict_self() with
> > LANDLOCK_RESTRICT_SELF_TSYNC. Since ERESTARTNOINTR causes a
> > transparent restart, userspace always sees the syscall succeed.
> >
> > This is a best-effort coverage test: the interruption path is exercised
> > when the signal lands during the preparation wait, which depends on
> > thread scheduling. The test creates enough idle sibling threads (200)
> > to ensure multiple serialized waves of credential preparation even on
> > machines with many cores (e.g., 64), widening the window for the
> > signaler. Deterministic coverage would require wrapping the wait call
> > with ALLOW_ERROR_INJECTION() and using CONFIG_FAIL_FUNCTION.
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Justin Suess <utilityemal77@gmail.com>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Cc: Yihan Ding <dingyihan@uniontech.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > tools/testing/selftests/landlock/tsync_test.c | 91 ++++++++++++++++++-
> > 1 file changed, 90 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/landlock/tsync_test.c b/tools/testing/selftests/landlock/tsync_test.c
> > index 37ef0d2270db..2b9ad4f154f4 100644
> > --- a/tools/testing/selftests/landlock/tsync_test.c
> > +++ b/tools/testing/selftests/landlock/tsync_test.c
> > @@ -6,9 +6,10 @@
> > */
> >
> > #define _GNU_SOURCE
> > +#include <linux/landlock.h>
> > #include <pthread.h>
> > +#include <signal.h>
> > #include <sys/prctl.h>
> > -#include <linux/landlock.h>
> >
> > #include "common.h"
> >
> > @@ -158,4 +159,92 @@ TEST(competing_enablement)
> > EXPECT_EQ(0, close(ruleset_fd));
> > }
> >
> > +static void signal_nop_handler(int sig)
> > +{
> > +}
> > +
> > +struct signaler_data {
> > + pthread_t target;
> > + volatile bool stop;
> > +};
> > +
> > +static void *signaler_thread(void *data)
> > +{
> > + struct signaler_data *sd = data;
> > +
> > + while (!sd->stop)
> > + pthread_kill(sd->target, SIGUSR1);
> > +
> > + return NULL;
> > +}
> > +
> > +/*
> > + * Number of idle sibling threads. This must be large enough that even on
> > + * machines with many cores, the sibling threads cannot all complete their
> > + * credential preparation in a single parallel wave, otherwise the signaler
> > + * thread has no window to interrupt wait_for_completion_interruptible().
> > + * 200 threads on a 64-core machine yields ~3 serialized waves, giving the
> > + * tight signal loop enough time to land an interruption.
> > + */
> > +#define NUM_IDLE_THREADS 200
> > +
> > +/*
> > + * Exercises the tsync interruption and cancellation paths in tsync.c.
> > + *
> > + * When a signal interrupts the calling thread while it waits for sibling
> > + * threads to finish their credential preparation
> > + * (wait_for_completion_interruptible in landlock_restrict_sibling_threads),
> > + * the kernel sets ERESTARTNOINTR, cancels queued task works that have not
> > + * started yet (cancel_tsync_works), then waits for the remaining works to
> > + * finish. On the error return, syscalls.c aborts the prepared credentials.
> > + * The kernel automatically restarts the syscall, so userspace sees success.
> > + */
> > +TEST(tsync_interrupt)
> > +{
> > + size_t i;
> > + pthread_t threads[NUM_IDLE_THREADS];
> > + pthread_t signaler;
> > + struct signaler_data sd;
> > + struct sigaction sa = {};
> > + const int ruleset_fd = create_ruleset(_metadata);
> > +
> > + disable_caps(_metadata);
> > +
> > + /* Install a no-op SIGUSR1 handler so the signal does not kill us. */
> > + sa.sa_handler = signal_nop_handler;
> > + sigemptyset(&sa.sa_mask);
> > + ASSERT_EQ(0, sigaction(SIGUSR1, &sa, NULL));
> > +
> > + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> > +
> > + for (i = 0; i < NUM_IDLE_THREADS; i++)
> > + ASSERT_EQ(0, pthread_create(&threads[i], NULL, idle, NULL));
> > +
> > + /*
> > + * Start a signaler thread that continuously sends SIGUSR1 to the
> > + * calling thread. This maximizes the chance of interrupting
> > + * wait_for_completion_interruptible() in the kernel's tsync path.
> > + */
> > + sd.target = pthread_self();
> > + sd.stop = false;
> > + ASSERT_EQ(0, pthread_create(&signaler, NULL, signaler_thread, &sd));
> > +
> > + /*
> > + * The syscall may be interrupted and transparently restarted by the
> > + * kernel (ERESTARTNOINTR). From userspace, it should always succeed.
> > + */
> > + EXPECT_EQ(0, landlock_restrict_self(ruleset_fd,
> > + LANDLOCK_RESTRICT_SELF_TSYNC));
> > +
> > + sd.stop = true;
> > + ASSERT_EQ(0, pthread_join(signaler, NULL));
> > +
> > + for (i = 0; i < NUM_IDLE_THREADS; i++) {
> > + ASSERT_EQ(0, pthread_cancel(threads[i]));
> > + ASSERT_EQ(0, pthread_join(threads[i], NULL));
> > + }
> > +
> > + EXPECT_EQ(0, close(ruleset_fd));
> > +}
> > +
> > TEST_HARNESS_MAIN
> > --
> > 2.53.0
>
> The purpose of a test is to catch errors, so I broke the
> ERESTARTNOINTR error handling code path, but I could not get the test
> to fail. Did you manage to reproduce any of these bugs with it, by
> any chance, and in what configuration did that work? I tried with
> both QEMU (a bit more) and UML (a bit less), but had no luck.
>
> (Does this need to run in a loop like the Syzkaller-generated deadlock
> reproducer, so that we have a chance of catching these bugs at all?)
There is no bug and the test should always pass, it's just to improve
test coverage in the new kernel code introduced by the tsync feature.
Without this patch, most of the time we get: 90.2% of 2105 lines.
With this patch, we always get: 91.1% of 2105 lines.
^ permalink raw reply
* Re: linux-next: Tree for Mar 18 (landlock docs)
From: Mickaël Salaün @ 2026-03-19 17:37 UTC (permalink / raw)
To: Randy Dunlap
Cc: Mark Brown, Linux Next Mailing List, Linux Kernel Mailing List,
Dan Cojocaru, git, linux-security-module, Günther Noack
In-Reply-To: <f2a83432-f0ee-40a2-a3d0-71eef9db48fc@infradead.org>
On Wed, Mar 18, 2026 at 03:12:39PM -0700, Randy Dunlap wrote:
>
>
> On 3/18/26 9:52 AM, Mark Brown wrote:
> > Hi all,
> >
> > Changes since 20260317:
> >
>
> linux-next/Documentation/userspace-api/landlock.rst:198: ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 105 supplied.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> .. code-block:: c
> __u32 restrict_flags =
>
>
> Please insert a blank line between ".. code-block:: c"
> and the code.
>
Thanks, it's now fixed and rebased.
>
> d37d3347aa59 ("landlock: Expand restrict flags example for ABI version 8")
>
> --
> ~Randy
>
>
^ permalink raw reply
* Re: [PATCH v6] backing_file: store user_path_file
From: Paul Moore @ 2026-03-19 15:56 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs
In-Reply-To: <CAOQ4uxj8SZqaFbrVifw0uw7skbUvJP1_nLWj4PYCeZuaLy5t_A@mail.gmail.com>
On Thu, Mar 19, 2026 at 10:54 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 12:47 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > Instead of storing the user_path, store an O_PATH file for the
> > > user_path with the original user file creds and a security context.
> > >
> > > The user_path_file is only exported as a const pointer and its refcnt
> > > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> > >
> > > The file_ref_init() helper was changed to accept the FILE_REF_ constant
> > > instead of the fake +1 integer count.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Christian,
> > >
> > > My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> > > Here are the changes you requested.
> > >
> > > I removed the ACKs and Tested-by because of the changes.
> > >
> > > Thanks,
> > > Amir.
> > >
> > > Changes since v5:
> > > - Restore file_ref_init() helper without refcnt -1 offset
> > > - Future proofing errors from backing_file_open_user_path()
> > >
> > > [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
> > >
> > > fs/backing-file.c | 26 ++++++++++--------
> > > fs/erofs/ishare.c | 13 +++++++--
> > > fs/file_table.c | 53 ++++++++++++++++++++++++++++--------
> > > fs/fuse/passthrough.c | 3 +-
> > > fs/internal.h | 5 ++--
> > > fs/overlayfs/dir.c | 3 +-
> > > fs/overlayfs/file.c | 1 +
> > > include/linux/backing-file.h | 29 ++++++++++++++++++--
> > > include/linux/file_ref.h | 4 +--
> > > 9 files changed, 103 insertions(+), 34 deletions(-)
> >
> > Still works for me. I'm going to update lsm/stable-7.0 with this
> > patch so we can get some more linux-next testing.
> >
> > Tested-by: Paul Moore <paul@paul-moore.com>
> >
>
> Paul,
>
> As you saw, syzbot found a nasty bug in this patch and it is too hard
> to fix it without introducing more hazards.
>
> Therefore, per Christian's request I am withdrawing this patch.
>
> Please see compile tested alternative solution for selinux without
> intrusive vfs change at:
> https://github.com/amir73il/linux/commits/user_path_file/
Let's let this thread die in favor of the other. I'll already
commented there, but the quick summary is that pushing the ugliness
into an individual LSM, or the LSM framework itself, is not a
solution.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Paul Moore @ 2026-03-19 15:54 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, Al Viro, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <CAOQ4uxiS1uM=mn4q6CQfganba1XcqyXYmXfQceWdfUVRFK_Pvg@mail.gmail.com>
On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
> > On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > > Skip setting them for O_PATH file, so that the O_PATH file could be
> > > opened with a negative dentry.
> > >
> > > This is not something that a user should be able to do, but vfs code,
> > > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > > before instantiating the dentry.
> > >
> > > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Christian,
> > >
> > > This patch fixes the syzbot report [1] that the
> > > backing_file_user_path_file() patch [2] introduces.
> > >
> > > This is not the only possible fix, but it is the cleanest one IMO.
> > > There is a small risk in introducing a state of an O_PATH file with
> > > NULL f_inode, but I (and the bots that I asked) did not find any
> > > obvious risk in this state.
> > >
> > > Note that specifically, the user path inode is accessed via d_inode()
> > > and not via file_inode(), which makes this safe for file_user_inode()
> > > callers.
> > >
> > > BTW, I missed this regression with the original patch because I
> > > only ran the quick overlayfs sanity test.
> > >
> > > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > > test case is covered and that the bug is detected.
> > >
> > > WDYT?
> > >
> > > Thanks,
> > > Amir.
> > >
> > > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> > >
> > > fs/open.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/open.c b/fs/open.c
> > > index 91f1139591abe..2004a8c0d9c97 100644
> > > --- a/fs/open.c
> > > +++ b/fs/open.c
> > > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> > >
> > > path_get(&f->f_path);
> > > f->f_inode = inode;
> > > - f->f_mapping = inode->i_mapping;
> > > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > - f->f_sb_err = file_sample_sb_err(f);
> > >
> > > if (unlikely(f->f_flags & O_PATH)) {
> > > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > > return 0;
> > > }
> > >
> > > + f->f_mapping = inode->i_mapping;
> > > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > > + f->f_sb_err = file_sample_sb_err(f);
> > > +
> > > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > > i_readcount_inc(inode);
> > > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
> >
> > I think this is really ugly and I'm really unhappy that we should adjust
> > initialization of generic vfs code for this. My preference is to push
> > the pain into the backing file stuff. And my ultimate preference is for
> > this backing file stuff to be removed again for a simple struct path.
> > We're working around some more fundamental cleanup here imho.
>
> Fair enough, we can rip the entire thing from vfs if you don't like it.
> The user path file can be opened and stored internally by selinux
> without adding all the associated risks in vfs.
>
> Paul,
>
> Please see compile tested code at:
> https://github.com/amir73il/linux/commits/user_path_file/
No. Definitely no. Ignoring the fact that there is no reason we
should pushing this into the LSM, doing it in this way means it is
very likely that each LSM wanting to provide mmap/mprotect controls on
overlayfs will have to create a new O_PATH file. No.
... and let me preemptively comment that this doesn't belong in the
LSM framework either.
As Christian already mentioned, this really needs to be addressed in
the backing file code, please do it there.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v6] backing_file: store user_path_file
From: Amir Goldstein @ 2026-03-19 14:53 UTC (permalink / raw)
To: Paul Moore
Cc: Christian Brauner, Miklos Szeredi, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs
In-Reply-To: <CAHC9VhTKvOzMS2ZyawE=qBN-EpyucfxvwcHZYjXF5zugRhE2rw@mail.gmail.com>
On Thu, Mar 19, 2026 at 12:47 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Mar 18, 2026 at 9:13 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Instead of storing the user_path, store an O_PATH file for the
> > user_path with the original user file creds and a security context.
> >
> > The user_path_file is only exported as a const pointer and its refcnt
> > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> >
> > The file_ref_init() helper was changed to accept the FILE_REF_ constant
> > instead of the fake +1 integer count.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > My v5 patch was sent by Paul along with his LSM/selinux pataches [1].
> > Here are the changes you requested.
> >
> > I removed the ACKs and Tested-by because of the changes.
> >
> > Thanks,
> > Amir.
> >
> > Changes since v5:
> > - Restore file_ref_init() helper without refcnt -1 offset
> > - Future proofing errors from backing_file_open_user_path()
> >
> > [1] https://lore.kernel.org/r/20260316213606.374109-6-paul@paul-moore.com/
> >
> > fs/backing-file.c | 26 ++++++++++--------
> > fs/erofs/ishare.c | 13 +++++++--
> > fs/file_table.c | 53 ++++++++++++++++++++++++++++--------
> > fs/fuse/passthrough.c | 3 +-
> > fs/internal.h | 5 ++--
> > fs/overlayfs/dir.c | 3 +-
> > fs/overlayfs/file.c | 1 +
> > include/linux/backing-file.h | 29 ++++++++++++++++++--
> > include/linux/file_ref.h | 4 +--
> > 9 files changed, 103 insertions(+), 34 deletions(-)
>
> Still works for me. I'm going to update lsm/stable-7.0 with this
> patch so we can get some more linux-next testing.
>
> Tested-by: Paul Moore <paul@paul-moore.com>
>
Paul,
As you saw, syzbot found a nasty bug in this patch and it is too hard
to fix it without introducing more hazards.
Therefore, per Christian's request I am withdrawing this patch.
Please see compile tested alternative solution for selinux without
intrusive vfs change at:
https://github.com/amir73il/linux/commits/user_path_file/
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 14:50 UTC (permalink / raw)
To: Christian Brauner, Paul Moore
Cc: Al Viro, Miklos Szeredi, Gao Xiang, linux-security-module,
selinux, linux-erofs, linux-fsdevel, linux-unionfs,
syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319-unentbehrlich-reitturnier-f78d9fb01230@brauner>
On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> > The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> > Skip setting them for O_PATH file, so that the O_PATH file could be
> > opened with a negative dentry.
> >
> > This is not something that a user should be able to do, but vfs code,
> > such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> > before instantiating the dentry.
> >
> > Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Christian,
> >
> > This patch fixes the syzbot report [1] that the
> > backing_file_user_path_file() patch [2] introduces.
> >
> > This is not the only possible fix, but it is the cleanest one IMO.
> > There is a small risk in introducing a state of an O_PATH file with
> > NULL f_inode, but I (and the bots that I asked) did not find any
> > obvious risk in this state.
> >
> > Note that specifically, the user path inode is accessed via d_inode()
> > and not via file_inode(), which makes this safe for file_user_inode()
> > callers.
> >
> > BTW, I missed this regression with the original patch because I
> > only ran the quick overlayfs sanity test.
> >
> > Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> > test case is covered and that the bug is detected.
> >
> > WDYT?
> >
> > Thanks,
> > Amir.
> >
> > [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> > [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
> >
> > fs/open.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/open.c b/fs/open.c
> > index 91f1139591abe..2004a8c0d9c97 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
> >
> > path_get(&f->f_path);
> > f->f_inode = inode;
> > - f->f_mapping = inode->i_mapping;
> > - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > - f->f_sb_err = file_sample_sb_err(f);
> >
> > if (unlikely(f->f_flags & O_PATH)) {
> > f->f_mode = FMODE_PATH | FMODE_OPENED;
> > @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> > return 0;
> > }
> >
> > + f->f_mapping = inode->i_mapping;
> > + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> > + f->f_sb_err = file_sample_sb_err(f);
> > +
> > if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> > i_readcount_inc(inode);
> > } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
>
> I think this is really ugly and I'm really unhappy that we should adjust
> initialization of generic vfs code for this. My preference is to push
> the pain into the backing file stuff. And my ultimate preference is for
> this backing file stuff to be removed again for a simple struct path.
> We're working around some more fundamental cleanup here imho.
Fair enough, we can rip the entire thing from vfs if you don't like it.
The user path file can be opened and stored internally by selinux
without adding all the associated risks in vfs.
Paul,
Please see compile tested code at:
https://github.com/amir73il/linux/commits/user_path_file/
Feel free to use my patch as is or reorder/squash/rebase as you see fit.
Feel free to attribute my contribution any way you see fit, just pls cc me for
review when you post v2.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Mimi Zohar @ 2026-03-19 14:28 UTC (permalink / raw)
To: Chris Fenner
Cc: Jarkko Sakkinen, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <CAMigqh2kW_srDgnOs+1t=sk9Q9jJgaQVswO0ZRHhV-6zrOAZ6A@mail.gmail.com>
On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> Apologies if my long message derailed this discussion. I meant to
> support Mimi's concern here and project a future vision where
> TCG_TPM2_HMAC doesn't conflict with other features.
>
> More concisely, I think that:
>
> > tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
>
> is not a compelling argument for removing TPM as an RNG source,
> because TCG_TPM2_HMAC is known to have poor performance already
> anyway.
Agreed. Thanks, Chris! FYI, we raised concerns about IMA performance with the
TPM HMAC and encrypted feature while it was being developed. James had some
ideas, at the time, as to how to resolve the performance issue for IMA. Yet it
was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
default on x86 systems.
Jarkko has queued this patch in the "queue" branch, without indicating whether
it will eventually be upstreamed or not.
Mimi
^ permalink raw reply
* Re: [PATCH 12/61] quota: Prefer IS_ERR_OR_NULL over manual NULL check
From: Jan Kara @ 2026-03-19 14:13 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Jan Kara
In-Reply-To: <20260310-b4-is_err_or_null-v1-12-bd63b656022d@avm.de>
On Tue 10-03-26 12:48:38, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
>
> To: Jan Kara <jack@suse.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Philipp Hahn <phahn-oss@avm.de>
Thanks for the patch but frankly I find the original variant clearer wrt
what is going on. So I prefer to keep the code as is.
Honza
> ---
> fs/quota/quota.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 33bacd70758007129e0375bab44d7431195ec441..2e09fc247d0cf45b9e83a4f8a0be7ea694c8c2a1 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -965,7 +965,7 @@ SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
> else
> drop_super_exclusive(sb);
> out:
> - if (pathp && !IS_ERR(pathp))
> + if (!IS_ERR_OR_NULL(pathp))
> path_put(pathp);
> return ret;
> }
>
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Christian Brauner @ 2026-03-19 13:13 UTC (permalink / raw)
To: Amir Goldstein
Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
In-Reply-To: <20260319124616.1544415-1-amir73il@gmail.com>
On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
> Skip setting them for O_PATH file, so that the O_PATH file could be
> opened with a negative dentry.
>
> This is not something that a user should be able to do, but vfs code,
> such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
> before instantiating the dentry.
>
> Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>
> Christian,
>
> This patch fixes the syzbot report [1] that the
> backing_file_user_path_file() patch [2] introduces.
>
> This is not the only possible fix, but it is the cleanest one IMO.
> There is a small risk in introducing a state of an O_PATH file with
> NULL f_inode, but I (and the bots that I asked) did not find any
> obvious risk in this state.
>
> Note that specifically, the user path inode is accessed via d_inode()
> and not via file_inode(), which makes this safe for file_user_inode()
> callers.
>
> BTW, I missed this regression with the original patch because I
> only ran the quick overlayfs sanity test.
>
> Now I ran a full quick fstest cycle and verified that the O_TMPFILE
> test case is covered and that the bug is detected.
>
> WDYT?
>
> Thanks,
> Amir.
>
> [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
>
> fs/open.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 91f1139591abe..2004a8c0d9c97 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
>
> path_get(&f->f_path);
> f->f_inode = inode;
> - f->f_mapping = inode->i_mapping;
> - f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> - f->f_sb_err = file_sample_sb_err(f);
>
> if (unlikely(f->f_flags & O_PATH)) {
> f->f_mode = FMODE_PATH | FMODE_OPENED;
> @@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
> return 0;
> }
>
> + f->f_mapping = inode->i_mapping;
> + f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
> + f->f_sb_err = file_sample_sb_err(f);
> +
> if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
> i_readcount_inc(inode);
> } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
I think this is really ugly and I'm really unhappy that we should adjust
initialization of generic vfs code for this. My preference is to push
the pain into the backing file stuff. And my ultimate preference is for
this backing file stuff to be removed again for a simple struct path.
We're working around some more fundamental cleanup here imho.
^ permalink raw reply
* [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry
From: Amir Goldstein @ 2026-03-19 12:46 UTC (permalink / raw)
To: Christian Brauner
Cc: Al Viro, Miklos Szeredi, Paul Moore, Gao Xiang,
linux-security-module, selinux, linux-erofs, linux-fsdevel,
linux-unionfs, syzbot+f34aab278bf5d664e2be
The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
Skip setting them for O_PATH file, so that the O_PATH file could be
opened with a negative dentry.
This is not something that a user should be able to do, but vfs code,
such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
before instantiating the dentry.
Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
Christian,
This patch fixes the syzbot report [1] that the
backing_file_user_path_file() patch [2] introduces.
This is not the only possible fix, but it is the cleanest one IMO.
There is a small risk in introducing a state of an O_PATH file with
NULL f_inode, but I (and the bots that I asked) did not find any
obvious risk in this state.
Note that specifically, the user path inode is accessed via d_inode()
and not via file_inode(), which makes this safe for file_user_inode()
callers.
BTW, I missed this regression with the original patch because I
only ran the quick overlayfs sanity test.
Now I ran a full quick fstest cycle and verified that the O_TMPFILE
test case is covered and that the bug is detected.
WDYT?
Thanks,
Amir.
[1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
[2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
fs/open.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 91f1139591abe..2004a8c0d9c97 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,
path_get(&f->f_path);
f->f_inode = inode;
- f->f_mapping = inode->i_mapping;
- f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
- f->f_sb_err = file_sample_sb_err(f);
if (unlikely(f->f_flags & O_PATH)) {
f->f_mode = FMODE_PATH | FMODE_OPENED;
@@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
return 0;
}
+ f->f_mapping = inode->i_mapping;
+ f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
+ f->f_sb_err = file_sample_sb_err(f);
+
if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
i_readcount_inc(inode);
} else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
--
2.53.0
^ permalink raw reply related
* Re: [apparmor][PATCH] apparmor: fix incorrect success return value in unpack_tag_headers()
From: Massimiliano Pellizzer @ 2026-03-19 8:23 UTC (permalink / raw)
To: John Johansen; +Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <779f2d5b-a5af-4137-a1ee-78dc9fed58e1@canonical.com>
On Wed, Mar 18, 2026 at 6:53 AM John Johansen
<john.johansen@canonical.com> wrote:
>
> On 2/10/26 09:21, Massimiliano Pellizzer wrote:
> > unpack_tag_headers() returns `true` (1) on success instead of 0.
> > Since it's caller unpack_tags() checks the return value with
> > `if (error)`, a non-zero success value is incorrectly treated as
> > a failure, causing tag header unpacking to always even if the data
> > is well-formed.
> >
> > Change the success return in unpack_tag_headers() from `true` to 0.
> >
> > Fixes: 3d28e2397af7 ("apparmor: add support loading per permission tagging")
> > Signed-off-by: Massimiliano Pellizzer <mpellizzer.dev@gmail.com>
>
> sorry, my reply to this seems to have failed. This was pulled in for the
> 7.0 PR
>
> Acked-by: John Johansen <john.johansen@canonical.com>
>
>
> > ---
> > security/apparmor/policy_unpack.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> > index dc908e1f5a88..221208788025 100644
> > --- a/security/apparmor/policy_unpack.c
> > +++ b/security/apparmor/policy_unpack.c
> > @@ -825,7 +825,7 @@ static int unpack_tag_headers(struct aa_ext *e, struct aa_tags_struct *tags)
> > tags->hdrs.size = size;
> > tags->hdrs.table = hdrs;
> > AA_DEBUG(DEBUG_UNPACK, "headers %ld size %d", (long) hdrs, size);
> > - return true;
> > + return 0;
> >
> > fail:
> > kfree_sensitive(hdrs);
>
Hello JJ,
I don't see this patch being part of v7.0-rc4:
$ git --no-pager log --grep "apparmor: fix incorrect success return
value in unpack"
I don't see the change applied to the apparmor-next branch either
(https://gitlab.com/apparmor/apparmor-kernel/-/blob/apparmor-next/security/apparmor/policy_unpack.c).
---
Massimiliano Pellizzer
^ permalink raw reply
* Re: [PATCH] apparmor: Fix string overrun due to missing termination
From: Greg KH @ 2026-03-19 6:40 UTC (permalink / raw)
To: Daniel J Blueman
Cc: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Thorsten Blum, apparmor, linux-security-module, linux-kernel,
stable
In-Reply-To: <20260319062433.17648-1-daniel@quora.org>
On Thu, Mar 19, 2026 at 02:24:32PM +0800, Daniel J Blueman wrote:
> When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
> Snapdragon X1 we see a string buffer overrun:
>
> BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
> Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120
>
> CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
> Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
> Call trace:
> show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
> dump_stack_lvl (lib/dump_stack.c:122)
> print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
> kasan_report (mm/kasan/report.c:597)
> __asan_report_load1_noabort (mm/kasan/report_generic.c:378)
> aa_dfa_match (security/apparmor/match.c:535)
> match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
> match_mnt (security/apparmor/mount.c:371)
> aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
>
> Allocated by task 2120:
> kasan_save_stack (mm/kasan/common.c:58)
> kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
> kasan_save_alloc_info (mm/kasan/generic.c:571)
> __kasan_kmalloc (mm/kasan/common.c:419)
> __kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
> aa_get_buffer (security/apparmor/lsm.c:2201)
> aa_bind_mount (security/apparmor/mount.c:442)
> apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
> security_sb_mount (security/security.c:1062 (discriminator 31))
> path_mount (fs/namespace.c:4101)
> __arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
> invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
> el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
> do_el0_svc (arch/arm64/kernel/syscall.c:152)
> el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
> el0t_64_sync (arch/arm64/kernel/entry.S:596)
>
> The buggy address belongs to the object at ffff0008901ca000
> which belongs to the cache kmalloc-rnd-06-8k of size 8192
> The buggy address is located 0 bytes to the right of
> allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
> head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
> flags: 0x8000000000000040(head|zone=2)
> page_type: f5(slab)
> raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
> head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
> head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
> head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>
> This was introduced by previous incorrect conversion from strcpy(). Fix it
> by adding the missing terminator.
>
> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
> ---
> security/apparmor/path.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/security/apparmor/path.c b/security/apparmor/path.c
> index 65a0ca5cc1bd..2494e8101538 100644
> --- a/security/apparmor/path.c
> +++ b/security/apparmor/path.c
> @@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
> }
>
> out:
> - /* Append "/" to directory paths, except for root "/" which
> - * already ends in a slash.
> + /* Append "/" to directory paths and reterminate string, except for
> + * root "/" which already ends in a slash.
> */
> if (!error && isdir) {
> bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
>
> - if (!is_root)
> + if (!is_root) {
> buf[aa_g_path_max - 2] = '/';
> + buf[aa_g_path_max - 1] = '\0';
> + }
> }
>
> return error;
> --
> 2.53.0
>
>
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply
* [PATCH] apparmor: Fix string overrun due to missing termination
From: Daniel J Blueman @ 2026-03-19 6:24 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel,
stable, Daniel J Blueman
When booting Ubuntu 26.04 with Linux 7.0-rc4 on an ARM64 Qualcomm
Snapdragon X1 we see a string buffer overrun:
BUG: KASAN: slab-out-of-bounds in aa_dfa_match (security/apparmor/match.c:535)
Read of size 1 at addr ffff0008901cc000 by task snap-update-ns/2120
CPU: 5 UID: 60578 PID: 2120 Comm: snap-update-ns Not tainted 7.0.0-rc4+ #22 PREEMPTLAZY
Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN60WW 09/11/2025
Call trace:
show_stack (arch/arm64/kernel/stacktrace.c:501) (C)
dump_stack_lvl (lib/dump_stack.c:122)
print_report (mm/kasan/report.c:379 mm/kasan/report.c:482)
kasan_report (mm/kasan/report.c:597)
__asan_report_load1_noabort (mm/kasan/report_generic.c:378)
aa_dfa_match (security/apparmor/match.c:535)
match_mnt_path_str (security/apparmor/mount.c:244 security/apparmor/mount.c:336)
match_mnt (security/apparmor/mount.c:371)
aa_bind_mount (security/apparmor/mount.c:447 (discriminator 4))
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)
Allocated by task 2120:
kasan_save_stack (mm/kasan/common.c:58)
kasan_save_track (./arch/arm64/include/asm/current.h:19 mm/kasan/common.c:70 mm/kasan/common.c:79)
kasan_save_alloc_info (mm/kasan/generic.c:571)
__kasan_kmalloc (mm/kasan/common.c:419)
__kmalloc_noprof (./include/linux/kasan.h:263 mm/slub.c:5260 mm/slub.c:5272)
aa_get_buffer (security/apparmor/lsm.c:2201)
aa_bind_mount (security/apparmor/mount.c:442)
apparmor_sb_mount (security/apparmor/lsm.c:719 (discriminator 1))
security_sb_mount (security/security.c:1062 (discriminator 31))
path_mount (fs/namespace.c:4101)
__arm64_sys_mount (fs/namespace.c:4172 fs/namespace.c:4361 fs/namespace.c:4338 fs/namespace.c:4338)
invoke_syscall.constprop.0 (arch/arm64/kernel/syscall.c:35 arch/arm64/kernel/syscall.c:49)
el0_svc_common.constprop.0 (./include/linux/thread_info.h:142 (discriminator 2) arch/arm64/kernel/syscall.c:140 (discriminator 2))
do_el0_svc (arch/arm64/kernel/syscall.c:152)
el0_svc (arch/arm64/kernel/entry-common.c:80 arch/arm64/kernel/entry-common.c:725)
el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:744)
el0t_64_sync (arch/arm64/kernel/entry.S:596)
The buggy address belongs to the object at ffff0008901ca000
which belongs to the cache kmalloc-rnd-06-8k of size 8192
The buggy address is located 0 bytes to the right of
allocated 8192-byte region [ffff0008901ca000, ffff0008901cc000)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9101c8
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:-1 pincount:0
flags: 0x8000000000000040(head|zone=2)
page_type: f5(slab)
raw: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
raw: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000040 ffff000800016c40 fffffdffe2d14e10 ffff000800015c70
head: 0000000000000000 0000000800010001 00000000f5000000 0000000000000000
head: 8000000000000003 fffffdffe2407201 fffffdffffffffff 00000000ffffffff
head: ffffffffffffffff 0000000000000000 00000000ffffffff 0000000000000008
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff0008901cbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff0008901cbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff0008901cc000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff0008901cc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff0008901cc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
This was introduced by previous incorrect conversion from strcpy(). Fix it
by adding the missing terminator.
Signed-off-by: Daniel J Blueman <daniel@quora.org>
Fixes: 93d4dbdc8da0 ("apparmor: Replace deprecated strcpy in d_namespace_path")
---
security/apparmor/path.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index 65a0ca5cc1bd..2494e8101538 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -164,14 +164,16 @@ static int d_namespace_path(const struct path *path, char *buf, char **name,
}
out:
- /* Append "/" to directory paths, except for root "/" which
- * already ends in a slash.
+ /* Append "/" to directory paths and reterminate string, except for
+ * root "/" which already ends in a slash.
*/
if (!error && isdir) {
bool is_root = (*name)[0] == '/' && (*name)[1] == '\0';
- if (!is_root)
+ if (!is_root) {
buf[aa_g_path_max - 2] = '/';
+ buf[aa_g_path_max - 1] = '\0';
+ }
}
return error;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v1] security/safesetid: fix comment and error handling
From: Micah Morton @ 2026-03-19 3:27 UTC (permalink / raw)
To: yanwei.gao; +Cc: paul, linux-security-module
In-Reply-To: <CAJ-EccNLTuj74vKsNQFnOSXgjdyRpUrFVARsuqn6VNatQu33WA@mail.gmail.com>
On Wed, Mar 18, 2026 at 1:49 PM Micah Morton <mortonm@chromium.org> wrote:
>
> On Mon, Mar 2, 2026 at 6:40 PM yanwei.gao <gaoyanwei.tx@gmail.com> wrote:
> >
> > - Fix comment in lsm.c: use CAP_SETGID instead of CAP_SETUID in the
> > GID capability check comment to match the actual logic.
> > - In handle_policy_update(), set err = -EINVAL and goto out_free_buf
> > when policy type is neither UID nor GID, so the error is returned
> > to the caller instead of only logging.
> > - In safesetid_init_securityfs(), return ret directly when
> > policy_dir creation fails instead of goto error (no cleanup needed
> > at that point).
> >
> > Signed-off-by: yanwei.gao <gaoyanwei.tx@gmail.com>
> > ---
> > security/safesetid/lsm.c | 2 +-
> > security/safesetid/securityfs.c | 4 +++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> > index d5fb949050dd..a7b68e65996c 100644
> > --- a/security/safesetid/lsm.c
> > +++ b/security/safesetid/lsm.c
> > @@ -128,7 +128,7 @@ static int safesetid_security_capable(const struct cred *cred,
> > if (setid_policy_lookup((kid_t){.gid = cred->gid}, INVALID_ID, GID) == SIDPOL_DEFAULT)
> > return 0;
> > /*
> > - * Reject use of CAP_SETUID for functionality other than calling
> > + * Reject use of CAP_SETGID for functionality other than calling
> Looks good.
> > * set*gid() (e.g. setting up userns gid mappings).
> > */
> > pr_warn("Operation requires CAP_SETGID, which is not available to GID %u for operations besides approved set*gid transitions\n",
> > diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> > index a71e548065a9..50682abd342b 100644
> > --- a/security/safesetid/securityfs.c
> > +++ b/security/safesetid/securityfs.c
> > @@ -224,6 +224,8 @@ static ssize_t handle_policy_update(struct file *file,
> > } else {
> > /* Error, policy type is neither UID or GID */
> > pr_warn("error: bad policy type");
> > + err = -EINVAL;
> > + goto out_free_buf;
>
> This makes sense to me and matches how things are done in the
> functions above in verify_ruleset() and parse_policy_line().
>
> Is this code actually reachable? I guess if verify_ruleset returns
> -EINVAL due to a bad policy type value the code still hits this spot?
>
> > }
> > err = len;
> >
> > @@ -321,7 +323,7 @@ int __init safesetid_init_securityfs(void)
> > policy_dir = securityfs_create_dir("safesetid", NULL);
> > if (IS_ERR(policy_dir)) {
> > ret = PTR_ERR(policy_dir);
> > - goto error;
> > + return ret;
>
> We still need the `securityfs_remove(policy_dir)` call to clean up the
> dir created with `policy_dir = securityfs_create_dir("safesetid",
> NULL)` don't we?
I guess securityfs_remove() just returns right away if the
'policy_dir' pointer IS_ERR, so either way is fine. I don't really see
a reason to change this though.. seems better to err on the side of
calling the securityfs_remove() function even if its currently a
no-op.
>
> > }
> >
> > uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
> > --
> > 2.43.5
> >
^ 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