* Re: [RFC PATCH v2 33/34] lsm: consolidate all of the LSM framework initcalls
From: Casey Schaufler @ 2025-07-24 23:37 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
Casey Schaufler
In-Reply-To: <20250721232142.77224-69-paul@paul-moore.com>
On 7/21/2025 4:21 PM, Paul Moore wrote:
> The LSM framework itself registers a small number of initcalls, this
> patch converts these initcalls into the new initcall mechanism.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/inode.c | 3 +--
> security/lsm.h | 4 ++++
> security/lsm_init.c | 14 ++++++++++++--
> security/min_addr.c | 5 +++--
> 4 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index 68ee6c9de833..d15a0b0f4b14 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -408,7 +408,7 @@ static const struct file_operations lsm_ops = {
> };
> #endif
>
> -static int __init securityfs_init(void)
> +int __init securityfs_init(void)
> {
> int retval;
>
> @@ -427,4 +427,3 @@ static int __init securityfs_init(void)
> #endif
> return 0;
> }
> -core_initcall(securityfs_init);
> diff --git a/security/lsm.h b/security/lsm.h
> index 8dc267977ae0..436219260376 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -35,4 +35,8 @@ extern struct kmem_cache *lsm_inode_cache;
> int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
> int lsm_task_alloc(struct task_struct *task);
>
> +/* LSM framework initializers */
> +int securityfs_init(void);
> +int min_addr_init(void);
> +
> #endif /* _LSM_H_ */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index ab739f9c2244..f178a9a2f9d4 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -482,7 +482,12 @@ int __init security_init(void)
> */
> static int __init security_initcall_pure(void)
> {
> - return lsm_initcall(pure);
> + int rc_adr, rc_lsm;
> +
> + rc_adr = min_addr_init();
> + rc_lsm = lsm_initcall(pure);
> +
> + return (rc_adr ? rc_adr : rc_lsm);
> }
> pure_initcall(security_initcall_pure);
>
> @@ -500,7 +505,12 @@ early_initcall(security_initcall_early);
> */
> static int __init security_initcall_core(void)
> {
> - return lsm_initcall(core);
> + int rc_sfs, rc_lsm;
> +
> + rc_sfs = securityfs_init();
> + rc_lsm = lsm_initcall(core);
> +
> + return (rc_sfs ? rc_sfs : rc_lsm);
> }
> core_initcall(security_initcall_core);
>
> diff --git a/security/min_addr.c b/security/min_addr.c
> index df1bc643d886..40714bdeefbe 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -4,6 +4,8 @@
> #include <linux/security.h>
> #include <linux/sysctl.h>
>
> +#include "lsm.h"
> +
> /* amount of vm to protect from userspace access by both DAC and the LSM*/
> unsigned long mmap_min_addr;
> /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -54,11 +56,10 @@ static const struct ctl_table min_addr_sysctl_table[] = {
> },
> };
>
> -static int __init init_mmap_min_addr(void)
> +int __init min_addr_init(void)
> {
> register_sysctl_init("vm", min_addr_sysctl_table);
> update_mmap_min_addr();
>
> return 0;
> }
> -pure_initcall(init_mmap_min_addr);
^ permalink raw reply
* Re: [RFC PATCH v2 10/34] lsm: rework lsm_active_cnt and lsm_idlist[]
From: Paul Moore @ 2025-07-25 0:26 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Tetsuo Handa,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <4ad43ad6-97b9-462f-af17-6d6db8ba3bf6@schaufler-ca.com>
On Thu, Jul 24, 2025 at 11:34 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/21/2025 4:21 PM, Paul Moore wrote:
> > Move the LSM active count and lsm_id list declarations out of a header
> > that is visible across the kernel and into a header that is limited to
> > the LSM framework. This not only helps keep the include/linux headers
> > smaller and cleaner, it helps prevent misuse of these variables.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
...
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index cbdfac31ede4..03d3e140e0b1 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -22,8 +22,8 @@ static __initdata const char *lsm_order_cmdline;
> > static __initdata const char *lsm_order_legacy;
> >
> > /* Ordered list of LSMs to initialize. */
> > -static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
> > static __initdata struct lsm_info *lsm_exclusive;
> > +static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
>
> I can't see there's a good reason for the reordering. I'm not
> objecting to it, but it's curious.
I'm pretty sure this is one of those things that came about while I
was upset with the state of this code and was going through it with a
hatchet; a lot of code was ripped out and put back, so it's likely
just an artifact of that. I'll flip it back around.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-07-25 2:28 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Tetsuo Handa,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <6e5422c4-0458-4a15-8833-462e318f283d@schaufler-ca.com>
On Thu, Jul 24, 2025 at 11:39 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/21/2025 4:21 PM, Paul Moore wrote:
> > The LSM currently has a lot of code to maintain a list of the currently
> > active LSMs in a human readable string, with the only user being the
> > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > generate the string on first use and then cache it for subsequent use.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> > include/linux/lsm_hooks.h | 1 -
> > security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
> > security/lsm_init.c | 49 --------------------------------
> > 3 files changed, 57 insertions(+), 52 deletions(-)
...
> > +/* NOTE: we never free the string below once it is set. */
> > +static DEFINE_SPINLOCK(lsm_read_lock);
> > +static char *lsm_read_str = NULL;
> > +static ssize_t lsm_read_len = 0;
> > +
> > static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> > loff_t *ppos)
> > {
> > - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> > - strlen(lsm_names));
> > + int i;
> > + char *str;
> > + ssize_t len;
> > +
> > +restart:
> > +
> > + rcu_read_lock();
> > + if (!lsm_read_str) {
> > + /* we need to generate the string and try again */
> > + rcu_read_unlock();
> > + goto generate_string;
> > + }
> > + len = simple_read_from_buffer(buf, count, ppos,
> > + rcu_dereference(lsm_read_str),
> > + lsm_read_len);
> > + rcu_read_unlock();
> > + return len;
> > +
> > +generate_string:
> > +
> > + for (i = 0; i < lsm_active_cnt; i++)
> > + /* the '+ 1' accounts for either a comma or a NUL */
> > + len += strlen(lsm_idlist[i]->name) + 1;
> > +
> > + str = kmalloc(len, GFP_KERNEL);
> > + if (!str)
> > + return -ENOMEM;
> > + str[0] = '\0';
> > +
> > + for (i = 0; i < lsm_active_cnt; i++) {
> > + if (i > 0)
> > + strcat(str, ",");
> > + strcat(str, lsm_idlist[i]->name);
> > + }
> > +
> > + spin_lock(&lsm_read_lock);
> > + if (lsm_read_str) {
> > + /* we raced and lost */
> > + spin_unlock(&lsm_read_lock);
> > + kfree(str);
> > + goto restart;
> > + }
> > + lsm_read_str = str;
> > + lsm_read_len = len;
>
> You're going to get a nul byte at the end of the string because
> you accounted for the ',' above, but there isn't one at the end
> of the string.
I'm not sure I understand your concern here, can you phrase it differently?
If you're worried about lsm_read_str potentially not being terminated
with a NUL byte, the strcat() should copy over the trailing NUL.
> > + spin_unlock(&lsm_read_lock);
> > +
> > + goto restart;
> > }
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 0/2] Secure Boot lock down
From: Paul Moore @ 2025-07-25 2:43 UTC (permalink / raw)
To: Nicolas Bouchinet
Cc: Hamza Mahfooz, Xiu Jianfeng, linux-kernel, Ard Biesheuvel,
James Morris, Serge E. Hallyn, Yue Haibing, Tanya Agarwal,
Kees Cook, linux-efi, linux-security-module
In-Reply-To: <xfabe3wvdsfkch3yhxmswhootf5vj6suyow5s3ffumcnjkojjz@e7ojgu3s7ion>
On Thu, Jul 24, 2025 at 8:59 AM Nicolas Bouchinet
<nicolas.bouchinet@oss.cyber.gouv.fr> wrote:
>
> Hi Hamza, thanks for your patch.
>
> Thanks, Paul, for the forward.
>
> Sorry for the delay, we took a bit of time to do some lore archaeology
> and discuss it with Xiu.
>
> As you might know, this has already been through debates in 2017 [1]. At
> that time, the decision was not to merge this behavior.
>
> Distros have indeed carried downstream patches reflecting this behavior
> for a long time and have been affected by vulnerabilities like
> CVE-2025-1272 [2], which is caused by the magic sprinkled in
> setup_arch().
>
> While your implementation looks cleaner to me. One of the points in
> previous debates was to have a Lockdown side Kconfig knob to enable or
> not this behavior. It would gate the registration of the Lockdown LSM to
> the security_lock_kernel_down() hook.
>
> However, what bothers me is that with this patch, if UEFI Secure Boot is
> activated and a user wants to disable Lockdown, they need to go through
> disabling Secure Boot. I'm really not fond of that. A user shouldn't
> have to be forced to disable security firmware settings because of a
> kernel feature.
>
> We thus might want to add a way to disable Lockdown through kernel
> cmdline.
One can enable/disable "normal" LSMs via the "lsm=" kernel command
line option, however, as Lockdown is an "early" LSM, it is enabled
prior to the command line option parsing in the kernel so that isn't
really an option unless we add some mechanism to later disable
Lockdown during the "normal" LSM initialization phase when the command
line options are available. This would result in a window of time
during very early boot where Lockdown would be enabled, before being
disabled, but I have no idea how problematic that might be for users.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 2/4] landlock: Fix handling of disconnected directories
From: Abhinav Saxena @ 2025-07-25 3:54 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Tingmao Wang, Jann Horn, John Johansen,
Al Viro, Ben Scarlato, Christian Brauner, Daniel Burgener,
Jeff Xu, NeilBrown, Paul Moore, Ryan Sullivan, Song Liu,
linux-fsdevel, linux-security-module
In-Reply-To: <20250724.ECiGoor4ulub@digikod.net>
[-- Attachment #1: Type: text/plain, Size: 13396 bytes --]
Hi!
Mickaël Salaün <mic@digikod.net> writes:
> On Thu, Jul 24, 2025 at 04:49:24PM +0200, Günther Noack wrote:
>> On Wed, Jul 23, 2025 at 11:01:42PM +0200, Mickaël Salaün wrote:
>> > On Tue, Jul 22, 2025 at 07:04:02PM +0100, Tingmao Wang wrote:
>> > > On the other hand, I’m still a bit uncertain about the domain check
>> > > semantics. While it would not cause a rename to be allowed if it is
>> > > otherwise not allowed by any rules on or above the mountpoint, this gets a
>> > > bit weird if we have a situation where renames are allowed on the
>> > > mountpoint or everywhere, but not read/writes, however read/writes are
>> > > allowed directly on a file, but the dir containing that file gets
>> > > disconnected so the sandboxed application can’t read or write to it.
>> > > (Maybe someone would set up such a policy where renames are allowed,
>> > > expecting Landlock to always prevent renames where additional permissions
>> > > would be exposed?)
>> > >
>> > > In the above situation, if the file is then moved to a connected
>> > > directory, it will become readable/writable again.
>> >
>> > We can generalize this issue to not only the end file but any component
>> > of the path: disconnected directories. In fact, the main issue is the
>> > potential inconsistency of access checks over time (e.g. between two
>> > renames). This could be exploited to bypass the security checks done
>> > for FS_REFER.
>> >
>> > I see two solutions:
>> >
>> > 1. *Always* walk down to the IS_ROOT directory, and then jump to the
>> > mount point. This makes it possible to have consistent access checks
>> > for renames and open/use. The first downside is that that would
>> > change the current behavior for bind mounts that could get more
>> > access rights (if the policy explicitly sets rights for the hidden
>> > directories). The second downside is that we’ll do more walk.
>> >
>> > 2. Return -EACCES (or -ENOENT) for actions involving disconnected
>> > directories, or renames of disconnected opened files. This second
>> > solution is simpler and safer but completely disables the use of
>> > disconnected directories and the rename of disconnected files for
>> > sandboxed processes.
>> >
>> > It would be much better to be able to handle opened directories as
>> > (object) capabilities, but that is not currently possible because of the
>> > way paths are handled by the VFS and LSM hooks.
>> >
>> > Tingmao, Günther, Jann, what do you think?
>>
>> I have to admit that so far, I still failed to wrap my head around the
>> full patch set and its possible corner cases. I hope I did not
>> misunderstand things all too badly below:
>>
>> As far as I understand the proposed patch, we are “checkpointing” the
>> intermediate results of the path walk at every mount point boundary,
>> and in the case where we run into a disconnected directory in one of
>> the nested mount points, we restore from the intermediate result at
>> the previous mount point directory and skip to the next mount point.
>
> Correct
>
>>
>> Visually speaking, if the layout is this (where “:” denotes a
>> mountpoint boundary between the mountpoints MP1, MP2, MP3):
>>
>> dirfd
>> |
>> : V :
>> : ham <— spam <— eggs <— x.txt
>> : (disconn.) :
>> : :
>> / <— foo <— bar <— baz :
>> : :
>> MP1 MP2 MP3
>>
>> When a process holds a reference to the “spam” directory, which is now
>> disconnected, and invokes openat(dirfd, “eggs/x.txt”, …), then we
>> would:
>>
>> * traverse x.txt
>> * traverse eggs (checkpointing the intermediate result) <-.
>> * traverse spam |
>> * traverse ham |
>> * discover that ham is disconnected: |
>> * restore the intermediate result from “eggs” ———’
>> * continue the walk at foo
>> * end up at the root
>>
>> So effectively, since the results from “spam” and “ham” are discarded,
>> we would traverse only the inodes in the outer and inner mountpoints
>> MP1 and MP3, but effectively return a result that looks like we did
>> not traverse MP2?
>
> We’d still check MP2’s inode, but otherwise yes.
>
I don’t know if it makes sense, but can access rights be cached as part
of the inode security blob? Although I am not sure if the LSM blob would
exist after unlinking.
But if it does, maybe during unlink, keep the cached rights for MP2,
and during openat():
1. Start at disconnected “spam” inode
2. Check spam->i_security->allowed_access <- Cached MP2 rights
3. Continue normal path walk with preserved access context
>>
>> Maybe (likely) I misread the code. :) It’s not clear to me what the
>> thinking behind this is. Also, if there was another directory in
>> between “spam” and “eggs” in MP2, wouldn’t we be missing the access
>> rights attached to this directory?
>
> Yes, we would ignore this access right because we don’t know that the
> path was resolved from spam.
>
>>
>>
>> Regarding the capability approach:
>>
>> I agree that a “capability” approach would be the better solution, but
>> it seems infeasible with the existing LSM hooks at the moment. I
>> would be in favor of it though.
>
> Yes, it would be a new feature with potential important changes.
>
> In the meantime, we still need a fix for disconnected directories, and
> this fix needs to be backported. That’s why the capability approach is
> not part of the two solutions. ;)
>
>>
>> To spell it out a bit more explicitly what that would mean in my mind:
>>
>> When a path is looked up relative to a dirfd, the path walk upwards
>> would terminate at the dirfd and use previously calculated access
>> rights stored in the associated struct file. These access rights
>> would be determined at the time of opening the dirfd, similar to how we
>> are already storing the “truncate” access right today for regular
>> files.
>>
>> (Remark: There might still be corner cases where we have to solve it
>> the hard way, if someone uses “..” together with a dirfd-relative
>> lookup.)
>
> Yep, real capabilities don’t have “..” in their design. On Linux (and
> Landlock), we need to properly handle “..”, which is challenging.
>
>>
>> I also looked at what it would take to change the LSM hooks to pass
>> the directory that the lookup was done relative to, but it seems that
>> this would have to be passed through a bunch of VFS callbacks as well,
>> which seems like a larger change. I would be curious whether that
>> would be deemed an acceptable change.
>>
>> —Günther
>>
>>
>> P.S. Related to relative directory lookups, there is some movement in
>> the BSDs as well to use dirfds as capabilities, by adding a flag to
>> open directories that enforces O_BENEATH on subsequent opens:
>>
>> * <https://undeadly.org/cgi?action=article;sid=20250529080623>
>> * <https://reviews.freebsd.org/D50371>
>>
>> (both found via <https://news.ycombinator.com/item?id=44575361>)
>>
>> If a dirfd had such a flag, that would get rid of the corner case
>> above.
>
> This would be nice but it would not solve the current issue because we
> cannot force all processes to use this flag (which breaks some use
> cases).
>
> FYI, Capsicum is a more complete implementation:
> <https://man.freebsd.org/cgi/man.cgi?query=capsicum&sektion=4>
> See the vfs.lookup_cap_dotdot sysctl too.
Also, my apologies, as this may be tangential to the current
conversation, but since object-based capabilities were mentioned, I had
some design ideas around it while working on the memfd feature [1]. I
don’t know if the design for object-based capabilities has been
internally formalized yet, but since we’re at this juncture, it would
make me glad if any of this is helpful in any way :)
If I understand things correctly, the domain currently applies to ALL
file operations via paths and persists until the process exits.
Therefore, with disconnected directories, once a path component is
unlinked, security policies can be bypassed, as access checks on
previously visible ancestors might get skipped.
Current Landlock Architecture:
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Process -> Landlock Domain -> Access Decision
{Filesystem Rules, Network Rules, Scope Restrictions}
Path/Port Resolution + Domain Boundary Checks
Enhanced Architecture with Object Capabilities:
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Process -> Enhanced Landlock Domain -> Access Decision
━━
━━
{Path Rules, Network Rules, (AND) {FD Capabilities}
Scope Restrictions} |
━━━━━━━━━━━━━━━
Per-FD Rights
━━━━━━━━━━━━━━━
Traditional Resolution (calculated)
Unlike SCOPE which provides coarse-grained blocking, object capabilities
provide with the facility to add domain specific fine-grained individual
FD operations. So that we have:
Child Domain = Parent Domain & New Restrictions
= {
path_rules: Parent.path_rules & Child.path_rules,
net_rules: Parent.net_rules & Child.net_rules,
scope: Parent.scope | Child.scope, /* Additive */
fd_caps: path_rules & net_rules & scope & Child.allowed_fd_operations
}
where the Child domain *must* be more restrictive than the parent. Here
is an example:
/* Example */
Parent Domain = {
path_rules: [“/var/www” -> READ_FILE|READ_DIR, “/var/log” -> WRITE_FILE],
net_rules: [“80” -> BIND_TCP, “443” -> BIND_TCP],
scope: [SIGNAL, ABSTRACT_UNIX],
/* Auto-derived FD capabilities */
fd_caps: {
3: READ_FILE, /* /var/www/index.html */
7: READ_DIR, /* /var/www directory */
12: WRITE_FILE, /* /var/log/access.log */
15: BIND_TCP, /* socket bound to port 80 */
20: READ_FILE|READ_DIR /* /var/www/images/ */
}
}
/* Child creates new domain with additional restrictions */
Child.new_restrictions = {
path_rules: ["/var/www" -> READ_FILE only], /* Remove READ_DIR */
net_rules: [], /* Remove all network */
scope: [SIGNAL, ABSTRACT_UNIX, MEMFD_EXEC], /* Add MEMFD restriction */
}
/* Child FD capabilities = Parent & Child restrictions */
Child.fd_caps = {
3: READ_FILE, /* READ_FILE & READ_FILE = READ_FILE */
7: 0, /* READ_DIR & READ_FILE = none (no access) */
12: WRITE_FILE, /* WRITE_FILE unchanged (not restricted) */
15: 0, /* BIND_TCP & none = none (network blocked) */
20: READ_FILE /* (READ_FILE|READ_DIR) & READ_FILE = READ_FILE */
}
API Design: Reusing Existing Flags
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
/* Extended ruleset - reuse existing flags where possible */
struct landlock_ruleset_attr {
__u64 handled_access_fs; /* Existing: also applies to FDs */
__u64 handled_access_net; /* Existing: also applies to FDs */
__u64 scoped; /* Existing: domain boundaries */
__u64 handled_access_fd; /* NEW: FD-specific operations only */
};
/* New syscalls */
long landlock_set_fd_capability(int fd, __u64 access_rights, __u32 flags);
/* Reuse existing filesystem/network flags for FD operations */
landlock_set_fd_capability(file_fd, LANDLOCK_ACCESS_FS_READ_FILE, 0);
landlock_set_fd_capability(dir_fd, LANDLOCK_ACCESS_FS_READ_DIR, 0);
landlock_set_fd_capability(sock_fd, LANDLOCK_ACCESS_NET_BIND_TCP, 0);
`============'
With object capabilities, we assign access rights to file descriptors
directly, at open/alloc time, eliminating the need for path resolution
during future use.
This solves the core issue because:
• FDs remain valid even when disconnected, and
• Rights are bound to the object rather than its pathname.
Therefore, openat with dirfd should still work.
int dirfd = open(“/tmp/work", O_RDONLY); // Connected
unlink(”/tmp/work"); // Now disconnected
openat(dirfd, “file.txt”, O_RDONLY); // Still works, FD bound
Moreover, no path resolution is needed at a later stage and sandboxed
processes don’t bypass restrictions.
Would love to hear any feedback and thoughts on this.
Best,
Abhinav
[1] - <https://lore.kernel.org/all/20250719-memfd-exec-v1-0-0ef7feba5821@gmail.com/>
^ permalink raw reply
* Re: [RFC PATCH v2 30/34] lockdown: move initcalls to the LSM framework
From: Xiu Jianfeng @ 2025-07-25 8:12 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Tetsuo Handa, Nicolas Bouchinet,
Xiu Jianfeng
In-Reply-To: <20250721232142.77224-66-paul@paul-moore.com>
On 2025/7/22 7:21, Paul Moore wrote:
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
> security/lockdown/lockdown.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4813f168ff93..8d46886d2cca 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -161,8 +161,6 @@ static int __init lockdown_secfs_init(void)
> return PTR_ERR_OR_ZERO(dentry);
> }
>
> -core_initcall(lockdown_secfs_init);
> -
> #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
> DEFINE_EARLY_LSM(lockdown) = {
> #else
> @@ -170,4 +168,5 @@ DEFINE_LSM(lockdown) = {
> #endif
> .id = &lockdown_lsmid,
> .init = lockdown_lsm_init,
> + .initcall_core = lockdown_secfs_init,
> };
^ permalink raw reply
* Re: [PATCH v3 2/4] landlock: Fix handling of disconnected directories
From: Mickaël Salaün @ 2025-07-25 9:05 UTC (permalink / raw)
To: Abhinav Saxena
Cc: Günther Noack, Tingmao Wang, Jann Horn, John Johansen,
Al Viro, Ben Scarlato, Christian Brauner, Daniel Burgener,
Jeff Xu, NeilBrown, Paul Moore, Ryan Sullivan, Song Liu,
linux-fsdevel, linux-security-module
In-Reply-To: <87tt307vtd.fsf@gmail.com>
On Thu, Jul 24, 2025 at 09:54:22PM -0600, Abhinav Saxena wrote:
> Hi!
>
> Mickaël Salaün <mic@digikod.net> writes:
> > On Thu, Jul 24, 2025 at 04:49:24PM +0200, Günther Noack wrote:
> >> On Wed, Jul 23, 2025 at 11:01:42PM +0200, Mickaël Salaün wrote:
> >> > On Tue, Jul 22, 2025 at 07:04:02PM +0100, Tingmao Wang wrote:
> >> > > On the other hand, I’m still a bit uncertain about the domain check
> >> > > semantics. While it would not cause a rename to be allowed if it is
> >> > > otherwise not allowed by any rules on or above the mountpoint, this gets a
> >> > > bit weird if we have a situation where renames are allowed on the
> >> > > mountpoint or everywhere, but not read/writes, however read/writes are
> >> > > allowed directly on a file, but the dir containing that file gets
> >> > > disconnected so the sandboxed application can’t read or write to it.
> >> > > (Maybe someone would set up such a policy where renames are allowed,
> >> > > expecting Landlock to always prevent renames where additional permissions
> >> > > would be exposed?)
> >> > >
> >> > > In the above situation, if the file is then moved to a connected
> >> > > directory, it will become readable/writable again.
> >> >
> >> > We can generalize this issue to not only the end file but any component
> >> > of the path: disconnected directories. In fact, the main issue is the
> >> > potential inconsistency of access checks over time (e.g. between two
> >> > renames). This could be exploited to bypass the security checks done
> >> > for FS_REFER.
> >> >
> >> > I see two solutions:
> >> >
> >> > 1. *Always* walk down to the IS_ROOT directory, and then jump to the
> >> > mount point. This makes it possible to have consistent access checks
> >> > for renames and open/use. The first downside is that that would
> >> > change the current behavior for bind mounts that could get more
> >> > access rights (if the policy explicitly sets rights for the hidden
> >> > directories). The second downside is that we’ll do more walk.
> >> >
> >> > 2. Return -EACCES (or -ENOENT) for actions involving disconnected
> >> > directories, or renames of disconnected opened files. This second
> >> > solution is simpler and safer but completely disables the use of
> >> > disconnected directories and the rename of disconnected files for
> >> > sandboxed processes.
> >> >
> >> > It would be much better to be able to handle opened directories as
> >> > (object) capabilities, but that is not currently possible because of the
> >> > way paths are handled by the VFS and LSM hooks.
> >> >
> >> > Tingmao, Günther, Jann, what do you think?
> >>
> >> I have to admit that so far, I still failed to wrap my head around the
> >> full patch set and its possible corner cases. I hope I did not
> >> misunderstand things all too badly below:
> >>
> >> As far as I understand the proposed patch, we are “checkpointing” the
> >> intermediate results of the path walk at every mount point boundary,
> >> and in the case where we run into a disconnected directory in one of
> >> the nested mount points, we restore from the intermediate result at
> >> the previous mount point directory and skip to the next mount point.
> >
> > Correct
> >
> >>
> >> Visually speaking, if the layout is this (where “:” denotes a
> >> mountpoint boundary between the mountpoints MP1, MP2, MP3):
> >>
> >> dirfd
> >> |
> >> : V :
> >> : ham <— spam <— eggs <— x.txt
> >> : (disconn.) :
> >> : :
> >> / <— foo <— bar <— baz :
> >> : :
> >> MP1 MP2 MP3
> >>
> >> When a process holds a reference to the “spam” directory, which is now
> >> disconnected, and invokes openat(dirfd, “eggs/x.txt”, …), then we
> >> would:
> >>
> >> * traverse x.txt
> >> * traverse eggs (checkpointing the intermediate result) <-.
> >> * traverse spam |
> >> * traverse ham |
> >> * discover that ham is disconnected: |
> >> * restore the intermediate result from “eggs” ———’
> >> * continue the walk at foo
> >> * end up at the root
> >>
> >> So effectively, since the results from “spam” and “ham” are discarded,
> >> we would traverse only the inodes in the outer and inner mountpoints
> >> MP1 and MP3, but effectively return a result that looks like we did
> >> not traverse MP2?
> >
> > We’d still check MP2’s inode, but otherwise yes.
> >
>
> I don’t know if it makes sense, but can access rights be cached as part
> of the inode security blob? Although I am not sure if the LSM blob would
> exist after unlinking.
We're not talking about unlinked files but renamed directories that are
opened in a bind mount. There is no issue with unlinked files, only
when opening something new from an open directory.
With Landlock, because it is unprivileged and then has standalone
security policies, we would need to have one cache per sandbox, which
has its own set of issues. That would be independant from the current
patch series anyway.
>
> But if it does, maybe during unlink, keep the cached rights for MP2,
> and during openat():
> 1. Start at disconnected “spam” inode
> 2. Check spam->i_security->allowed_access <- Cached MP2 rights
> 3. Continue normal path walk with preserved access context
As explained, we cannot do that with the current LSM and VFS
APIs
>
> >>
> >> Maybe (likely) I misread the code. :) It’s not clear to me what the
> >> thinking behind this is. Also, if there was another directory in
> >> between “spam” and “eggs” in MP2, wouldn’t we be missing the access
> >> rights attached to this directory?
> >
> > Yes, we would ignore this access right because we don’t know that the
> > path was resolved from spam.
> >
> >>
> >>
> >> Regarding the capability approach:
> >>
> >> I agree that a “capability” approach would be the better solution, but
> >> it seems infeasible with the existing LSM hooks at the moment. I
> >> would be in favor of it though.
> >
> > Yes, it would be a new feature with potential important changes.
> >
> > In the meantime, we still need a fix for disconnected directories, and
> > this fix needs to be backported. That’s why the capability approach is
> > not part of the two solutions. ;)
> >
> >>
> >> To spell it out a bit more explicitly what that would mean in my mind:
> >>
> >> When a path is looked up relative to a dirfd, the path walk upwards
> >> would terminate at the dirfd and use previously calculated access
> >> rights stored in the associated struct file. These access rights
> >> would be determined at the time of opening the dirfd, similar to how we
> >> are already storing the “truncate” access right today for regular
> >> files.
> >>
> >> (Remark: There might still be corner cases where we have to solve it
> >> the hard way, if someone uses “..” together with a dirfd-relative
> >> lookup.)
> >
> > Yep, real capabilities don’t have “..” in their design. On Linux (and
> > Landlock), we need to properly handle “..”, which is challenging.
> >
> >>
> >> I also looked at what it would take to change the LSM hooks to pass
> >> the directory that the lookup was done relative to, but it seems that
> >> this would have to be passed through a bunch of VFS callbacks as well,
> >> which seems like a larger change. I would be curious whether that
> >> would be deemed an acceptable change.
> >>
> >> —Günther
> >>
> >>
> >> P.S. Related to relative directory lookups, there is some movement in
> >> the BSDs as well to use dirfds as capabilities, by adding a flag to
> >> open directories that enforces O_BENEATH on subsequent opens:
> >>
> >> * <https://undeadly.org/cgi?action=article;sid=20250529080623>
> >> * <https://reviews.freebsd.org/D50371>
> >>
> >> (both found via <https://news.ycombinator.com/item?id=44575361>)
> >>
> >> If a dirfd had such a flag, that would get rid of the corner case
> >> above.
> >
> > This would be nice but it would not solve the current issue because we
> > cannot force all processes to use this flag (which breaks some use
> > cases).
> >
> > FYI, Capsicum is a more complete implementation:
> > <https://man.freebsd.org/cgi/man.cgi?query=capsicum&sektion=4>
> > See the vfs.lookup_cap_dotdot sysctl too.
>
> Also, my apologies, as this may be tangential to the current
> conversation, but since object-based capabilities were mentioned, I had
> some design ideas around it while working on the memfd feature [1]. I
Thanks for your patch series! It is on my todo list but I'll need a bit
of time to review it.
> don’t know if the design for object-based capabilities has been
> internally formalized yet, but since we’re at this juncture, it would
> make me glad if any of this is helpful in any way :)
>
> If I understand things correctly, the domain currently applies to ALL
> file operations via paths and persists until the process exits.
Yes (but it doesn't apply on already opened file descriptors).
> Therefore, with disconnected directories, once a path component is
> unlinked, security policies can be bypassed, as access checks on
> previously visible ancestors might get skipped.
There is no security bypass, because Landlock is a deny-by-default (when
handled) access control, and also because filesystem's parent
directories are evaluated (but not necessarily all the visible/mounted
filesystems).
Also, there is no issue with unlinked files.
>
> Current Landlock Architecture:
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> Process -> Landlock Domain -> Access Decision
>
> {Filesystem Rules, Network Rules, Scope Restrictions}
>
> Path/Port Resolution + Domain Boundary Checks
>
>
> Enhanced Architecture with Object Capabilities:
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> Process -> Enhanced Landlock Domain -> Access Decision
> ━━
>
> ━━
> {Path Rules, Network Rules, (AND) {FD Capabilities}
> Scope Restrictions} |
> ━━━━━━━━━━━━━━━
> Per-FD Rights
> ━━━━━━━━━━━━━━━
> Traditional Resolution (calculated)
>
> Unlike SCOPE which provides coarse-grained blocking, object capabilities
> provide with the facility to add domain specific fine-grained individual
> FD operations. So that we have:
>
> Child Domain = Parent Domain & New Restrictions
> = {
> path_rules: Parent.path_rules & Child.path_rules,
> net_rules: Parent.net_rules & Child.net_rules,
> scope: Parent.scope | Child.scope, /* Additive */
> fd_caps: path_rules & net_rules & scope & Child.allowed_fd_operations
> }
>
> where the Child domain *must* be more restrictive than the parent. Here
> is an example:
>
> /* Example */
> Parent Domain = {
> path_rules: [“/var/www” -> READ_FILE|READ_DIR, “/var/log” -> WRITE_FILE],
> net_rules: [“80” -> BIND_TCP, “443” -> BIND_TCP],
> scope: [SIGNAL, ABSTRACT_UNIX],
>
> /* Auto-derived FD capabilities */
> fd_caps: {
> 3: READ_FILE, /* /var/www/index.html */
> 7: READ_DIR, /* /var/www directory */
> 12: WRITE_FILE, /* /var/log/access.log */
> 15: BIND_TCP, /* socket bound to port 80 */
> 20: READ_FILE|READ_DIR /* /var/www/images/ */
> }
> }
>
> /* Child creates new domain with additional restrictions */
> Child.new_restrictions = {
> path_rules: ["/var/www" -> READ_FILE only], /* Remove READ_DIR */
> net_rules: [], /* Remove all network */
> scope: [SIGNAL, ABSTRACT_UNIX, MEMFD_EXEC], /* Add MEMFD restriction */
> }
>
> /* Child FD capabilities = Parent & Child restrictions */
> Child.fd_caps = {
> 3: READ_FILE, /* READ_FILE & READ_FILE = READ_FILE */
> 7: 0, /* READ_DIR & READ_FILE = none (no access) */
> 12: WRITE_FILE, /* WRITE_FILE unchanged (not restricted) */
> 15: 0, /* BIND_TCP & none = none (network blocked) */
> 20: READ_FILE /* (READ_FILE|READ_DIR) & READ_FILE = READ_FILE */
> }
>
> API Design: Reusing Existing Flags
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> /* Extended ruleset - reuse existing flags where possible */
> struct landlock_ruleset_attr {
> __u64 handled_access_fs; /* Existing: also applies to FDs */
> __u64 handled_access_net; /* Existing: also applies to FDs */
> __u64 scoped; /* Existing: domain boundaries */
> __u64 handled_access_fd; /* NEW: FD-specific operations only */
> };
>
> /* New syscalls */
> long landlock_set_fd_capability(int fd, __u64 access_rights, __u32 flags);
Such a syscall could be useful in some scenarios but in most cases
programs should not require to be modified.
>
> /* Reuse existing filesystem/network flags for FD operations */
> landlock_set_fd_capability(file_fd, LANDLOCK_ACCESS_FS_READ_FILE, 0);
> landlock_set_fd_capability(dir_fd, LANDLOCK_ACCESS_FS_READ_DIR, 0);
> landlock_set_fd_capability(sock_fd, LANDLOCK_ACCESS_NET_BIND_TCP, 0);
>
> `============'
>
> With object capabilities, we assign access rights to file descriptors
> directly, at open/alloc time, eliminating the need for path resolution
> during future use.
Except when ".." is used to resolve a path relative to such
capability/FD, and we need to support that for compatibility reasons.
>
> This solves the core issue because:
> • FDs remain valid even when disconnected, and
> • Rights are bound to the object rather than its pathname.
>
> Therefore, openat with dirfd should still work.
>
> int dirfd = open(“/tmp/work", O_RDONLY); // Connected
> unlink(”/tmp/work"); // Now disconnected
It's not possible to unlink a directory nor rmdir a non-empty one.
> openat(dirfd, “file.txt”, O_RDONLY); // Still works, FD bound
>
> Moreover, no path resolution is needed at a later stage and sandboxed
> processes don’t bypass restrictions.
>
> Would love to hear any feedback and thoughts on this.
I think we all agree that capabilities would be nice, but the VFS and
LSM's APIs would need to be updated, and that would need to update user
space code, while breaking some use cases.
>
> Best,
> Abhinav
>
> [1] - <https://lore.kernel.org/all/20250719-memfd-exec-v1-0-0ef7feba5821@gmail.com/>
^ permalink raw reply
* [PATCH -next] apparmor: Remove the unused variable rules
From: Jiapeng Chong @ 2025-07-25 9:52 UTC (permalink / raw)
To: john.johansen
Cc: paul, jmorris, serge, apparmor, linux-security-module,
linux-kernel, Jiapeng Chong, Abaci Robot
Variable rules is not effectively used, so delete it.
security/apparmor/lsm.c:182:23: warning: variable ‘rules’ set but not used.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=22942
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
security/apparmor/lsm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 5b1b5ac831e8..182a0e55802e 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -179,10 +179,8 @@ static int apparmor_capget(const struct task_struct *target, kernel_cap_t *effec
struct label_it i;
label_for_each_confined(i, label, profile) {
- struct aa_ruleset *rules;
kernel_cap_t allowed;
- rules = profile->label.rules[0];
allowed = aa_profile_capget(profile);
*effective = cap_intersect(*effective, allowed);
*permitted = cap_intersect(*permitted, allowed);
--
2.43.5
^ permalink raw reply related
* [GIT PULL] Landlock update for v6.17-rc1
From: Mickaël Salaün @ 2025-07-25 9:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Mickaël Salaün, Brahmajit Das, Günther Noack,
Song Liu, Tingmao Wang, linux-kernel, linux-security-module
Linus,
This PR fixes test issues, improves build compatibility, and adds new tests.
Please pull these changes for v6.17-rc1 . These commits merge cleanly
with your master branch. They have been been tested in the latest
linux-next releases.
Test coverage for security/landlock is unchanged (with Kselftest).
Regards,
Mickaël
--
The following changes since commit e04c78d86a9699d136910cfc0bdcf01087e3267e:
Linux 6.16-rc2 (2025-06-15 13:49:41 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git tags/landlock-6.17-rc1
for you to fetch changes up to 6803b6ebb8164c1d306600f8836a39b6fceffeec:
landlock: Fix cosmetic change (2025-07-19 12:44:16 +0200)
----------------------------------------------------------------
Landlock update for v6.17-rc1
----------------------------------------------------------------
Brahmajit Das (1):
samples/landlock: Fix building on musl libc
Mickaël Salaün (3):
selftests/landlock: Fix readlink check
selftests/landlock: Add test to check rule tied to covered mount point
landlock: Fix cosmetic change
Song Liu (1):
selftests/landlock: Fix build of audit_test
Tingmao Wang (1):
landlock: Fix warning from KUnit tests
samples/landlock/sandboxer.c | 5 +-
security/landlock/fs.c | 1 +
security/landlock/id.c | 69 ++++++++++++++++-----------
tools/testing/selftests/landlock/audit.h | 7 +--
tools/testing/selftests/landlock/audit_test.c | 1 +
tools/testing/selftests/landlock/fs_test.c | 40 ++++++++++++++++
6 files changed, 92 insertions(+), 31 deletions(-)
^ permalink raw reply
* Re: [RFC PATCH v2 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Casey Schaufler @ 2025-07-25 14:26 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Tetsuo Handa,
Nicolas Bouchinet, Xiu Jianfeng, Casey Schaufler
In-Reply-To: <CAHC9VhThNtGCA-jmjRagJfzPJaTh9myqFcwqA4J5Zv3ojEFDfQ@mail.gmail.com>
On 7/24/2025 7:28 PM, Paul Moore wrote:
> On Thu, Jul 24, 2025 at 11:39 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 7/21/2025 4:21 PM, Paul Moore wrote:
>>> The LSM currently has a lot of code to maintain a list of the currently
>>> active LSMs in a human readable string, with the only user being the
>>> "/sys/kernel/security/lsm" code. Let's drop all of that code and
>>> generate the string on first use and then cache it for subsequent use.
>>>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>> ---
>>> include/linux/lsm_hooks.h | 1 -
>>> security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
>>> security/lsm_init.c | 49 --------------------------------
>>> 3 files changed, 57 insertions(+), 52 deletions(-)
> ..
>
>>> +/* NOTE: we never free the string below once it is set. */
>>> +static DEFINE_SPINLOCK(lsm_read_lock);
>>> +static char *lsm_read_str = NULL;
>>> +static ssize_t lsm_read_len = 0;
>>> +
>>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>>> loff_t *ppos)
>>> {
>>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
>>> - strlen(lsm_names));
>>> + int i;
>>> + char *str;
>>> + ssize_t len;
>>> +
>>> +restart:
>>> +
>>> + rcu_read_lock();
>>> + if (!lsm_read_str) {
>>> + /* we need to generate the string and try again */
>>> + rcu_read_unlock();
>>> + goto generate_string;
>>> + }
>>> + len = simple_read_from_buffer(buf, count, ppos,
>>> + rcu_dereference(lsm_read_str),
>>> + lsm_read_len);
>>> + rcu_read_unlock();
>>> + return len;
>>> +
>>> +generate_string:
>>> +
>>> + for (i = 0; i < lsm_active_cnt; i++)
>>> + /* the '+ 1' accounts for either a comma or a NUL */
>>> + len += strlen(lsm_idlist[i]->name) + 1;
>>> +
>>> + str = kmalloc(len, GFP_KERNEL);
>>> + if (!str)
>>> + return -ENOMEM;
>>> + str[0] = '\0';
>>> +
>>> + for (i = 0; i < lsm_active_cnt; i++) {
>>> + if (i > 0)
>>> + strcat(str, ",");
>>> + strcat(str, lsm_idlist[i]->name);
>>> + }
>>> +
>>> + spin_lock(&lsm_read_lock);
>>> + if (lsm_read_str) {
>>> + /* we raced and lost */
>>> + spin_unlock(&lsm_read_lock);
>>> + kfree(str);
>>> + goto restart;
>>> + }
>>> + lsm_read_str = str;
>>> + lsm_read_len = len;
>> You're going to get a nul byte at the end of the string because
>> you accounted for the ',' above, but there isn't one at the end
>> of the string.
> I'm not sure I understand your concern here, can you phrase it differently?
"lockdown,capability,...,evm\0" You get the '\0' because you always expect
a trailing ','. On the last element there is no ',' but the length is added
as if there is.
+ lsm_read_len = len - 1;
will fix the problem.
>
> If you're worried about lsm_read_str potentially not being terminated
> with a NUL byte, the strcat() should copy over the trailing NUL.
>
>>> + spin_unlock(&lsm_read_lock);
>>> +
>>> + goto restart;
>>> }
^ permalink raw reply
* Re: [RFC PATCH v2 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-07-25 16:42 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Tetsuo Handa,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <6621fbb0-fb66-4aa0-b77b-1cd0db195660@schaufler-ca.com>
On Fri, Jul 25, 2025 at 10:27 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/24/2025 7:28 PM, Paul Moore wrote:
> > On Thu, Jul 24, 2025 at 11:39 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 7/21/2025 4:21 PM, Paul Moore wrote:
> >>> The LSM currently has a lot of code to maintain a list of the currently
> >>> active LSMs in a human readable string, with the only user being the
> >>> "/sys/kernel/security/lsm" code. Let's drop all of that code and
> >>> generate the string on first use and then cache it for subsequent use.
> >>>
> >>> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >>> ---
> >>> include/linux/lsm_hooks.h | 1 -
> >>> security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
> >>> security/lsm_init.c | 49 --------------------------------
> >>> 3 files changed, 57 insertions(+), 52 deletions(-)
> > ..
> >
> >>> +/* NOTE: we never free the string below once it is set. */
> >>> +static DEFINE_SPINLOCK(lsm_read_lock);
> >>> +static char *lsm_read_str = NULL;
> >>> +static ssize_t lsm_read_len = 0;
> >>> +
> >>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> >>> loff_t *ppos)
> >>> {
> >>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> >>> - strlen(lsm_names));
> >>> + int i;
> >>> + char *str;
> >>> + ssize_t len;
> >>> +
> >>> +restart:
> >>> +
> >>> + rcu_read_lock();
> >>> + if (!lsm_read_str) {
> >>> + /* we need to generate the string and try again */
> >>> + rcu_read_unlock();
> >>> + goto generate_string;
> >>> + }
> >>> + len = simple_read_from_buffer(buf, count, ppos,
> >>> + rcu_dereference(lsm_read_str),
> >>> + lsm_read_len);
> >>> + rcu_read_unlock();
> >>> + return len;
> >>> +
> >>> +generate_string:
> >>> +
> >>> + for (i = 0; i < lsm_active_cnt; i++)
> >>> + /* the '+ 1' accounts for either a comma or a NUL */
> >>> + len += strlen(lsm_idlist[i]->name) + 1;
> >>> +
> >>> + str = kmalloc(len, GFP_KERNEL);
> >>> + if (!str)
> >>> + return -ENOMEM;
> >>> + str[0] = '\0';
> >>> +
> >>> + for (i = 0; i < lsm_active_cnt; i++) {
> >>> + if (i > 0)
> >>> + strcat(str, ",");
> >>> + strcat(str, lsm_idlist[i]->name);
> >>> + }
> >>> +
> >>> + spin_lock(&lsm_read_lock);
> >>> + if (lsm_read_str) {
> >>> + /* we raced and lost */
> >>> + spin_unlock(&lsm_read_lock);
> >>> + kfree(str);
> >>> + goto restart;
> >>> + }
> >>> + lsm_read_str = str;
> >>> + lsm_read_len = len;
> >> You're going to get a nul byte at the end of the string because
> >> you accounted for the ',' above, but there isn't one at the end
> >> of the string.
> > I'm not sure I understand your concern here, can you phrase it differently?
>
> "lockdown,capability,...,evm\0" You get the '\0' because you always expect
> a trailing ','. On the last element there is no ',' but the length is added
> as if there is.
>
> + lsm_read_len = len - 1;
>
> will fix the problem.
Ah, yes, gotcha. Thanks for catching this, the fix will be in the
next revision.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 30/34] lockdown: move initcalls to the LSM framework
From: Paul Moore @ 2025-07-25 16:51 UTC (permalink / raw)
To: Xiu Jianfeng
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <3101077d-a5e2-d08b-03c2-2ed064a35b54@huaweicloud.com>
On Fri, Jul 25, 2025 at 4:12 AM Xiu Jianfeng
<xiujianfeng@huaweicloud.com> wrote:
> On 2025/7/22 7:21, Paul Moore wrote:
> > Reviewed-by: Kees Cook <kees@kernel.org>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Reviewed-by: Xiu Jianfeng <xiujianfeng@huawei.com>
Thank you for reviewing this patch. As you are a Lockdown maintainer,
can I change your reviewed-by into an acked-by tag?
> > ---
> > security/lockdown/lockdown.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> > index 4813f168ff93..8d46886d2cca 100644
> > --- a/security/lockdown/lockdown.c
> > +++ b/security/lockdown/lockdown.c
> > @@ -161,8 +161,6 @@ static int __init lockdown_secfs_init(void)
> > return PTR_ERR_OR_ZERO(dentry);
> > }
> >
> > -core_initcall(lockdown_secfs_init);
> > -
> > #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
> > DEFINE_EARLY_LSM(lockdown) = {
> > #else
> > @@ -170,4 +168,5 @@ DEFINE_LSM(lockdown) = {
> > #endif
> > .id = &lockdown_lsmid,
> > .init = lockdown_lsm_init,
> > + .initcall_core = lockdown_secfs_init,
> > };
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] lsm/lsm-pr-20250725
From: Paul Moore @ 2025-07-25 20:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
Linus,
Three fairly small LSM framework patches for the upcoming merge window,
but I wanted to mention that this pull request isn't based on the usual
-rc1 tag, but rather on a VFS merge that happened shortly after -rc2 so
we could pick up an important xattr/LSM fix.
- Add Nicolas Bouchinet and Xiu Jianfeng as Lockdown maintainers
The Lockdown LSM has been without a dedicated mantainer since its
original acceptance upstream, and it has suffered as a result.
Thankfully we have two new volunteers who together I believe have
the background and desire to help ensure Lockdown is properly
supported.
- Remove the unused cap_mmap_file() declaration.
Included in the LSM framework pull request with Serge's ACK.
Paul
--
The following changes since commit fe78e02600f83d81e55f6fc352d82c4f264a2901:
Merge tag 'vfs-6.16-rc3.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
(2025-06-16 08:18:43 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
tags/lsm-pr-20250725
for you to fetch changes up to 5d8b97c946777118930e1cfb075cab59a139ca7c:
MAINTAINERS: Add Xiu and myself as Lockdown maintainers
(2025-07-10 17:56:09 -0400)
----------------------------------------------------------------
lsm/stable-6.17 PR 20250725
----------------------------------------------------------------
Kalevi Kolttonen (1):
lsm: trivial comment fix
Nicolas Bouchinet (1):
MAINTAINERS: Add Xiu and myself as Lockdown maintainers
Yue Haibing (1):
security: Remove unused declaration cap_mmap_file()
MAINTAINERS | 4 +++-
include/linux/security.h | 2 --
security/security.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] selinux/selinux-pr-20250725
From: Paul Moore @ 2025-07-25 20:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
Linus,
Six SELinux patches for the upcoming merge window, the highlights are
below, but I also wanted to mention that this pull request isn't based
on the usual -rc1 tag, but rather on a VFS merge that happened shortly
after -rc2 so we could pick up an important xattr/LSM fix.
- Introduce the concept of a SELinux "neveraudit" type which prevents
all auditing of the given type/domain.
Taken by itself, the benefit of marking a SELinux domain with the
"neveraudit" tag is likely not very interesting, especially given
the significant overlap with the "dontaudit" tag. However, given
that the "neveraudit" tag applies to *all* auditing of the tagged
domain, we can do some fairly interesting optimizations when a
SELinux domain is marked as both "permissive" and "dontaudit" (think
of the unconfined_t domain). While this pull request includes
optimized inode permission and getattr hooks, these optimizations
require SELinux policy changes, therefore the improvements may not be
visible on standard downstream Linux distos for a period of time.
- Continue the deprecation process of /sys/fs/selinux/user.
After removing the associated userspace code in 2020, we marked the
/sys/fs/selinux/user interface as deprecated in Linux v6.13 with
pr_warn() and the usual documention update. This pull request adds
a five second sleep after the pr_warn(), following a previous
deprecation process pattern that has worked well for us in the past
in helping identify any existing users that we haven't yet reached.
- Add a __GFP_NOWARN flag to our initial hash table allocation.
Fuzzers such a syzbot often attempt abnormally large SELinux policy
loads, which the SELinux code gracefully handles by checking for
allocation failures, but not before the allocator emits a warning
which causes the automated fuzzing to flag this as an error and
report it to the list. While we want to continue to support the
work done by the fuzzing teams, we want to focus on proper issues
and not an error case that is already handled safely. Add a NOWARN
flag to quiet the allocator and prevent syzbot from tripping on this
again.
- Remove some unnecessary selinuxfs cleanup code, courtesy of Al.
- Update the SELinux in-kernel documentation with pointers to additional
information.
Paul
--
The following changes since commit fe78e02600f83d81e55f6fc352d82c4f264a2901:
Merge tag 'vfs-6.16-rc3.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
(2025-06-16 08:18:43 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20250725
for you to fetch changes up to ee79ba39b3d6fdcfa53de6519d7e259e284e78f7:
selinux: don't bother with selinuxfs_info_free() on failures
(2025-06-24 19:39:28 -0400)
----------------------------------------------------------------
selinux/stable-6.17 PR 20250725
----------------------------------------------------------------
Al Viro (1):
selinux: don't bother with selinuxfs_info_free() on failures
Paul Moore (2):
selinux: add a 5 second sleep to /sys/fs/selinux/user
selinux: add __GFP_NOWARN to hashtab_init() allocations
Stephen Smalley (3):
documentation: add links to SELinux resources
selinux: introduce neveraudit types
selinux: optimize selinux_inode_getattr/permission() based on
neveraudit|permissive
Documentation/admin-guide/LSM/SELinux.rst | 11 +++++++++++
security/selinux/hooks.c | 14 +++++++++++++-
security/selinux/include/avc.h | 4 ++++
security/selinux/include/objsec.h | 8 ++++++++
security/selinux/include/security.h | 4 +++-
security/selinux/selinuxfs.c | 3 +--
security/selinux/ss/hashtab.c | 3 ++-
security/selinux/ss/policydb.c | 19 +++++++++++++++++++
security/selinux/ss/policydb.h | 2 ++
security/selinux/ss/services.c | 20 ++++++++++++++++++++
10 files changed, 83 insertions(+), 5 deletions(-)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v4 0/4] stackleak: Support Clang stack depth tracking
From: Nathan Chancellor @ 2025-07-26 0:43 UTC (permalink / raw)
To: Kees Cook
Cc: Arnd Bergmann, Will Deacon, Ard Biesheuvel, Catalin Marinas,
Jonathan Cameron, Gavin Shan, Russell King (Oracle), James Morse,
Oza Pawandeep, Anshuman Khandual, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Mike Rapoport, Vitaly Kuznetsov, Henrique de Moraes Holschuh,
Hans de Goede, Ilpo Järvinen, Rafael J. Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, Marco Elver, Andrey Konovalov, Andrey Ryabinin,
Hou Wenlong, Andrew Morton, Masahiro Yamada,
Peter Zijlstra (Intel), Luis Chamberlain, Sami Tolvanen,
Christophe Leroy, Nicolas Schier, Gustavo A. R. Silva,
Andy Lutomirski, Baoquan He, Alexander Graf, Changyuan Lyu,
Paul Moore, James Morris, Serge E. Hallyn, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jan Beulich, Boqun Feng,
Viresh Kumar, Paul E. McKenney, Bibo Mao, linux-kernel,
linux-arm-kernel, x86, kvm, ibm-acpi-devel, platform-driver-x86,
linux-acpi, linux-trace-kernel, linux-efi, linux-mm, kasan-dev,
linux-kbuild, linux-hardening, kexec, linux-security-module, llvm
In-Reply-To: <20250724054419.it.405-kees@kernel.org>
Hi Kees,
On Wed, Jul 23, 2025 at 10:50:24PM -0700, Kees Cook wrote:
> v4:
> - rebase on for-next/hardening tree (took subset of v3 patches)
> - improve commit logs for x86 and arm64 changes (Mike, Will, Ard)
> v3: https://lore.kernel.org/lkml/20250717231756.make.423-kees@kernel.org/
> v2: https://lore.kernel.org/lkml/20250523043251.it.550-kees@kernel.org/
> v1: https://lore.kernel.org/lkml/20250507180852.work.231-kees@kernel.org/
>
> Hi,
>
> These are the remaining changes needed to support Clang stack depth
> tracking for kstack_erase (nee stackleak).
A few build issues that I see when building next-20250725, which seem
related to this series.
1. I see
ld.lld: error: undefined symbol: __sanitizer_cov_stack_depth
>>> referenced by atags_to_fdt.c
>>> arch/arm/boot/compressed/atags_to_fdt.o:(atags_to_fdt)
make[5]: *** [arch/arm/boot/compressed/Makefile:152: arch/arm/boot/compressed/vmlinux] Error 1
when building ARCH=arm allmodconfig on next-20250725. The following diff appears to cure that one.
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index f9075edfd773..f6142946b162 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -9,7 +9,6 @@ OBJS =
HEAD = head.o
OBJS += misc.o decompress.o
-CFLAGS_decompress.o += $(DISABLE_KSTACK_ERASE)
ifeq ($(CONFIG_DEBUG_UNCOMPRESS),y)
OBJS += debug.o
AFLAGS_head.o += -DDEBUG
@@ -96,7 +95,7 @@ KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
-I$(srctree)/scripts/dtc/libfdt -fno-stack-protector \
- -I$(obj)
+ -I$(obj) $(DISABLE_KSTACK_ERASE)
ccflags-remove-$(CONFIG_FUNCTION_TRACER) += -pg
asflags-y := -DZIMAGE
--
2. I see
kernel/kstack_erase.c:168:2: warning: function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only' [-Wexcessive-regsave]
168 | BUILD_BUG_ON(CONFIG_KSTACK_ERASE_TRACK_MIN_SIZE > KSTACK_ERASE_SEARCH_DEPTH);
| ^
include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^
include/linux/compiler_types.h:568:2: note: expanded from macro 'compiletime_assert'
568 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:556:2: note: expanded from macro '_compiletime_assert'
556 | __compiletime_assert(condition, msg, prefix, suffix)
| ^
include/linux/compiler_types.h:549:4: note: expanded from macro '__compiletime_assert'
549 | prefix ## suffix(); \
| ^
<scratch space>:97:1: note: expanded from here
97 | __compiletime_assert_521
| ^
kernel/kstack_erase.c:168:2: note: '__compiletime_assert_521' declared here
include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^
include/linux/compiler_types.h:568:2: note: expanded from macro 'compiletime_assert'
568 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:556:2: note: expanded from macro '_compiletime_assert'
556 | __compiletime_assert(condition, msg, prefix, suffix)
| ^
include/linux/compiler_types.h:546:26: note: expanded from macro '__compiletime_assert'
546 | __noreturn extern void prefix ## suffix(void) \
| ^
<scratch space>:96:1: note: expanded from here
96 | __compiletime_assert_521
| ^
kernel/kstack_erase.c:172:11: warning: function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only' [-Wexcessive-regsave]
172 | if (sp < current->lowest_stack &&
| ^
arch/x86/include/asm/current.h:28:17: note: expanded from macro 'current'
28 | #define current get_current()
| ^
arch/x86/include/asm/current.h:20:44: note: 'get_current' declared here
20 | static __always_inline struct task_struct *get_current(void)
| ^
kernel/kstack_erase.c:173:37: warning: function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only' [-Wexcessive-regsave]
173 | sp >= stackleak_task_low_bound(current)) {
| ^
arch/x86/include/asm/current.h:28:17: note: expanded from macro 'current'
28 | #define current get_current()
| ^
arch/x86/include/asm/current.h:20:44: note: 'get_current' declared here
20 | static __always_inline struct task_struct *get_current(void)
| ^
when building ARCH=i386 allmodconfig.
3. I see
In file included from kernel/fork.c:96:
include/linux/kstack_erase.h:29:37: error: passing 'const struct task_struct *' to parameter of type 'struct task_struct *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
29 | return (unsigned long)end_of_stack(tsk) + sizeof(unsigned long);
| ^~~
include/linux/sched/task_stack.h:56:63: note: passing argument to parameter 'p' here
56 | static inline unsigned long *end_of_stack(struct task_struct *p)
| ^
when building ARCH=loongarch allmodconfig, which does not support
CONFIG_THREAD_INFO_IN_TASK it seems.
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH v4 0/4] stackleak: Support Clang stack depth tracking
From: Kees Cook @ 2025-07-26 6:27 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Will Deacon, Ard Biesheuvel, Catalin Marinas,
Jonathan Cameron, Gavin Shan, Russell King (Oracle), James Morse,
Oza Pawandeep, Anshuman Khandual, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Mike Rapoport, Vitaly Kuznetsov, Henrique de Moraes Holschuh,
Hans de Goede, Ilpo Järvinen, Rafael J. Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, Marco Elver, Andrey Konovalov, Andrey Ryabinin,
Hou Wenlong, Andrew Morton, Masahiro Yamada,
Peter Zijlstra (Intel), Luis Chamberlain, Sami Tolvanen,
Christophe Leroy, Nicolas Schier, Gustavo A. R. Silva,
Andy Lutomirski, Baoquan He, Alexander Graf, Changyuan Lyu,
Paul Moore, James Morris, Serge E. Hallyn, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jan Beulich, Boqun Feng,
Viresh Kumar, Paul E. McKenney, Bibo Mao, linux-kernel,
linux-arm-kernel, x86, kvm, ibm-acpi-devel, platform-driver-x86,
linux-acpi, linux-trace-kernel, linux-efi, linux-mm, kasan-dev,
linux-kbuild, linux-hardening, kexec, linux-security-module, llvm
In-Reply-To: <20250726004313.GA3650901@ax162>
On Fri, Jul 25, 2025 at 05:43:13PM -0700, Nathan Chancellor wrote:
> A few build issues that I see when building next-20250725, which seem
> related to this series.
AH! Thank you for letting me know!
> 1. I see
>
> ld.lld: error: undefined symbol: __sanitizer_cov_stack_depth
> >>> referenced by atags_to_fdt.c
> >>> arch/arm/boot/compressed/atags_to_fdt.o:(atags_to_fdt)
> make[5]: *** [arch/arm/boot/compressed/Makefile:152: arch/arm/boot/compressed/vmlinux] Error 1
>
> when building ARCH=arm allmodconfig on next-20250725. The following diff appears to cure that one.
Ah-ha perfect. Yes, that matches what I was expecting to fix it, I was
just about to start working on it, but you beat me to it. :) The same
was reported here:
https://lore.kernel.org/all/CA+G9fYtBk8qnpWvoaFwymCx5s5i-5KXtPGpmf=_+UKJddCOnLA@mail.gmail.com
> 2. I see
>
> kernel/kstack_erase.c:168:2: warning: function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only' [-Wexcessive-regsave]
> [...]
> when building ARCH=i386 allmodconfig.
Oh, hm, I will figure that out.
> 3. I see
>
> In file included from kernel/fork.c:96:
> include/linux/kstack_erase.h:29:37: error: passing 'const struct task_struct *' to parameter of type 'struct task_struct *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> 29 | return (unsigned long)end_of_stack(tsk) + sizeof(unsigned long);
> | ^~~
> include/linux/sched/task_stack.h:56:63: note: passing argument to parameter 'p' here
> 56 | static inline unsigned long *end_of_stack(struct task_struct *p)
> | ^
>
> when building ARCH=loongarch allmodconfig, which does not support
> CONFIG_THREAD_INFO_IN_TASK it seems.
Oh, eek. Yeah, I'll need to make an explicit dependency I guess? ("How
did this ever work?")
Thanks again!
--
Kees Cook
^ permalink raw reply
* Re: [RFC PATCH v2 30/34] lockdown: move initcalls to the LSM framework
From: xiujianfeng @ 2025-07-26 9:38 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhR_24Zv7u0Btz8pSk420Totnx2uRyVdoHU1tXevWKw5mA@mail.gmail.com>
On 2025/7/26 0:51, Paul Moore wrote:
> On Fri, Jul 25, 2025 at 4:12 AM Xiu Jianfeng
> <xiujianfeng@huaweicloud.com> wrote:
>> On 2025/7/22 7:21, Paul Moore wrote:
>>> Reviewed-by: Kees Cook <kees@kernel.org>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>
>> Reviewed-by: Xiu Jianfeng <xiujianfeng@huawei.com>
>
> Thank you for reviewing this patch. As you are a Lockdown maintainer,
> can I change your reviewed-by into an acked-by tag?
Yes, absolutely! Thanks for checking!
>
>>> ---
>>> security/lockdown/lockdown.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
>>> index 4813f168ff93..8d46886d2cca 100644
>>> --- a/security/lockdown/lockdown.c
>>> +++ b/security/lockdown/lockdown.c
>>> @@ -161,8 +161,6 @@ static int __init lockdown_secfs_init(void)
>>> return PTR_ERR_OR_ZERO(dentry);
>>> }
>>>
>>> -core_initcall(lockdown_secfs_init);
>>> -
>>> #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
>>> DEFINE_EARLY_LSM(lockdown) = {
>>> #else
>>> @@ -170,4 +168,5 @@ DEFINE_LSM(lockdown) = {
>>> #endif
>>> .id = &lockdown_lsmid,
>>> .init = lockdown_lsm_init,
>>> + .initcall_core = lockdown_secfs_init,
>>> };
>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 0/5] bpf path iterator
From: Song Liu @ 2025-07-26 9:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: NeilBrown, Christian Brauner, Tingmao Wang, Song Liu,
bpf@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, Kernel Team,
andrii@kernel.org, eddyz87@gmail.com, ast@kernel.org,
daniel@iogearbox.net, martin.lau@linux.dev,
viro@zeniv.linux.org.uk, jack@suse.cz, kpsingh@kernel.org,
mattbobrowski@google.com, Günther Noack, Jann Horn
In-Reply-To: <20250724.ij7AhF9quoow@digikod.net>
> On Jul 25, 2025, at 1:35 AM, Mickaël Salaün <mic@digikod.net> wrote:
[...]
>>>
>>> Do my questions above make any sense? Or maybe I totally
>>> misunderstood something?
>>
>> Hi Neil,
>>
>> Did my questions/comments above make sense? I am hoping we can
>> agree on some design soon.
>>
>> Christian and Mickaël,
>>
>> Could you please also share your thoughts on this?
>>
>> Current requirements from BPF side is straightforward: we just
>> need a mechanism to “walk up one level and hold reference”. So
>> most of the requirement comes from LandLock side.
>
> Have you thought about how to handle disconnected directories?
In the case of open-coded path iterator, the iterator will
return a special value for disconnected roots and disconnected
directories. Then the BPF program need to handle them based on
the policy.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH 00/19] smack: clean up xattr handling
From: Casey Schaufler @ 2025-07-26 17:41 UTC (permalink / raw)
To: Konstantin Andreev; +Cc: linux-security-module, Casey Schaufler
In-Reply-To: <cover.1753356770.git.andreev@swemel.ru>
On 7/24/2025 6:09 AM, Konstantin Andreev wrote:
> A set of minor bug fixes and optimizations in Smack xattr handling.
> Logically independent, but with the code dependencies.
Please break this into two (or more) patch sets. The patches regarding
restrictions on getting and setting the file type specific attributes
should be presented independently of the xattr "fixes".
There appears to be a misunderstanding regarding "valid" Smack labels.
A Smack label is a text string. The intention is that a label is "valid"
if the system is exposed to it. For example,
# echo Oatmeal > /proc/self/attr/smack/current
should introduce "Oatmeal" as a Smack label if is has never been used
before. After a reboot the system may find the label "Bacon" on a file,
and if the label isn't known it is imported. Similarly, if a CIPSO packet
includes a label that has not been seen in is added.
This policy is necessary in part because there is a valid use case for
a Smack label with no explicit access rules.
I tried out the combined set and encountered many unexpected failures.
>
> The patch set applies on top of:
> https://github.com/cschaufler/smack-next/commits/next
> commit 6ddd169d0288
>
> Konstantin Andreev (19):
> smack: fix bug: changing Smack xattrs requires cap_sys_admin
> smack: fix bug: changing Smack xattrs requires cap_mac_override
> smack: fix bug: setting label-containing xattrs silently ignores input garbage
> smack: stop polling other LSMs & VFS to getxattr() unsupported SMACK64IPIN/OUT
> smack: restrict getxattr() SMACK64TRANSMUTE to directories
> smack: fix bug: getxattr() returns invalid SMACK64EXEC/MMAP
> smack: deduplicate task label validation
> smack: smack_inode_setsecurity: prevent setting SMACK64EXEC/MMAP in other LSMs
> smack: smack_inode_setsecurity: prevent setting SMACK64IPIN/OUT in other LSMs
> smack: fix bug: smack_inode_setsecurity() imports alien xattrs as labels
> smack: fix bug: smack_inode_setsecurity() false EINVAL for alien xattrs
> smack: restrict setxattr() SMACK64IPIN/IPOUT to sockets
> smack: restrict setxattr() SMACK64EXEC/MMAP to regular files
> smack: return EOPNOTSUPP for setxattr() unsupported SMACK64(TRANSMUTE)
> smack: smack_inode_setsecurity(): skip checks for SMACK64TRANSMUTE
> smack: smack_inode_notifysecctx(): reject invalid labels
> smack: smack_inode_post_setxattr(): find label instead of import
> smack: smack_inode_setsecurity(): find label instead of import
> smack: deduplicate strcmp(name, XATTR_{,NAME_}SMACK*)
>
> Documentation/admin-guide/LSM/Smack.rst | 3 +-
> security/smack/smack.h | 2 +
> security/smack/smack_access.c | 22 +-
> security/smack/smack_lsm.c | 492 +++++++++++++++---------
> 4 files changed, 324 insertions(+), 195 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v4 0/4] stackleak: Support Clang stack depth tracking
From: Kees Cook @ 2025-07-26 21:47 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Will Deacon, Ard Biesheuvel, Catalin Marinas,
Jonathan Cameron, Gavin Shan, Russell King (Oracle), James Morse,
Oza Pawandeep, Anshuman Khandual, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Mike Rapoport, Vitaly Kuznetsov, Henrique de Moraes Holschuh,
Hans de Goede, Ilpo Järvinen, Rafael J. Wysocki, Len Brown,
Masami Hiramatsu, Michal Wilczynski, Juergen Gross,
Andy Shevchenko, Kirill A. Shutemov, Roger Pau Monne,
David Woodhouse, Usama Arif, Guilherme G. Piccoli, Thomas Huth,
Brian Gerst, Marco Elver, Andrey Konovalov, Andrey Ryabinin,
Hou Wenlong, Andrew Morton, Masahiro Yamada,
Peter Zijlstra (Intel), Luis Chamberlain, Sami Tolvanen,
Christophe Leroy, Nicolas Schier, Gustavo A. R. Silva,
Andy Lutomirski, Baoquan He, Alexander Graf, Changyuan Lyu,
Paul Moore, James Morris, Serge E. Hallyn, Nick Desaulniers,
Bill Wendling, Justin Stitt, Jan Beulich, Boqun Feng,
Viresh Kumar, Paul E. McKenney, Bibo Mao, linux-kernel,
linux-arm-kernel, x86, kvm, ibm-acpi-devel, platform-driver-x86,
linux-acpi, linux-trace-kernel, linux-efi, linux-mm, kasan-dev,
linux-kbuild, linux-hardening, kexec, linux-security-module, llvm
In-Reply-To: <20250726004313.GA3650901@ax162>
On Fri, Jul 25, 2025 at 05:43:13PM -0700, Nathan Chancellor wrote:
> ld.lld: error: undefined symbol: __sanitizer_cov_stack_depth
> >>> referenced by atags_to_fdt.c
Proposed fix:
https://lore.kernel.org/lkml/20250726212945.work.975-kees@kernel.org/
> kernel/kstack_erase.c:168:2: warning: function with attribute 'no_caller_saved_registers' should only call a function with attribute 'no_caller_saved_registers' or be compiled with '-mgeneral-regs-only' [-Wexcessive-regsave]
Proposed fix:
https://lore.kernel.org/lkml/20250726212615.work.800-kees@kernel.org/
> In file included from kernel/fork.c:96:
> include/linux/kstack_erase.h:29:37: error: passing 'const struct task_struct *' to parameter of type 'struct task_struct *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> 29 | return (unsigned long)end_of_stack(tsk) + sizeof(unsigned long);
> | ^~~
> include/linux/sched/task_stack.h:56:63: note: passing argument to parameter 'p' here
> 56 | static inline unsigned long *end_of_stack(struct task_struct *p)
> | ^
Proposed fix:
https://lore.kernel.org/lkml/20250726210641.work.114-kees@kernel.org/
Thanks for the reports! :)
-Kees
--
Kees Cook
^ permalink raw reply
* Re[2]: [PATCH 00/19] smack: clean up xattr handling
From: Konstantin Andreev @ 2025-07-27 14:32 UTC (permalink / raw)
To: Casey Schaufler; +Cc: linux-security-module
In-Reply-To: <f4b697c8-7851-4e46-ad33-bd0eed50af06@schaufler-ca.com>
Casey Schaufler, 26 Jul 2025:
> On 7/24/2025 6:09 AM, Konstantin Andreev wrote:
>> A set of minor bug fixes and optimizations in Smack xattr handling.
>> Logically independent, but with the code dependencies.
>
> Please break this into two (or more) patch sets. The patches regarding
> restrictions on getting and setting the file type specific attributes
> should be presented independently of the xattr "fixes".
Hi, Casey.
Each patch in the set is finite by itself,
addresses an independend issue, and I ought
to send them separately. There are several reasons
why I ended up to collect them into the set:
1) they are very small and have common subject
2) they have code dependencies
3) the feedback time for Smack patches may be high,
and sending patches one-by-one may be long.
4) to give you an overview of the offered changes
May it be suitable for you to consider this set
as independent patches, that require, however,
the specific order of applying?
With this approach you could stop considering the
sequence at the 1st unacceptable/wrong/failed patch.
Else, I can collect them into two (or more) patch sets.
> There appears to be a misunderstanding regarding "valid" Smack labels.
> A Smack label is a text string. The intention is that a label is "valid"
> if the system is exposed to it. For example,
>
> # echo Oatmeal > /proc/self/attr/smack/current
>
> should introduce "Oatmeal" as a Smack label if is has never been used
> before. After a reboot the system may find the label "Bacon" on a file,
> and if the label isn't known it is imported.
I think I understand this.
> Similarly, if a CIPSO packet
> includes a label that has not been seen in is added.
I have never seen this. The unseen CIPSO label is not added,
instead, the incoming packet is marked with `*' label.
Just rechecked, it's so. E.g., tcp/ipv4 connection,
listener side writes the audit record like:
| lsm=SMACK fn=smack_socket_sock_rcv_skb action=denied subject="*" object="foo" requested=w ...
when incoming CIPSO has [bar 250/2,3,7,10,11,16,18,19,20,23]
at the initiator, but unseen at the listener side.
The documentation does not define either way of processing
of the unseen Cipso labels, but current implementation looks
reasonable, as sheer import of network-provided labels
has no limits and opens the door for denial of service.
> This policy is necessary in part because there is a valid use case for
> a Smack label with no explicit access rules.
>
> I tried out the combined set and encountered many unexpected failures.
>
[... skip ...]
^ permalink raw reply
* Re: [PATCH v3 2/4] landlock: Fix handling of disconnected directories
From: Tingmao Wang @ 2025-07-28 0:13 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack, Jann Horn,
John Johansen
Cc: Al Viro, Ben Scarlato, Christian Brauner, Daniel Burgener,
Jeff Xu, NeilBrown, Paul Moore, Ryan Sullivan, Song Liu,
linux-fsdevel, linux-security-module, Abhinav Saxena
In-Reply-To: <20250723.vouso1Kievao@digikod.net>
On 7/23/25 22:01, Mickaël Salaün wrote:
> On Tue, Jul 22, 2025 at 07:04:02PM +0100, Tingmao Wang wrote:
>> On 7/19/25 11:42, Mickaël Salaün wrote:
>>> [...]
>>> @@ -784,12 +787,18 @@ static bool is_access_to_paths_allowed(
>>> if (WARN_ON_ONCE(!layer_masks_parent1))
>>> return false;
>>>
>>> - allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
>>> -
>>> if (unlikely(layer_masks_parent2)) {
>>> if (WARN_ON_ONCE(!dentry_child1))
>>> return false;
>>>
>>> + /*
>>> + * Creates a backup of the initial layer masks to be able to restore
>>> + * them if we find out that we were walking a disconnected directory,
>>> + * which would make the collected access rights inconsistent (cf.
>>> + * reset_to_mount_root).
>>> + */
>>
>> This comment is duplicate with the one below, is this intentional?
>>
>>> [...]
>>
>> On the other hand, I'm still a bit uncertain about the domain check
>> semantics. While it would not cause a rename to be allowed if it is
>> otherwise not allowed by any rules on or above the mountpoint, this gets a
>> bit weird if we have a situation where renames are allowed on the
>> mountpoint or everywhere, but not read/writes, however read/writes are
>> allowed directly on a file, but the dir containing that file gets
>> disconnected so the sandboxed application can't read or write to it.
>> (Maybe someone would set up such a policy where renames are allowed,
>> expecting Landlock to always prevent renames where additional permissions
>> would be exposed?)
>>
>> In the above situation, if the file is then moved to a connected
>> directory, it will become readable/writable again.
>
> We can generalize this issue to not only the end file but any component
> of the path: disconnected directories. In fact, the main issue is the
> potential inconsistency of access checks over time (e.g. between two
> renames). This could be exploited to bypass the security checks done
> for FS_REFER.
Hi Mickaël,
(replying to this one even though I've read the further replies, since I
don't have anything to address there)
I spent some time thinking about how this could be exploited and I can see
one concrete situation where disconnected directories can be taken
advantaged of, which is when an application has rename access across the
"source" of a bind mount, as well as some place outside of it, but only
has read/write access on certain subdirs of it, in which case it can use
this to move files under the bind mount that it does not have access to,
to a place where it has, by first making the target destination
disconnected, then connecting it back:
/ # mkdir bind_src bind_dst outside
/ # mount --bind bind_src bind_dst
/ # mkdir -p bind_src/d1
/ # echo secret > /bind_src/foo
/ # LL_FS_RO=/usr:/bin:/lib:/etc LL_FS_RW=/bind_src/d1:/outside LL_FS_CREATE_DELETE_REFER=/ /sandboxer bash
## (sandboxer patched with ability to add just create/delete/refer
rule on a path - maybe we can add this to the sandboxer but
call it something like LL_FS_DIR_RW?)
Executing the sandboxed command...
/ # cat /bind_src/foo /bind_dst/foo
cat: /bind_src/foo: Permission denied
cat: /bind_dst/foo: Permission denied
/ # cd /bind_dst/d1
/bind_dst/d1 # mv -v /bind_src/d1 /outside
renamed '/bind_src/d1' -> '/outside/d1'
/bind_dst/d1 # ls ..
ls: cannot access '..': No such file or directory
/bind_dst/d1 # mv -v /bind_dst/foo .
renamed '/bind_dst/foo' -> './foo'
/bind_dst/d1 # mv -v /outside/d1 /bind_src/d1
renamed '/outside/d1' -> '/bind_src/d1'
/bind_dst/d1 # cat foo
secret
(I think this is probably not an issue with the existing Landlock code, as
the application would need to have rules outside the bind mount in order
for Landlock to not see it when the directory is disconnected, but in that
case it already has access to the target file anyway due to hierarchy
inheritance)
The other way I can think of where this inconsistency could be taken
advantaged of is for regaining access to a file within a disconnected
directory that the application originally had access to (basically the
situation I wrote about earlier), but that is arguably a less worrisome
case and your option 1 would make access in this case allowed anyway. So
far I can't think of any other way an application could use this to gain
access it didn't have on the file before.
>
> I see two solutions:
>
> 1. *Always* walk down to the IS_ROOT directory, and then jump to the
> mount point. This makes it possible to have consistent access checks
> for renames and open/use. The first downside is that that would
> change the current behavior for bind mounts that could get more
> access rights (if the policy explicitly sets rights for the hidden
> directories). The second downside is that we'll do more walk.
>
> 2. Return -EACCES (or -ENOENT) for actions involving disconnected
> directories, or renames of disconnected opened files. This second
> solution is simpler and safer but completely disables the use of
> disconnected directories and the rename of disconnected files for
> sandboxed processes.
I think -ENOENT might be confusing if the disconnection is accidentally
triggered (e.g. due to a bug, race etc) (I guess given that ".." doesn't
work in disconnected dirs, nobody should be setting them up deliberately,
so maybe any situation where a disconnected directory exists is always a
bug, or at least a temporary race?), but on the other hand VFS already
returns -ENOENT for ".." when in disconnected directories so maybe it's
OK, as long as this is clarified in the doc, I guess.
Also by "rename" I assume we're implicitly including links as well (and
since one couldn't rename an opened file just from the fd anyway, any
rename would be for files underneath).
>
> It would be much better to be able to handle opened directories as
> (object) capabilities, but that is not currently possible because of the
> way paths are handled by the VFS and LSM hooks.
>
> Tingmao, Günther, Jann, what do you think?
I wonder if there is a third option where we do option 1, but only for
disconnected dirs, so basically for any disconnected directories Landlock
would allow rules either on the path from the target to its fs root, or on
the path from its mountpoint to the real root. In addition, domain check
for rename/links would be stricter, in that it would make sure there are
no rules granting more access on the destination than the source even if
those rules are "hidden" beneath the mountpoint, so that even if the
destination later become disconnected access is not widened.
While this does leaves inconsistency since now some "hidden" rules would
be applied only in the disconnected case, maybe this could still be a
better option since it prevents accidentally widening access checks in the
common case, especially for existing code (after all, currently Landlock
does not apply those "hidden" rules, and if an application can't move the
files into a disconnected directory, for example because write access are
not granted to any bind mounted places, or because it only has access to
the mount destination but not the source (achievable by putting the rule
above the mountpoint, and in that case it can't move a dir outside the
mount so it will always be connected), it will not be able to surface
them).
One could argue that a reasonable policy is not supposed to have rules on
those "hidden" directories in the first place, but in that case whether or
not we do this extra walk to fs root for the non-disconnected case makes
no difference.
The advantage of this option is also that we don't increase the
performance overhead for the non-disconnected case which is the vast
majority of cases (and this perf benefit might be significant since we're
likely not backporting any ref-less pathwalk). Also, while I'm not sure
whether this option or option 1 has any difference in terms of backporting
difficulty, I think option 1 is a more significant change compared to
this, and maybe in the future if we do get to have opened directory
capabilities, keeping the changes introduced by the current "workaround"
small would be better? This would probably also make it easier to adopt
Song's path iterator too.
If we do this, then we would probably have a warning in e.g.
Documentation/userspace-api/landlock.rst about the fact that Landlock will
do this walk to the fs root if the directory is disconnected (either
caused by the sandboxed application or by other means).
Just thoughts and suggestions tho, my opinions aren't very strong, and I
probably don't have a good grasp of how Landlock is currently used in
practice.
Best,
Tingmao
>
> It looks like AppArmor also denies access to disconnected path in some
> cases, but it tries to reconstruct the path for known internal
> filesystems, and it seems to specifically handle the case of chroot. I
> don't know when PATH_CONNECT_PATH is set though.
>
> John, could you please clarify how disconnected directories and files
> are handled by AppArmor?
>
>>
>> Here is an example test, using the layout1_bind fixture for flexibility
>> [...]
^ permalink raw reply
* Re: [syzbot] [apparmor?] linux-next test error: WARNING in apparmor_unix_stream_connect
From: Alexander Potapenko @ 2025-07-28 8:16 UTC (permalink / raw)
To: john.johansen
Cc: apparmor, jmorris, john, linux-kernel, linux-next,
linux-security-module, paul, serge, sfr, syzkaller-bugs
In-Reply-To: <687e09e3.a70a0220.693ce.00eb.GAE@google.com>
On Mon, Jul 21, 2025 at 11:35 AM syzbot
<syzbot+cd38ee04bcb3866b0c6d@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
John, do you have an idea what's going on?
This is pretty likely to be related to your "apparmor: make sure unix
socket labeling is correctly updated." patch.
^ permalink raw reply
* Re: [RFC PATCH v2 26/34] smack: move initcalls to the LSM framework
From: Roberto Sassu @ 2025-07-28 9:46 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Tetsuo Handa, Nicolas Bouchinet,
Xiu Jianfeng
In-Reply-To: <20250721232142.77224-62-paul@paul-moore.com>
On Mon, 2025-07-21 at 19:21 -0400, Paul Moore wrote:
> As the LSM framework only supports one LSM initcall callback for each
> initcall type, the init_smk_fs() and smack_nf_ip_init() functions were
> wrapped with a new function, smack_initcall() that is registered with
> the LSM framework.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/smack/smack.h | 7 +++++++
> security/smack/smack_lsm.c | 9 +++++++++
> security/smack/smack_netfilter.c | 4 +---
> security/smack/smackfs.c | 4 +---
> 4 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index bf6a6ed3946c..885a2f2929fd 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -275,6 +275,13 @@ struct smk_audit_info {
> #endif
> };
>
> +/*
> + * Initialization
> + */
> +int init_smk_fs(void);
> +int smack_nf_ip_init(void);
I made the following changes (due to not having
CONFIG_SECURITY_SMACK_NETFILTER):
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 885a2f2929fd..4401cee2bbb7 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -279,9 +279,17 @@ struct smk_audit_info {
* Initialization
*/
int init_smk_fs(void);
-int smack_nf_ip_init(void);
int smack_initcall(void);
+#ifdef CONFIG_SECURITY_SMACK_NETFILTER
+int smack_nf_ip_init(void);
+#else
+static inline int smack_nf_ip_init(void)
+{
+ return 0;
+}
+#endif
+
Roberto
> +int smack_initcall(void);
> +
> /*
> * These functions are in smack_access.c
> */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index e09490c75f59..f14d536c516b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -5270,6 +5270,14 @@ static __init int smack_init(void)
> return 0;
> }
>
> +int __init smack_initcall(void)
> +{
> + int rc_fs = init_smk_fs();
> + int rc_nf = smack_nf_ip_init();
> +
> + return rc_fs ? rc_fs : rc_nf;
> +}
> +
> /*
> * Smack requires early initialization in order to label
> * all processes and objects when they are created.
> @@ -5279,4 +5287,5 @@ DEFINE_LSM(smack) = {
> .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
> .blobs = &smack_blob_sizes,
> .init = smack_init,
> + .initcall_device = smack_initcall,
> };
> diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
> index 8fd747b3653a..17ba578b1308 100644
> --- a/security/smack/smack_netfilter.c
> +++ b/security/smack/smack_netfilter.c
> @@ -68,7 +68,7 @@ static struct pernet_operations smack_net_ops = {
> .exit = smack_nf_unregister,
> };
>
> -static int __init smack_nf_ip_init(void)
> +int __init smack_nf_ip_init(void)
> {
> if (smack_enabled == 0)
> return 0;
> @@ -76,5 +76,3 @@ static int __init smack_nf_ip_init(void)
> printk(KERN_DEBUG "Smack: Registering netfilter hooks\n");
> return register_pernet_subsys(&smack_net_ops);
> }
> -
> -__initcall(smack_nf_ip_init);
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index b1e5e62f5cbd..405ace6db109 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -2978,7 +2978,7 @@ static struct vfsmount *smackfs_mount;
> * Returns true if we were not chosen on boot or if
> * we were chosen and filesystem registration succeeded.
> */
> -static int __init init_smk_fs(void)
> +int __init init_smk_fs(void)
> {
> int err;
> int rc;
> @@ -3021,5 +3021,3 @@ static int __init init_smk_fs(void)
>
> return err;
> }
> -
> -__initcall(init_smk_fs);
^ permalink raw reply related
* Re: [RFC PATCH v2 31/34] ima,evm: move initcalls to the LSM framework
From: Nicolas Bouchinet @ 2025-07-28 9:46 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Xiu Jianfeng
In-Reply-To: <20250721232142.77224-67-paul@paul-moore.com>
Hi Paul,
With `CONFIG_INTEGRITY=y` but not `CONFIG_IMA=y` or `CONFIG_EVM=y` it
does not compile :
```
ld: vmlinux.o: in function `integrity_late_init':
security/integrity/initcalls.c:32:(.init.text+0x47f85): undefined reference to `init_ima'
ld: security/integrity/initcalls.c:36:(.init.text+0x47f96): undefined reference to `init_evm'
make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux.unstripped] Error 1
make[1]: *** [Makefile:1236: vmlinux] Error 2
make: *** [Makefile:248: __sub-make] Error 2
```
> security/integrity/Makefile | 2 +-
> security/integrity/evm/evm_main.c | 6 ++---
> security/integrity/iint.c | 4 +--
> security/integrity/ima/ima_main.c | 6 ++---
> security/integrity/initcalls.c | 41 +++++++++++++++++++++++++++++++
> security/integrity/initcalls.h | 13 ++++++++++
> 6 files changed, 63 insertions(+), 9 deletions(-)
> create mode 100644 security/integrity/initcalls.c
> create mode 100644 security/integrity/initcalls.h
>
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 92b63039c654..6ea330ea88b1 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -5,7 +5,7 @@
>
> obj-$(CONFIG_INTEGRITY) += integrity.o
>
> -integrity-y := iint.o
> +integrity-y := iint.o initcalls.o
> integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
> integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
> integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
---
> diff --git a/security/integrity/initcalls.h b/security/integrity/initcalls.h
> new file mode 100644
> index 000000000000..5511c62f8166
> --- /dev/null
> +++ b/security/integrity/initcalls.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef PLATFORM_CERTS_INITCALLS_H
> +#define PLATFORM_CERTS_INITCALLS_H
> +
> +int integrity_fs_init(void);
> +
> +int init_ima(void);
> +int init_evm(void);
> +
> +int integrity_late_init(void);
> +
> +#endif
> --
> 2.50.1
>
Nicolas
^ 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