linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] lsm: introduce lsm_manage_policy() syscall
@ 2025-05-06 14:32 Maxime Bélair
  2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
                   ` (2 more replies)
  0 siblings, 3 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-06 14:32 UTC (permalink / raw)
  To: linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel, Maxime Bélair

This patchset introduces a new syscall, lsm_manage_policy(), and the
associated Linux Security Module hook security_lsm_manage_policy(),
providing a unified interface for loading and managing LSM policies.
This syscall complements the existing per‑LSM pseudo‑filesystem mechanism
and works even when those filesystems are not mounted or available.

With this new syscall, administrators may lock down access to the
pseudo‑filesystem yet still manage LSM policies. A single, tightly scoped
entry point then replaces the many file operations exposed by those
filesystems, significantly reducing the attack surface. This is
particularly useful in containers or processes already confined by
Landlock, where these pseudo‑filesystems are typically unavailable.

Because it provides a logical and unified interface, lsm_manage_policy()
is simpler to use than several heterogeneous pseudo‑filesystems and
avoids edge cases such as partially loaded policies. It also eliminates
VFS overhead, yielding performance gains notably when many policies are
loaded, for instance at boot time.

This initial implementation is intentionally minimal to limit the scope
of changes. Currently, only policy loading is supported, and only
AppArmor registers this LSM hook. However, any LSM can adopt this
interface, and future patches could extend this syscall to support more
operations, such as replacing, removing, or querying loaded policies.

Landlock already provides three Landlock‑specific syscalls (e.g.
landlock_add_rule()) to restrict ambient rights for sets of processes
without touching any pseudo-filesystem. lsm_manage_policy() generalizes
that approach to the entire LSM layer, so any module can expose its
policy operations through one uniform interface and reap the advantages
outlined above.

This patchset is available at [1] and a minimal user space example
showing how to use this syscall with AppArmor is at [2].

[1] https://github.com/emixam16/linux/tree/lsm_syscall
[2] https://gitlab.com/emixam16/apparmor/tree/lsm_syscall

Maxime Bélair (3):
  Wire up the lsm_manage_policy syscall
  lsm: introduce security_lsm_manage_policy hook
  AppArmor: add support for lsm_manage_policy

 arch/alpha/kernel/syscalls/syscall.tbl        |  1 +
 arch/arm/tools/syscall.tbl                    |  1 +
 arch/x86/entry/syscalls/syscall_32.tbl        |  1 +
 arch/x86/entry/syscalls/syscall_64.tbl        |  1 +
 include/linux/lsm_hook_defs.h                 |  2 ++
 include/linux/security.h                      |  7 +++++++
 include/linux/syscalls.h                      |  4 ++++
 include/uapi/asm-generic/unistd.h             |  4 +++-
 include/uapi/linux/lsm.h                      |  8 +++++++
 kernel/sys_ni.c                               |  1 +
 security/apparmor/apparmorfs.c                | 19 +++++++++++++++++
 security/apparmor/include/apparmorfs.h        |  3 +++
 security/apparmor/lsm.c                       | 16 ++++++++++++++
 security/lsm_syscalls.c                       | 11 ++++++++++
 security/security.c                           | 21 +++++++++++++++++++
 tools/include/uapi/asm-generic/unistd.h       |  4 +++-
 .../arch/x86/entry/syscalls/syscall_64.tbl    |  1 +
 17 files changed, 103 insertions(+), 2 deletions(-)


base-commit: 9c32cda43eb78f78c73aee4aa344b777714e259b
-- 
2.48.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-06 14:32 [PATCH 0/3] lsm: introduce lsm_manage_policy() syscall Maxime Bélair
@ 2025-05-06 14:32 ` Maxime Bélair
  2025-05-07  6:26   ` Song Liu
  2025-05-07 13:58   ` kernel test robot
  2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
  2025-05-06 14:32 ` [PATCH 3/3] AppArmor: add support for lsm_manage_policy Maxime Bélair
  2 siblings, 2 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-06 14:32 UTC (permalink / raw)
  To: linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel, Maxime Bélair

Add support for the new lsm_manage_policy syscall, providing a unified
API for loading and modifying LSM policies without requiring the LSM’s
pseudo-filesystem.

Benefits:
  - Works even if the LSM pseudo-filesystem isn’t mounted or available
    (e.g. in containers)
  - Offers a logical and unified interface rather than multiple
    heterogeneous pseudo-filesystems.
  - Avoids overhead of other kernel interfaces for better efficiency

Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
---
 arch/alpha/kernel/syscalls/syscall.tbl            | 1 +
 arch/arm/tools/syscall.tbl                        | 1 +
 arch/x86/entry/syscalls/syscall_32.tbl            | 1 +
 arch/x86/entry/syscalls/syscall_64.tbl            | 1 +
 include/linux/syscalls.h                          | 4 ++++
 include/uapi/asm-generic/unistd.h                 | 4 +++-
 kernel/sys_ni.c                                   | 1 +
 security/lsm_syscalls.c                           | 6 ++++++
 tools/include/uapi/asm-generic/unistd.h           | 4 +++-
 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl | 1 +
 10 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 2dd6340de6b4..dfe6cd43c584 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -507,3 +507,4 @@
 575	common	listxattrat			sys_listxattrat
 576	common	removexattrat			sys_removexattrat
 577	common	open_tree_attr			sys_open_tree_attr
+578	common	lsm_manage_policy		sys_lsm_manage_policy
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 27c1d5ebcd91..60abcb3a8a1b 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -482,3 +482,4 @@
 465	common	listxattrat			sys_listxattrat
 466	common	removexattrat			sys_removexattrat
 467	common	open_tree_attr			sys_open_tree_attr
+468	common	lsm_manage_policy		sys_lsm_manage_policy
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index ac007ea00979..bb91a929757a 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -473,3 +473,4 @@
 465	i386	listxattrat		sys_listxattrat
 466	i386	removexattrat		sys_removexattrat
 467	i386	open_tree_attr		sys_open_tree_attr
+468	i386	lsm_manage_policy	sys_lsm_manage_policy
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index cfb5ca41e30d..83819d4a5c8a 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -391,6 +391,7 @@
 465	common	listxattrat		sys_listxattrat
 466	common	removexattrat		sys_removexattrat
 467	common	open_tree_attr		sys_open_tree_attr
+468	common	lsm_manage_policy	sys_lsm_manage_policy
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e5603cc91963..f52a0678b1d0 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -989,6 +989,10 @@ asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx __user *
 				      u32 size, u32 flags);
 asmlinkage long sys_lsm_list_modules(u64 __user *ids, u32 __user *size, u32 flags);
 
+asmlinkage long sys_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
+		u32 __user *size, u32 flags);
+
+
 /*
  * Architecture-specific system calls
  */
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 2892a45023af..b94369baded8 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -851,9 +851,11 @@ __SYSCALL(__NR_listxattrat, sys_listxattrat)
 __SYSCALL(__NR_removexattrat, sys_removexattrat)
 #define __NR_open_tree_attr 467
 __SYSCALL(__NR_open_tree_attr, sys_open_tree_attr)
+#define __NR_lsm_manage_policy 468
+__SYSCALL(__NR_lsm_manage_policy, lsm_manage_policy)
 
 #undef __NR_syscalls
-#define __NR_syscalls 468
+#define __NR_syscalls 469
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index c00a86931f8c..e556b07d8716 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -172,6 +172,7 @@ COND_SYSCALL_COMPAT(fadvise64_64);
 COND_SYSCALL(lsm_get_self_attr);
 COND_SYSCALL(lsm_set_self_attr);
 COND_SYSCALL(lsm_list_modules);
+COND_SYSCALL(lsm_manage_policy);
 
 /* CONFIG_MMU only */
 COND_SYSCALL(swapon);
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index 8440948a690c..dcaad8818679 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -118,3 +118,9 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
 
 	return lsm_active_cnt;
 }
+
+SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
+		__user *, size, u32, flags)
+{
+	return 0;
+}
diff --git a/tools/include/uapi/asm-generic/unistd.h b/tools/include/uapi/asm-generic/unistd.h
index 2892a45023af..b94369baded8 100644
--- a/tools/include/uapi/asm-generic/unistd.h
+++ b/tools/include/uapi/asm-generic/unistd.h
@@ -851,9 +851,11 @@ __SYSCALL(__NR_listxattrat, sys_listxattrat)
 __SYSCALL(__NR_removexattrat, sys_removexattrat)
 #define __NR_open_tree_attr 467
 __SYSCALL(__NR_open_tree_attr, sys_open_tree_attr)
+#define __NR_lsm_manage_policy 468
+__SYSCALL(__NR_lsm_manage_policy, lsm_manage_policy)
 
 #undef __NR_syscalls
-#define __NR_syscalls 468
+#define __NR_syscalls 469
 
 /*
  * 32 bit systems traditionally used different
diff --git a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
index cfb5ca41e30d..83819d4a5c8a 100644
--- a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
@@ -391,6 +391,7 @@
 465	common	listxattrat		sys_listxattrat
 466	common	removexattrat		sys_removexattrat
 467	common	open_tree_attr		sys_open_tree_attr
+468	common	lsm_manage_policy	sys_lsm_manage_policy
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-06 14:32 [PATCH 0/3] lsm: introduce lsm_manage_policy() syscall Maxime Bélair
  2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
@ 2025-05-06 14:32 ` Maxime Bélair
  2025-05-07  6:19   ` Song Liu
                     ` (2 more replies)
  2025-05-06 14:32 ` [PATCH 3/3] AppArmor: add support for lsm_manage_policy Maxime Bélair
  2 siblings, 3 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-06 14:32 UTC (permalink / raw)
  To: linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel, Maxime Bélair

Define a new LSM hook security_lsm_manage_policy and wire it into the
lsm_manage_policy() syscall so that LSMs can register a unified interface
for policy management. This initial, minimal implementation only supports
the LSM_POLICY_LOAD operation to limit changes.

Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
---
 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/security.h      |  7 +++++++
 include/uapi/linux/lsm.h      |  8 ++++++++
 security/lsm_syscalls.c       |  7 ++++++-
 security/security.c           | 21 +++++++++++++++++++++
 5 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index bf3bbac4e02a..04b6e34d5111 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -464,3 +464,5 @@ LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
 LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
 LSM_HOOK(int, 0, bdev_setintegrity, struct block_device *bdev,
 	 enum lsm_integrity_type type, const void *value, size_t size)
+LSM_HOOK(int, 0, lsm_manage_policy, u32 lsm_id, u32 op, void __user *buf,
+	 size_t size, u32 flags)
diff --git a/include/linux/security.h b/include/linux/security.h
index cc9b54d95d22..dab547ee691c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -581,6 +581,8 @@ void security_bdev_free(struct block_device *bdev);
 int security_bdev_setintegrity(struct block_device *bdev,
 			       enum lsm_integrity_type type, const void *value,
 			       size_t size);
+int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
+			       size_t size, u32 flags);
 #else /* CONFIG_SECURITY */
 
 /**
@@ -1602,6 +1604,11 @@ static inline int security_bdev_setintegrity(struct block_device *bdev,
 {
 	return 0;
 }
+static int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
+				      size_t size, u32 flags)
+
+	return -EOPNOTSUPP;
+}
 
 #endif	/* CONFIG_SECURITY */
 
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
index 938593dfd5da..7335f9723114 100644
--- a/include/uapi/linux/lsm.h
+++ b/include/uapi/linux/lsm.h
@@ -90,4 +90,12 @@ struct lsm_ctx {
  */
 #define LSM_FLAG_SINGLE	0x0001
 
+/*
+ * LSM_POLICY_XXX definition identifies operation to manage lsm
+ * policies
+ */
+
+#define LSM_POLICY_UNDEF	0
+#define LSM_POLICY_LOAD		100
+
 #endif /* _UAPI_LINUX_LSM_H */
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index dcaad8818679..b39e6635a7d5 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
 SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
 		__user *, size, u32, flags)
 {
-	return 0;
+	size_t usize;
+
+	if (get_user(usize, size))
+		return -EFAULT;
+
+	return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
 }
diff --git a/security/security.c b/security/security.c
index fb57e8fddd91..256104e338b1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5883,6 +5883,27 @@ int security_bdev_setintegrity(struct block_device *bdev,
 }
 EXPORT_SYMBOL(security_bdev_setintegrity);
 
+/**
+ * security_lsm_manage_policy() - Manage the policies of LSMs
+ * @lsm_id: id of the lsm to target
+ * @op: Operation to perform (one of the LSM_POLICY_XXX values)
+ * @buf:  userspace pointer to policy data
+ * @size: size of @buf
+ * @flags: lsm policy management flags
+ *
+ * Manage the policies of a LSM. This notably allows to update them even when
+ * the lsmfs is unavailable is restricted. Currently, only LSM_POLICY_LOAD is
+ * supported.
+ *
+ * Return: Returns 0 on success, error on failure.
+ */
+int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
+			       size_t size, u32 flags)
+{
+	return call_int_hook(lsm_manage_policy, lsm_id, op, buf, size, flags);
+}
+EXPORT_SYMBOL(security_lsm_manage_policy);
+
 #ifdef CONFIG_PERF_EVENTS
 /**
  * security_perf_event_open() - Check if a perf event open is allowed
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 3/3] AppArmor: add support for lsm_manage_policy
  2025-05-06 14:32 [PATCH 0/3] lsm: introduce lsm_manage_policy() syscall Maxime Bélair
  2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
  2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
@ 2025-05-06 14:32 ` Maxime Bélair
  2 siblings, 0 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-06 14:32 UTC (permalink / raw)
  To: linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel, Maxime Bélair

Enable users to manage AppArmor policies through the new hook
lsm_manage_policy. Currently, policies can be added but not replaced
using this new mechanism, ensuring that this interface can only further
confine the system.

Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
---
 security/apparmor/apparmorfs.c         | 19 +++++++++++++++++++
 security/apparmor/include/apparmorfs.h |  3 +++
 security/apparmor/lsm.c                | 16 ++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 6039afae4bfc..9abb17e8fdd0 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -439,6 +439,25 @@ static ssize_t policy_update(u32 mask, const char __user *buf, size_t size,
 	return error;
 }
 
+/**
+ * aa_profile_load_current_ns - load a profile into the current namespace
+ * @buf buffer containing the user-provided policy
+ * @size size of @buf
+ * @ppos position pointer in the file
+ *
+ * Returns: 0 on success, negative value on error
+ */
+ssize_t aa_profile_load_current_ns(const void __user *buf, size_t size,
+				   loff_t *ppos)
+{
+	struct aa_ns *ns = aa_get_current_ns();
+	int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, ppos, ns);
+
+	aa_put_ns(ns);
+
+	return error >= 0 ? 0 : error;
+}
+
 /* .load file hook fn to load policy */
 static ssize_t profile_load(struct file *f, const char __user *buf, size_t size,
 			    loff_t *pos)
diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h
index 1e94904f68d9..ba2384e3fb93 100644
--- a/security/apparmor/include/apparmorfs.h
+++ b/security/apparmor/include/apparmorfs.h
@@ -112,6 +112,9 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent);
 void __aafs_ns_rmdir(struct aa_ns *ns);
 int __aafs_ns_mkdir(struct aa_ns *ns, struct dentry *parent, const char *name,
 		     struct dentry *dent);
+ssize_t aa_profile_load_current_ns(const void __user *buf, size_t size,
+				   loff_t *ppos);
+
 
 struct aa_loaddata;
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 9b6c2f157f83..21f3c4db0e4e 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1275,6 +1275,20 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
 	return aa_sock_perm(OP_SHUTDOWN, AA_MAY_SHUTDOWN, sock);
 }
 
+static int apparmor_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
+				      size_t size, u32 flags)
+{
+	loff_t pos = 0; // Partial writing is not currently supported
+
+	if (lsm_id != LSM_ID_APPARMOR)
+		return 0;
+
+	if (op != LSM_POLICY_LOAD || flags)
+		return -EOPNOTSUPP;
+
+	return aa_profile_load_current_ns(buf, size, &pos);
+}
+
 #ifdef CONFIG_NETWORK_SECMARK
 /**
  * apparmor_socket_sock_rcv_skb - check perms before associating skb to sk
@@ -1483,6 +1497,8 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt),
 	LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt),
 	LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown),
+
+	LSM_HOOK_INIT(lsm_manage_policy, apparmor_lsm_manage_policy),
 #ifdef CONFIG_NETWORK_SECMARK
 	LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb),
 #endif
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
@ 2025-05-07  6:19   ` Song Liu
  2025-05-07 15:37     ` Maxime Bélair
  2025-05-07 10:40   ` Tetsuo Handa
  2025-05-07 12:04   ` kernel test robot
  2 siblings, 1 reply; 36+ messages in thread
From: Song Liu @ 2025-05-07  6:19 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel

On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
<maxime.belair@canonical.com> wrote:
>
> Define a new LSM hook security_lsm_manage_policy and wire it into the
> lsm_manage_policy() syscall so that LSMs can register a unified interface
> for policy management. This initial, minimal implementation only supports
> the LSM_POLICY_LOAD operation to limit changes.
>
> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
[...]
> diff --git a/security/security.c b/security/security.c
> index fb57e8fddd91..256104e338b1 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5883,6 +5883,27 @@ int security_bdev_setintegrity(struct block_device *bdev,
>  }
>  EXPORT_SYMBOL(security_bdev_setintegrity);
>
> +/**
> + * security_lsm_manage_policy() - Manage the policies of LSMs
> + * @lsm_id: id of the lsm to target
> + * @op: Operation to perform (one of the LSM_POLICY_XXX values)
> + * @buf:  userspace pointer to policy data
> + * @size: size of @buf
> + * @flags: lsm policy management flags
> + *
> + * Manage the policies of a LSM. This notably allows to update them even when
> + * the lsmfs is unavailable is restricted. Currently, only LSM_POLICY_LOAD is
> + * supported.
> + *
> + * Return: Returns 0 on success, error on failure.
> + */
> +int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
> +                              size_t size, u32 flags)
> +{
> +       return call_int_hook(lsm_manage_policy, lsm_id, op, buf, size, flags);

If the LSM doesn't implement this hook, sys_lsm_manage_policy will return 0
for any inputs, right? This is gonna be so confusing for users.

Thanks,
Song

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
@ 2025-05-07  6:26   ` Song Liu
  2025-05-07 15:37     ` Maxime Bélair
  2025-05-08  7:12     ` John Johansen
  2025-05-07 13:58   ` kernel test robot
  1 sibling, 2 replies; 36+ messages in thread
From: Song Liu @ 2025-05-07  6:26 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel

On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
<maxime.belair@canonical.com> wrote:
>
> Add support for the new lsm_manage_policy syscall, providing a unified
> API for loading and modifying LSM policies without requiring the LSM’s
> pseudo-filesystem.
>
> Benefits:
>   - Works even if the LSM pseudo-filesystem isn’t mounted or available
>     (e.g. in containers)
>   - Offers a logical and unified interface rather than multiple
>     heterogeneous pseudo-filesystems.

These two do not feel like real benefits:
- Not working in containers is often not an issue, but a feature.
- One syscall cannot fit all use cases well...

>   - Avoids overhead of other kernel interfaces for better efficiency

.. and it is is probably less efficient, because everything need to
fit in the same API.

Overall, this set doesn't feel like a good change to me.

Thanks,
Song

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
  2025-05-07  6:19   ` Song Liu
@ 2025-05-07 10:40   ` Tetsuo Handa
  2025-05-07 15:37     ` Maxime Bélair
                       ` (2 more replies)
  2025-05-07 12:04   ` kernel test robot
  2 siblings, 3 replies; 36+ messages in thread
From: Tetsuo Handa @ 2025-05-07 10:40 UTC (permalink / raw)
  To: Maxime Bélair, linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, linux-api, apparmor,
	linux-kernel

On 2025/05/06 23:32, Maxime Bélair wrote:
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> index dcaad8818679..b39e6635a7d5 100644
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>  SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
>  		__user *, size, u32, flags)
>  {
> -	return 0;
> +	size_t usize;
> +
> +	if (get_user(usize, size))
> +		return -EFAULT;
> +
> +	return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
>  }

syzbot will report user-controlled unbounded huge size memory allocation attempt. ;-)

This interface might be fine for AppArmor, but TOMOYO won't use this interface because
TOMOYO's policy is line-oriented ASCII text data where the destination is switched via
pseudo‑filesystem's filename; use of filename helps restricting which type of policy
can be manipulated by which process.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
  2025-05-07  6:19   ` Song Liu
  2025-05-07 10:40   ` Tetsuo Handa
@ 2025-05-07 12:04   ` kernel test robot
  2 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-05-07 12:04 UTC (permalink / raw)
  To: Maxime Bélair, linux-security-module
  Cc: oe-kbuild-all, john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel, Maxime Bélair

Hi Maxime,

kernel test robot noticed the following build errors:

[auto build test ERROR on 9c32cda43eb78f78c73aee4aa344b777714e259b]

url:    https://github.com/intel-lab-lkp/linux/commits/Maxime-B-lair/Wire-up-the-lsm_manage_policy-syscall/20250506-224212
base:   9c32cda43eb78f78c73aee4aa344b777714e259b
patch link:    https://lore.kernel.org/r/20250506143254.718647-3-maxime.belair%40canonical.com
patch subject: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20250507/202505071924.MeIKUbEX-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250507/202505071924.MeIKUbEX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505071924.MeIKUbEX-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   include/linux/perf_event.h:2034:1: note: in expansion of macro 'DECLARE_STATIC_CALL'
    2034 | DECLARE_STATIC_CALL(perf_snapshot_branch_stack, perf_snapshot_branch_stack_t);
         | ^~~~~~~~~~~~~~~~~~~
   include/linux/static_call_types.h:15:41: error: storage class specified for parameter '__SCT__perf_snapshot_branch_stack'
      15 | #define STATIC_CALL_TRAMP_PREFIX        __SCT__
         |                                         ^~~~~~~
   include/linux/compiler_types.h:83:23: note: in definition of macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   include/linux/static_call_types.h:18:41: note: in expansion of macro '__PASTE'
      18 | #define STATIC_CALL_TRAMP(name)         __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                         ^~~~~~~
   include/linux/static_call_types.h:18:49: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
      18 | #define STATIC_CALL_TRAMP(name)         __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/static_call_types.h:39:29: note: in expansion of macro 'STATIC_CALL_TRAMP'
      39 |         extern typeof(func) STATIC_CALL_TRAMP(name);
         |                             ^~~~~~~~~~~~~~~~~
   include/linux/perf_event.h:2034:1: note: in expansion of macro 'DECLARE_STATIC_CALL'
    2034 | DECLARE_STATIC_CALL(perf_snapshot_branch_stack, perf_snapshot_branch_stack_t);
         | ^~~~~~~~~~~~~~~~~~~
   include/linux/perf_event.h:2034:78: error: expected declaration specifiers before ';' token
    2034 | DECLARE_STATIC_CALL(perf_snapshot_branch_stack, perf_snapshot_branch_stack_t);
         |                                                                              ^
   include/linux/perf_event.h:2038:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
    2038 | {
         | ^
   In file included from arch/riscv/include/asm/kvm_vcpu_pmu.h:14:
   arch/riscv/include/asm/sbi.h:15:1: warning: empty declaration
      15 | enum sbi_ext_id {
         | ^~~~
   arch/riscv/include/asm/sbi.h:48:1: warning: empty declaration
      48 | enum sbi_ext_base_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:58:1: warning: empty declaration
      58 | enum sbi_ext_time_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:62:1: warning: empty declaration
      62 | enum sbi_ext_ipi_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:66:1: warning: empty declaration
      66 | enum sbi_ext_rfence_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:76:1: warning: empty declaration
      76 | enum sbi_ext_hsm_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:83:1: warning: empty declaration
      83 | enum sbi_hsm_hart_state {
         | ^~~~
   arch/riscv/include/asm/sbi.h:106:1: warning: empty declaration
     106 | enum sbi_ext_srst_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:110:1: warning: empty declaration
     110 | enum sbi_srst_reset_type {
         | ^~~~
   arch/riscv/include/asm/sbi.h:116:1: warning: empty declaration
     116 | enum sbi_srst_reset_reason {
         | ^~~~
   arch/riscv/include/asm/sbi.h:121:1: warning: empty declaration
     121 | enum sbi_ext_susp_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:125:1: warning: empty declaration
     125 | enum sbi_ext_susp_sleep_type {
         | ^~~~
   arch/riscv/include/asm/sbi.h:129:1: warning: empty declaration
     129 | enum sbi_ext_pmu_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:140:1: warning: empty declaration
     140 | union sbi_pmu_ctr_info {
         | ^~~~~
   arch/riscv/include/asm/sbi.h:155:1: warning: empty declaration
     155 | struct riscv_pmu_snapshot_data {
         | ^~~~~~
   arch/riscv/include/asm/sbi.h:167:1: warning: empty declaration
     167 | enum sbi_pmu_hw_generic_events_t {
         | ^~~~
   arch/riscv/include/asm/sbi.h:188:1: warning: empty declaration
     188 | enum sbi_pmu_fw_generic_events_t {
         | ^~~~
   arch/riscv/include/asm/sbi.h:217:1: warning: empty declaration
     217 | enum sbi_pmu_event_type {
         | ^~~~
   arch/riscv/include/asm/sbi.h:225:1: warning: empty declaration
     225 | enum sbi_pmu_ctr_type {
         | ^~~~
   arch/riscv/include/asm/sbi.h:265:1: warning: empty declaration
     265 | enum sbi_ext_dbcn_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:272:1: warning: empty declaration
     272 | enum sbi_ext_sta_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:276:1: warning: empty declaration
     276 | struct sbi_sta_struct {
         | ^~~~~~
   arch/riscv/include/asm/sbi.h:286:1: warning: empty declaration
     286 | enum sbi_ext_nacl_fid {
         | ^~~~
   arch/riscv/include/asm/sbi.h:294:1: warning: empty declaration
     294 | enum sbi_ext_nacl_feature {
         | ^~~~
>> arch/riscv/include/asm/sbi.h:423:22: error: storage class specified for parameter 'sbi_spec_version'
     423 | extern unsigned long sbi_spec_version;
         |                      ^~~~~~~~~~~~~~~~
   arch/riscv/include/asm/sbi.h:424:1: warning: empty declaration
     424 | struct sbiret {
         | ^~~~~~
>> arch/riscv/include/asm/sbi.h:442:48: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     442 | static inline void sbi_console_putchar(int ch) { }
         |                                                ^
   arch/riscv/include/asm/sbi.h:443:45: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     443 | static inline int sbi_console_getchar(void) { return -ENOENT; }
         |                                             ^
   arch/riscv/include/asm/sbi.h:475:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     475 | {
         | ^
   arch/riscv/include/asm/sbi.h:481:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     481 | {
         | ^
   arch/riscv/include/asm/sbi.h:488:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     488 | {
         | ^
   arch/riscv/include/asm/sbi.h:495:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     495 | {
         | ^
   arch/riscv/include/asm/sbi.h:501:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     501 | {
         | ^
>> arch/riscv/include/asm/sbi.h:518:13: error: storage class specified for parameter 'sbi_debug_console_available'
     518 | extern bool sbi_debug_console_available;
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/sbi.h:539:51: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     539 | static inline bool riscv_use_sbi_for_rfence(void) { return false; }
         |                                                   ^
   arch/riscv/include/asm/sbi.h:540:39: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     540 | static inline void sbi_ipi_init(void) { }
         |                                       ^
   arch/riscv/include/asm/kvm_vcpu_pmu.h:105:1: warning: empty declaration
     105 | struct kvm_pmu {
         | ^~~~~~
>> arch/riscv/include/asm/kvm_vcpu_pmu.h:111:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     111 | {
         | ^
   arch/riscv/include/asm/kvm_vcpu_pmu.h:123:67: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     123 | static inline void kvm_riscv_vcpu_pmu_init(struct kvm_vcpu *vcpu) {}
         |                                                                   ^
   arch/riscv/include/asm/kvm_vcpu_pmu.h:125:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     125 | {
         | ^
   arch/riscv/include/asm/kvm_vcpu_pmu.h:129:69: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     129 | static inline void kvm_riscv_vcpu_pmu_deinit(struct kvm_vcpu *vcpu) {}
         |                                                                     ^
   arch/riscv/include/asm/kvm_vcpu_pmu.h:130:68: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     130 | static inline void kvm_riscv_vcpu_pmu_reset(struct kvm_vcpu *vcpu) {}
         |                                                                    ^
   arch/riscv/include/asm/kvm_host.h:57:1: warning: empty declaration
      57 | enum kvm_riscv_hfence_type {
         | ^~~~
   arch/riscv/include/asm/kvm_host.h:65:1: warning: empty declaration
      65 | struct kvm_riscv_hfence {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:75:1: warning: empty declaration
      75 | struct kvm_vm_stat {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:79:1: warning: empty declaration
      79 | struct kvm_vcpu_stat {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:97:1: warning: empty declaration
      97 | struct kvm_arch_memory_slot {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:100:1: warning: empty declaration
     100 | struct kvm_vmid {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:109:1: warning: empty declaration
     109 | struct kvm_arch {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:124:1: warning: empty declaration
     124 | struct kvm_cpu_trap {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:132:1: warning: empty declaration
     132 | struct kvm_cpu_context {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:172:1: warning: empty declaration
     172 | struct kvm_vcpu_csr {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:186:1: warning: empty declaration
     186 | struct kvm_vcpu_config {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:192:1: warning: empty declaration
     192 | struct kvm_vcpu_smstateen_csr {
         | ^~~~~~
   arch/riscv/include/asm/kvm_host.h:196:1: warning: empty declaration
     196 | struct kvm_vcpu_arch {
         | ^~~~~~
>> arch/riscv/include/asm/kvm_host.h:300:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
     300 | {
         | ^
>> arch/riscv/include/asm/kvm_host.h:365:13: error: section attribute not allowed for 'kvm_riscv_gstage_mode_detect'
     365 | void __init kvm_riscv_gstage_mode_detect(void);
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/kvm_host.h:365:1: warning: 'cold' attribute ignored [-Wattributes]
     365 | void __init kvm_riscv_gstage_mode_detect(void);
         | ^~~~
>> arch/riscv/include/asm/kvm_host.h:366:22: error: section attribute not allowed for 'kvm_riscv_gstage_mode'
     366 | unsigned long __init kvm_riscv_gstage_mode(void);
         |                      ^~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/kvm_host.h:366:1: warning: 'cold' attribute ignored [-Wattributes]
     366 | unsigned long __init kvm_riscv_gstage_mode(void);
         | ^~~~~~~~
>> arch/riscv/include/asm/kvm_host.h:369:13: error: section attribute not allowed for 'kvm_riscv_gstage_vmid_detect'
     369 | void __init kvm_riscv_gstage_vmid_detect(void);
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/kvm_host.h:369:1: warning: 'cold' attribute ignored [-Wattributes]
     369 | void __init kvm_riscv_gstage_vmid_detect(void);
         | ^~~~
   In file included from arch/riscv/kernel/asm-offsets.c:15:
>> arch/riscv/include/asm/cpu_ops_sbi.h:13:36: error: storage class specified for parameter 'cpu_ops_sbi'
      13 | extern const struct cpu_operations cpu_ops_sbi;
         |                                    ^~~~~~~~~~~
   arch/riscv/include/asm/cpu_ops_sbi.h:21:1: warning: empty declaration
      21 | struct sbi_hart_boot_data {
         | ^~~~~~
   In file included from arch/riscv/kernel/asm-offsets.c:16:
   arch/riscv/include/asm/stacktrace.h:9:1: warning: empty declaration
       9 | struct stackframe {
         | ^~~~~~
>> arch/riscv/include/asm/stacktrace.h:14:21: error: storage class specified for parameter 'walk_stackframe'
      14 | extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
         |                     ^~~~~~~~~~~~~~~
>> arch/riscv/include/asm/stacktrace.h:14:21: error: 'no_instrument_function' attribute applies only to functions
>> arch/riscv/include/asm/stacktrace.h:16:13: error: storage class specified for parameter 'dump_backtrace'
      16 | extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task,
         |             ^~~~~~~~~~~~~~
>> arch/riscv/include/asm/stacktrace.h:20:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
      20 | {
         | ^
   In file included from arch/riscv/kernel/asm-offsets.c:17:
   arch/riscv/include/asm/suspend.h:12:1: warning: empty declaration
      12 | struct suspend_context {
         | ^~~~~~
>> arch/riscv/include/asm/suspend.h:31:12: error: storage class specified for parameter 'in_suspend'
      31 | extern int in_suspend;
         |            ^~~~~~~~~~
>> arch/riscv/kernel/asm-offsets.c:22:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
      22 | {
         | ^
   include/linux/security.h:1607:12: error: old-style parameter declarations in prototyped function definition
    1607 | static int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/riscv/kernel/asm-offsets.c:514: error: expected '{' at end of input
>> arch/riscv/kernel/asm-offsets.c:513:1: warning: no return statement in function returning non-void [-Wreturn-type]
     513 | }
         | ^
   include/linux/security.h: At top level:
   include/linux/security.h:1607:12: warning: 'security_lsm_manage_policy' defined but not used [-Wunused-function]
    1607 | static int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
   make[3]: *** [scripts/Makefile.build:98: arch/riscv/kernel/asm-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1282: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/sbi_spec_version +423 arch/riscv/include/asm/sbi.h

6cfc624576a641 Andrew Jones    2023-12-20  275  
6cfc624576a641 Andrew Jones    2023-12-20 @276  struct sbi_sta_struct {
6cfc624576a641 Andrew Jones    2023-12-20  277  	__le32 sequence;
6cfc624576a641 Andrew Jones    2023-12-20  278  	__le32 flags;
6cfc624576a641 Andrew Jones    2023-12-20  279  	__le64 steal;
6cfc624576a641 Andrew Jones    2023-12-20  280  	u8 preempted;
6cfc624576a641 Andrew Jones    2023-12-20  281  	u8 pad[47];
6cfc624576a641 Andrew Jones    2023-12-20  282  } __packed;
6cfc624576a641 Andrew Jones    2023-12-20  283  
3ddb6d4df67dad Atish Patra     2024-04-20  284  #define SBI_SHMEM_DISABLE		-1
6cfc624576a641 Andrew Jones    2023-12-20  285  
5daf89e73d77a5 Anup Patel      2024-10-21  286  enum sbi_ext_nacl_fid {
5daf89e73d77a5 Anup Patel      2024-10-21  287  	SBI_EXT_NACL_PROBE_FEATURE = 0x0,
5daf89e73d77a5 Anup Patel      2024-10-21  288  	SBI_EXT_NACL_SET_SHMEM = 0x1,
5daf89e73d77a5 Anup Patel      2024-10-21  289  	SBI_EXT_NACL_SYNC_CSR = 0x2,
5daf89e73d77a5 Anup Patel      2024-10-21  290  	SBI_EXT_NACL_SYNC_HFENCE = 0x3,
5daf89e73d77a5 Anup Patel      2024-10-21  291  	SBI_EXT_NACL_SYNC_SRET = 0x4,
5daf89e73d77a5 Anup Patel      2024-10-21  292  };
5daf89e73d77a5 Anup Patel      2024-10-21  293  
5daf89e73d77a5 Anup Patel      2024-10-21 @294  enum sbi_ext_nacl_feature {
5daf89e73d77a5 Anup Patel      2024-10-21  295  	SBI_NACL_FEAT_SYNC_CSR = 0x0,
5daf89e73d77a5 Anup Patel      2024-10-21  296  	SBI_NACL_FEAT_SYNC_HFENCE = 0x1,
5daf89e73d77a5 Anup Patel      2024-10-21  297  	SBI_NACL_FEAT_SYNC_SRET = 0x2,
5daf89e73d77a5 Anup Patel      2024-10-21  298  	SBI_NACL_FEAT_AUTOSWAP_CSR = 0x3,
5daf89e73d77a5 Anup Patel      2024-10-21  299  };
5daf89e73d77a5 Anup Patel      2024-10-21  300  
5daf89e73d77a5 Anup Patel      2024-10-21  301  #define SBI_NACL_SHMEM_ADDR_SHIFT	12
5daf89e73d77a5 Anup Patel      2024-10-21  302  #define SBI_NACL_SHMEM_SCRATCH_OFFSET	0x0000
5daf89e73d77a5 Anup Patel      2024-10-21  303  #define SBI_NACL_SHMEM_SCRATCH_SIZE	0x1000
5daf89e73d77a5 Anup Patel      2024-10-21  304  #define SBI_NACL_SHMEM_SRET_OFFSET	0x0000
5daf89e73d77a5 Anup Patel      2024-10-21  305  #define SBI_NACL_SHMEM_SRET_SIZE	0x0200
5daf89e73d77a5 Anup Patel      2024-10-21  306  #define SBI_NACL_SHMEM_AUTOSWAP_OFFSET	(SBI_NACL_SHMEM_SRET_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  307  					 SBI_NACL_SHMEM_SRET_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  308  #define SBI_NACL_SHMEM_AUTOSWAP_SIZE	0x0080
5daf89e73d77a5 Anup Patel      2024-10-21  309  #define SBI_NACL_SHMEM_UNUSED_OFFSET	(SBI_NACL_SHMEM_AUTOSWAP_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  310  					 SBI_NACL_SHMEM_AUTOSWAP_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  311  #define SBI_NACL_SHMEM_UNUSED_SIZE	0x0580
5daf89e73d77a5 Anup Patel      2024-10-21  312  #define SBI_NACL_SHMEM_HFENCE_OFFSET	(SBI_NACL_SHMEM_UNUSED_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  313  					 SBI_NACL_SHMEM_UNUSED_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  314  #define SBI_NACL_SHMEM_HFENCE_SIZE	0x0780
5daf89e73d77a5 Anup Patel      2024-10-21  315  #define SBI_NACL_SHMEM_DBITMAP_OFFSET	(SBI_NACL_SHMEM_HFENCE_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  316  					 SBI_NACL_SHMEM_HFENCE_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  317  #define SBI_NACL_SHMEM_DBITMAP_SIZE	0x0080
5daf89e73d77a5 Anup Patel      2024-10-21  318  #define SBI_NACL_SHMEM_CSR_OFFSET	(SBI_NACL_SHMEM_DBITMAP_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  319  					 SBI_NACL_SHMEM_DBITMAP_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  320  #define SBI_NACL_SHMEM_CSR_SIZE		((__riscv_xlen / 8) * 1024)
5daf89e73d77a5 Anup Patel      2024-10-21  321  #define SBI_NACL_SHMEM_SIZE		(SBI_NACL_SHMEM_CSR_OFFSET + \
5daf89e73d77a5 Anup Patel      2024-10-21  322  					 SBI_NACL_SHMEM_CSR_SIZE)
5daf89e73d77a5 Anup Patel      2024-10-21  323  
5daf89e73d77a5 Anup Patel      2024-10-21  324  #define SBI_NACL_SHMEM_CSR_INDEX(__csr_num)	\
5daf89e73d77a5 Anup Patel      2024-10-21  325  		((((__csr_num) & 0xc00) >> 2) | ((__csr_num) & 0xff))
5daf89e73d77a5 Anup Patel      2024-10-21  326  
5daf89e73d77a5 Anup Patel      2024-10-21  327  #define SBI_NACL_SHMEM_HFENCE_ENTRY_SZ		((__riscv_xlen / 8) * 4)
5daf89e73d77a5 Anup Patel      2024-10-21  328  #define SBI_NACL_SHMEM_HFENCE_ENTRY_MAX		\
5daf89e73d77a5 Anup Patel      2024-10-21  329  		(SBI_NACL_SHMEM_HFENCE_SIZE /	\
5daf89e73d77a5 Anup Patel      2024-10-21  330  		 SBI_NACL_SHMEM_HFENCE_ENTRY_SZ)
5daf89e73d77a5 Anup Patel      2024-10-21  331  #define SBI_NACL_SHMEM_HFENCE_ENTRY(__num)	\
5daf89e73d77a5 Anup Patel      2024-10-21  332  		(SBI_NACL_SHMEM_HFENCE_OFFSET +	\
5daf89e73d77a5 Anup Patel      2024-10-21  333  		 (__num) * SBI_NACL_SHMEM_HFENCE_ENTRY_SZ)
5daf89e73d77a5 Anup Patel      2024-10-21  334  #define SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(__num)	\
5daf89e73d77a5 Anup Patel      2024-10-21  335  		SBI_NACL_SHMEM_HFENCE_ENTRY(__num)
5daf89e73d77a5 Anup Patel      2024-10-21  336  #define SBI_NACL_SHMEM_HFENCE_ENTRY_PNUM(__num)\
5daf89e73d77a5 Anup Patel      2024-10-21  337  		(SBI_NACL_SHMEM_HFENCE_ENTRY(__num) + (__riscv_xlen / 8))
5daf89e73d77a5 Anup Patel      2024-10-21  338  #define SBI_NACL_SHMEM_HFENCE_ENTRY_PCOUNT(__num)\
5daf89e73d77a5 Anup Patel      2024-10-21  339  		(SBI_NACL_SHMEM_HFENCE_ENTRY(__num) + \
5daf89e73d77a5 Anup Patel      2024-10-21  340  		 ((__riscv_xlen / 8) * 3))
5daf89e73d77a5 Anup Patel      2024-10-21  341  
5daf89e73d77a5 Anup Patel      2024-10-21  342  #define SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_BITS	1
5daf89e73d77a5 Anup Patel      2024-10-21  343  #define SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_SHIFT	\
5daf89e73d77a5 Anup Patel      2024-10-21  344  		(__riscv_xlen - SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_BITS)
5daf89e73d77a5 Anup Patel      2024-10-21  345  #define SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_MASK	\
5daf89e73d77a5 Anup Patel      2024-10-21  346  		((1UL << SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_BITS) - 1)
5daf89e73d77a5 Anup Patel      2024-10-21  347  #define SBI_NACL_SHMEM_HFENCE_CONFIG_PEND		\
5daf89e73d77a5 Anup Patel      2024-10-21  348  		(SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_MASK << \
5daf89e73d77a5 Anup Patel      2024-10-21  349  		 SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_SHIFT)
5daf89e73d77a5 Anup Patel      2024-10-21  350  
5daf89e73d77a5 Anup Patel      2024-10-21  351  #define SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD1_BITS	3
5daf89e73d77a5 Anup Patel      2024-10-21  352  #define SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD1_SHIFT \
5daf89e73d77a5 Anup Patel      2024-10-21  353  		(SBI_NACL_SHMEM_HFENCE_CONFIG_PEND_SHIFT - \
5daf89e73d77a5 Anup Patel      2024-10-21  354  		 SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD1_BITS)
5daf89e73d77a5 Anup Patel      2024-10-21  355  
5daf89e73d77a5 Anup Patel      2024-10-21  356  #define SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_BITS	4
5daf89e73d77a5 Anup Patel      2024-10-21  357  #define SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_SHIFT	\
5daf89e73d77a5 Anup Patel      2024-10-21  358  		(SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD1_SHIFT - \
5daf89e73d77a5 Anup Patel      2024-10-21  359  		 SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_BITS)
5daf89e73d77a5 Anup Patel      2024-10-21  360  #define SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_MASK	\
5daf89e73d77a5 Anup Patel      2024-10-21  361  		((1UL << SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_BITS) - 1)
5daf89e73d77a5 Anup Patel      2024-10-21  362  
5daf89e73d77a5 Anup Patel      2024-10-21  363  #define SBI_NACL_SHMEM_HFENCE_TYPE_GVMA		0x0
5daf89e73d77a5 Anup Patel      2024-10-21  364  #define SBI_NACL_SHMEM_HFENCE_TYPE_GVMA_ALL	0x1
5daf89e73d77a5 Anup Patel      2024-10-21  365  #define SBI_NACL_SHMEM_HFENCE_TYPE_GVMA_VMID	0x2
5daf89e73d77a5 Anup Patel      2024-10-21  366  #define SBI_NACL_SHMEM_HFENCE_TYPE_GVMA_VMID_ALL 0x3
5daf89e73d77a5 Anup Patel      2024-10-21  367  #define SBI_NACL_SHMEM_HFENCE_TYPE_VVMA		0x4
5daf89e73d77a5 Anup Patel      2024-10-21  368  #define SBI_NACL_SHMEM_HFENCE_TYPE_VVMA_ALL	0x5
5daf89e73d77a5 Anup Patel      2024-10-21  369  #define SBI_NACL_SHMEM_HFENCE_TYPE_VVMA_ASID	0x6
5daf89e73d77a5 Anup Patel      2024-10-21  370  #define SBI_NACL_SHMEM_HFENCE_TYPE_VVMA_ASID_ALL 0x7
5daf89e73d77a5 Anup Patel      2024-10-21  371  
5daf89e73d77a5 Anup Patel      2024-10-21  372  #define SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD2_BITS	1
5daf89e73d77a5 Anup Patel      2024-10-21  373  #define SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD2_SHIFT \
5daf89e73d77a5 Anup Patel      2024-10-21  374  		(SBI_NACL_SHMEM_HFENCE_CONFIG_TYPE_SHIFT - \
5daf89e73d77a5 Anup Patel      2024-10-21  375  		 SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD2_BITS)
5daf89e73d77a5 Anup Patel      2024-10-21  376  
5daf89e73d77a5 Anup Patel      2024-10-21  377  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ORDER_BITS	7
5daf89e73d77a5 Anup Patel      2024-10-21  378  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ORDER_SHIFT \
5daf89e73d77a5 Anup Patel      2024-10-21  379  		(SBI_NACL_SHMEM_HFENCE_CONFIG_RSVD2_SHIFT - \
5daf89e73d77a5 Anup Patel      2024-10-21  380  		 SBI_NACL_SHMEM_HFENCE_CONFIG_ORDER_BITS)
5daf89e73d77a5 Anup Patel      2024-10-21  381  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ORDER_MASK	\
5daf89e73d77a5 Anup Patel      2024-10-21  382  		((1UL << SBI_NACL_SHMEM_HFENCE_CONFIG_ORDER_BITS) - 1)
5daf89e73d77a5 Anup Patel      2024-10-21  383  #define SBI_NACL_SHMEM_HFENCE_ORDER_BASE	12
5daf89e73d77a5 Anup Patel      2024-10-21  384  
5daf89e73d77a5 Anup Patel      2024-10-21  385  #if __riscv_xlen == 32
5daf89e73d77a5 Anup Patel      2024-10-21  386  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ASID_BITS	9
5daf89e73d77a5 Anup Patel      2024-10-21  387  #define SBI_NACL_SHMEM_HFENCE_CONFIG_VMID_BITS	7
5daf89e73d77a5 Anup Patel      2024-10-21  388  #else
5daf89e73d77a5 Anup Patel      2024-10-21  389  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ASID_BITS	16
5daf89e73d77a5 Anup Patel      2024-10-21  390  #define SBI_NACL_SHMEM_HFENCE_CONFIG_VMID_BITS	14
5daf89e73d77a5 Anup Patel      2024-10-21  391  #endif
5daf89e73d77a5 Anup Patel      2024-10-21  392  #define SBI_NACL_SHMEM_HFENCE_CONFIG_VMID_SHIFT	\
5daf89e73d77a5 Anup Patel      2024-10-21  393  				SBI_NACL_SHMEM_HFENCE_CONFIG_ASID_BITS
5daf89e73d77a5 Anup Patel      2024-10-21  394  #define SBI_NACL_SHMEM_HFENCE_CONFIG_ASID_MASK	\
5daf89e73d77a5 Anup Patel      2024-10-21  395  		((1UL << SBI_NACL_SHMEM_HFENCE_CONFIG_ASID_BITS) - 1)
5daf89e73d77a5 Anup Patel      2024-10-21  396  #define SBI_NACL_SHMEM_HFENCE_CONFIG_VMID_MASK	\
5daf89e73d77a5 Anup Patel      2024-10-21  397  		((1UL << SBI_NACL_SHMEM_HFENCE_CONFIG_VMID_BITS) - 1)
5daf89e73d77a5 Anup Patel      2024-10-21  398  
5daf89e73d77a5 Anup Patel      2024-10-21  399  #define SBI_NACL_SHMEM_AUTOSWAP_FLAG_HSTATUS	BIT(0)
5daf89e73d77a5 Anup Patel      2024-10-21  400  #define SBI_NACL_SHMEM_AUTOSWAP_HSTATUS		((__riscv_xlen / 8) * 1)
5daf89e73d77a5 Anup Patel      2024-10-21  401  
5daf89e73d77a5 Anup Patel      2024-10-21  402  #define SBI_NACL_SHMEM_SRET_X(__i)		((__riscv_xlen / 8) * (__i))
5daf89e73d77a5 Anup Patel      2024-10-21  403  #define SBI_NACL_SHMEM_SRET_X_LAST		31
5daf89e73d77a5 Anup Patel      2024-10-21  404  
6cfc624576a641 Andrew Jones    2023-12-20  405  /* SBI spec version fields */
b9dcd9e415872a Atish Patra     2020-03-17  406  #define SBI_SPEC_VERSION_DEFAULT	0x1
b9dcd9e415872a Atish Patra     2020-03-17  407  #define SBI_SPEC_VERSION_MAJOR_SHIFT	24
b9dcd9e415872a Atish Patra     2020-03-17  408  #define SBI_SPEC_VERSION_MAJOR_MASK	0x7f
b9dcd9e415872a Atish Patra     2020-03-17  409  #define SBI_SPEC_VERSION_MINOR_MASK	0xffffff
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  410  
b9dcd9e415872a Atish Patra     2020-03-17  411  /* SBI return error codes */
b9dcd9e415872a Atish Patra     2020-03-17  412  #define SBI_SUCCESS		0
b9dcd9e415872a Atish Patra     2020-03-17  413  #define SBI_ERR_FAILURE		-1
b9dcd9e415872a Atish Patra     2020-03-17  414  #define SBI_ERR_NOT_SUPPORTED	-2
b9dcd9e415872a Atish Patra     2020-03-17  415  #define SBI_ERR_INVALID_PARAM	-3
b9dcd9e415872a Atish Patra     2020-03-17  416  #define SBI_ERR_DENIED		-4
b9dcd9e415872a Atish Patra     2020-03-17  417  #define SBI_ERR_INVALID_ADDRESS	-5
3e1d86569c210e Atish Patra     2021-11-18  418  #define SBI_ERR_ALREADY_AVAILABLE -6
90beae5185c260 Atish Patra     2022-02-18  419  #define SBI_ERR_ALREADY_STARTED -7
90beae5185c260 Atish Patra     2022-02-18  420  #define SBI_ERR_ALREADY_STOPPED -8
8f486ced2860e1 Atish Patra     2024-04-20  421  #define SBI_ERR_NO_SHMEM	-9
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  422  
b9dcd9e415872a Atish Patra     2020-03-17 @423  extern unsigned long sbi_spec_version;
b9dcd9e415872a Atish Patra     2020-03-17  424  struct sbiret {
b9dcd9e415872a Atish Patra     2020-03-17  425  	long error;
b9dcd9e415872a Atish Patra     2020-03-17  426  	long value;
b9dcd9e415872a Atish Patra     2020-03-17  427  };
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  428  
641e8cd2cbf045 Kefeng Wang     2020-11-26  429  void sbi_init(void);
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  430  long __sbi_base_ecall(int fid);
16badacd8af489 Alexandre Ghiti 2024-03-22  431  struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
16badacd8af489 Alexandre Ghiti 2024-03-22  432  			  unsigned long arg2, unsigned long arg3,
16badacd8af489 Alexandre Ghiti 2024-03-22  433  			  unsigned long arg4, unsigned long arg5,
16badacd8af489 Alexandre Ghiti 2024-03-22  434  			  int fid, int ext);
16badacd8af489 Alexandre Ghiti 2024-03-22  435  #define sbi_ecall(e, f, a0, a1, a2, a3, a4, a5)	\
16badacd8af489 Alexandre Ghiti 2024-03-22  436  		__sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  437  
f503b167b66007 Anup Patel      2023-11-24  438  #ifdef CONFIG_RISCV_SBI_V01
b9dcd9e415872a Atish Patra     2020-03-17  439  void sbi_console_putchar(int ch);
b9dcd9e415872a Atish Patra     2020-03-17  440  int sbi_console_getchar(void);
f503b167b66007 Anup Patel      2023-11-24  441  #else
f503b167b66007 Anup Patel      2023-11-24 @442  static inline void sbi_console_putchar(int ch) { }
f503b167b66007 Anup Patel      2023-11-24  443  static inline int sbi_console_getchar(void) { return -ENOENT; }
f503b167b66007 Anup Patel      2023-11-24  444  #endif
183787c6fcc2c7 Vincent Chen    2021-03-22  445  long sbi_get_mvendorid(void);
183787c6fcc2c7 Vincent Chen    2021-03-22  446  long sbi_get_marchid(void);
183787c6fcc2c7 Vincent Chen    2021-03-22  447  long sbi_get_mimpid(void);
b9dcd9e415872a Atish Patra     2020-03-17  448  void sbi_set_timer(uint64_t stime_value);
b9dcd9e415872a Atish Patra     2020-03-17  449  void sbi_shutdown(void);
832f15f4264681 Anup Patel      2023-03-28  450  void sbi_send_ipi(unsigned int cpu);
26fb751ca37846 Atish Patra     2022-01-20  451  int sbi_remote_fence_i(const struct cpumask *cpu_mask);
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  452  
26fb751ca37846 Atish Patra     2022-01-20  453  int sbi_remote_sfence_vma_asid(const struct cpumask *cpu_mask,
b9dcd9e415872a Atish Patra     2020-03-17  454  				unsigned long start,
b9dcd9e415872a Atish Patra     2020-03-17  455  				unsigned long size,
b9dcd9e415872a Atish Patra     2020-03-17  456  				unsigned long asid);
26fb751ca37846 Atish Patra     2022-01-20  457  int sbi_remote_hfence_gvma(const struct cpumask *cpu_mask,
1ef46c231df4b8 Atish Patra     2020-03-17  458  			   unsigned long start,
1ef46c231df4b8 Atish Patra     2020-03-17  459  			   unsigned long size);
26fb751ca37846 Atish Patra     2022-01-20  460  int sbi_remote_hfence_gvma_vmid(const struct cpumask *cpu_mask,
1ef46c231df4b8 Atish Patra     2020-03-17  461  				unsigned long start,
1ef46c231df4b8 Atish Patra     2020-03-17  462  				unsigned long size,
1ef46c231df4b8 Atish Patra     2020-03-17  463  				unsigned long vmid);
26fb751ca37846 Atish Patra     2022-01-20  464  int sbi_remote_hfence_vvma(const struct cpumask *cpu_mask,
1ef46c231df4b8 Atish Patra     2020-03-17  465  			   unsigned long start,
1ef46c231df4b8 Atish Patra     2020-03-17  466  			   unsigned long size);
26fb751ca37846 Atish Patra     2022-01-20  467  int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask,
1ef46c231df4b8 Atish Patra     2020-03-17  468  				unsigned long start,
1ef46c231df4b8 Atish Patra     2020-03-17  469  				unsigned long size,
1ef46c231df4b8 Atish Patra     2020-03-17  470  				unsigned long asid);
41cad8284d5e6b Andrew Jones    2023-04-27  471  long sbi_probe_extension(int ext);
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  472  
b9dcd9e415872a Atish Patra     2020-03-17  473  /* Check if current SBI specification version is 0.1 or not */
b9dcd9e415872a Atish Patra     2020-03-17  474  static inline int sbi_spec_is_0_1(void)
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  475  {
b9dcd9e415872a Atish Patra     2020-03-17  476  	return (sbi_spec_version == SBI_SPEC_VERSION_DEFAULT) ? 1 : 0;
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  477  }
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  478  
b9dcd9e415872a Atish Patra     2020-03-17  479  /* Get the major version of SBI */
b9dcd9e415872a Atish Patra     2020-03-17  480  static inline unsigned long sbi_major_version(void)
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  481  {
b9dcd9e415872a Atish Patra     2020-03-17  482  	return (sbi_spec_version >> SBI_SPEC_VERSION_MAJOR_SHIFT) &
b9dcd9e415872a Atish Patra     2020-03-17  483  		SBI_SPEC_VERSION_MAJOR_MASK;
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  484  }
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  485  
b9dcd9e415872a Atish Patra     2020-03-17  486  /* Get the minor version of SBI */
b9dcd9e415872a Atish Patra     2020-03-17  487  static inline unsigned long sbi_minor_version(void)
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10 @488  {
b9dcd9e415872a Atish Patra     2020-03-17  489  	return sbi_spec_version & SBI_SPEC_VERSION_MINOR_MASK;
6d60b6ee0c9777 Palmer Dabbelt  2017-07-10  490  }
f90b43ce176c12 Atish Patra     2020-03-17  491  
b579dfe71a6a5c Anup Patel      2021-06-09  492  /* Make SBI version */
b579dfe71a6a5c Anup Patel      2021-06-09  493  static inline unsigned long sbi_mk_version(unsigned long major,
b579dfe71a6a5c Anup Patel      2021-06-09  494  					    unsigned long minor)
b579dfe71a6a5c Anup Patel      2021-06-09  495  {
b737fc24a12ceb Atish Patra     2024-04-20  496  	return ((major & SBI_SPEC_VERSION_MAJOR_MASK) << SBI_SPEC_VERSION_MAJOR_SHIFT)
b737fc24a12ceb Atish Patra     2024-04-20  497  		| (minor & SBI_SPEC_VERSION_MINOR_MASK);
b579dfe71a6a5c Anup Patel      2021-06-09  498  }
b579dfe71a6a5c Anup Patel      2021-06-09  499  
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  500  static inline int sbi_err_map_linux_errno(int err)
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  501  {
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  502  	switch (err) {
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  503  	case SBI_SUCCESS:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  504  		return 0;
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  505  	case SBI_ERR_DENIED:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  506  		return -EPERM;
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  507  	case SBI_ERR_INVALID_PARAM:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  508  		return -EINVAL;
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  509  	case SBI_ERR_INVALID_ADDRESS:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  510  		return -EFAULT;
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  511  	case SBI_ERR_NOT_SUPPORTED:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  512  	case SBI_ERR_FAILURE:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  513  	default:
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  514  		return -ENOTSUPP;
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  515  	};
1ff95eb2bebda5 Alexandre Ghiti 2024-08-29  516  }
f43fabf444ca3c Anup Patel      2023-11-24  517  
f43fabf444ca3c Anup Patel      2023-11-24 @518  extern bool sbi_debug_console_available;
f43fabf444ca3c Anup Patel      2023-11-24  519  int sbi_debug_console_write(const char *bytes, unsigned int num_bytes);
f43fabf444ca3c Anup Patel      2023-11-24  520  int sbi_debug_console_read(char *bytes, unsigned int num_bytes);
f43fabf444ca3c Anup Patel      2023-11-24  521  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
  2025-05-07  6:26   ` Song Liu
@ 2025-05-07 13:58   ` kernel test robot
  1 sibling, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-05-07 13:58 UTC (permalink / raw)
  To: Maxime Bélair, linux-security-module
  Cc: llvm, oe-kbuild-all, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel, Maxime Bélair

Hi Maxime,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 9c32cda43eb78f78c73aee4aa344b777714e259b]

url:    https://github.com/intel-lab-lkp/linux/commits/Maxime-B-lair/Wire-up-the-lsm_manage_policy-syscall/20250506-224212
base:   9c32cda43eb78f78c73aee4aa344b777714e259b
patch link:    https://lore.kernel.org/r/20250506143254.718647-2-maxime.belair%40canonical.com
patch subject: [PATCH 1/3] Wire up the lsm_manage_policy syscall
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20250507/202505072131.ogtsaLPI-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250507/202505072131.ogtsaLPI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505072131.ogtsaLPI-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> <stdin>:1618:2: warning: syscall lsm_manage_policy not implemented [-W#warnings]
    1618 | #warning syscall lsm_manage_policy not implemented
         |  ^
   1 warning generated.
--
>> <stdin>:1618:2: warning: syscall lsm_manage_policy not implemented [-W#warnings]
    1618 | #warning syscall lsm_manage_policy not implemented
         |  ^
   1 warning generated.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-07  6:26   ` Song Liu
@ 2025-05-07 15:37     ` Maxime Bélair
  2025-05-07 22:04       ` Tetsuo Handa
  2025-05-08  6:06       ` Song Liu
  2025-05-08  7:12     ` John Johansen
  1 sibling, 2 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-07 15:37 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel



On 5/7/25 08:26, Song Liu wrote:
> On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
> <maxime.belair@canonical.com> wrote:
>>
>> Add support for the new lsm_manage_policy syscall, providing a unified
>> API for loading and modifying LSM policies without requiring the LSM’s
>> pseudo-filesystem.
>>
>> Benefits:
>>   - Works even if the LSM pseudo-filesystem isn’t mounted or available
>>     (e.g. in containers)
>>   - Offers a logical and unified interface rather than multiple
>>     heterogeneous pseudo-filesystems.
> 
> These two do not feel like real benefits:
> - One syscall cannot fit all use cases well...

This syscall is not intended to cover every case, nor to replace existing kernel
interfaces.

Each LSM can decide which operations it wants to support (if any). For example, when
loading policies, an LSM may choose to allow only policies that further restrict
privileges.

> - Not working in containers is often not an issue, but a feature.

Indeed, using this syscall requires appropriate capabilities and will not permit
unprivileged containers to manage policies arbitrarily.

With this syscall, capability checks remain the responsibility of each LSM.

For instance, in the AppArmor patch, a profile can be loaded only if
aa_policy_admin_capable() succeeds (which requires CAP_MAC_ADMIN). Moreover, by design,
policies can be loaded only in the current namespace.

I see this syscall as a middle point between exposing the entire sysfs, creating a large
attack surface, and blocking everything.

Landlock’s existing syscalls already improve security by allowing processes to further
restrict their ambient rights while adding only a modest attack surface.

This syscall is a further step in that direction: it lets LSMs add restrictive policies 
without requiring exposing every other interface.

Again, each module decides which operations to expose through this syscall. In many cases
the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
that choose this interface remain secure while gaining its advantages.

>>   - Avoids overhead of other kernel interfaces for better efficiency
> 
> .. and it is is probably less efficient, because everything need to
> fit in the same API.

As shown below, the syscall can significantly improve the performance of policy management.
A more detailed benchmark is available in [1].

The following table presents the time required to load an AppArmor profile.

For every cell, the first value is the total time taken by aa-load, and the value in
parentheses is the time spent to load the policy in the kernel only (total - dry‑run).

Results are in microseconds and are averaged over 10 000 runs to reduce variance. 


| t (µs)    | syscall     | pseudofs    | Speedup       |
|-----------|-------------|-------------|---------------|
| 1password | 4257 (1127) | 3333 (192)  | x1.28 (x5.86) |
| Xorg      | 6099 (2961) | 5167 (2020) | x1.18 (x1.47) |

If an LSM wants to allow several operations for a single LSM_POLICY_XXX it can multiplex a sub‑opcode in flags, and select the appropriate handler, this incurs negligible overhead.

Thanks,

Maxime

[1] https://gitlab.com/-/snippets/4840792

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07  6:19   ` Song Liu
@ 2025-05-07 15:37     ` Maxime Bélair
  2025-05-08  8:20       ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Maxime Bélair @ 2025-05-07 15:37 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel



On 5/7/25 08:19, Song Liu wrote:
> On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
> <maxime.belair@canonical.com> wrote:
>>
>> Define a new LSM hook security_lsm_manage_policy and wire it into the
>> lsm_manage_policy() syscall so that LSMs can register a unified interface
>> for policy management. This initial, minimal implementation only supports
>> the LSM_POLICY_LOAD operation to limit changes.
>>
>> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
> [...]
>> diff --git a/security/security.c b/security/security.c
>> index fb57e8fddd91..256104e338b1 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -5883,6 +5883,27 @@ int security_bdev_setintegrity(struct block_device *bdev,
>>  }
>>  EXPORT_SYMBOL(security_bdev_setintegrity);
>>
>> +/**
>> + * security_lsm_manage_policy() - Manage the policies of LSMs
>> + * @lsm_id: id of the lsm to target
>> + * @op: Operation to perform (one of the LSM_POLICY_XXX values)
>> + * @buf:  userspace pointer to policy data
>> + * @size: size of @buf
>> + * @flags: lsm policy management flags
>> + *
>> + * Manage the policies of a LSM. This notably allows to update them even when
>> + * the lsmfs is unavailable is restricted. Currently, only LSM_POLICY_LOAD is
>> + * supported.
>> + *
>> + * Return: Returns 0 on success, error on failure.
>> + */
>> +int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
>> +                              size_t size, u32 flags)
>> +{
>> +       return call_int_hook(lsm_manage_policy, lsm_id, op, buf, size, flags);
> 
> If the LSM doesn't implement this hook, sys_lsm_manage_policy will return 0
> for any inputs, right? This is gonna be so confusing for users.

Indeed, that was an oversight. It will return -EOPNOTSUPP in the next patch revision.

> 
> Thanks,
> Song

Thanks,

Maxime


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07 10:40   ` Tetsuo Handa
@ 2025-05-07 15:37     ` Maxime Bélair
  2025-05-07 20:25     ` Paul Moore
  2025-05-08  8:25     ` John Johansen
  2 siblings, 0 replies; 36+ messages in thread
From: Maxime Bélair @ 2025-05-07 15:37 UTC (permalink / raw)
  To: Tetsuo Handa, linux-security-module
  Cc: john.johansen, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, linux-api, apparmor,
	linux-kernel



On 5/7/25 12:40, Tetsuo Handa wrote:
> On 2025/05/06 23:32, Maxime Bélair wrote:
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> index dcaad8818679..b39e6635a7d5 100644
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>>  SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
>>  		__user *, size, u32, flags)
>>  {
>> -	return 0;
>> +	size_t usize;
>> +
>> +	if (get_user(usize, size))
>> +		return -EFAULT;
>> +
>> +	return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
>>  }
> 
> syzbot will report user-controlled unbounded huge size memory allocation attempt. ;-)
> 
> This interface might be fine for AppArmor, but TOMOYO won't use this interface because
> TOMOYO's policy is line-oriented ASCII text data where the destination is switched via
> pseudo‑filesystem's filename; use of filename helps restricting which type of policy
> can be manipulated by which process. 

First, like any LSM, TOMOYO is not obliged to implement every operation. It can simply
expose the one that makes sense for its use case. For instance, I don't think it needs an
equivalent of the manager interface.

If TOMOYO wants to support several sub‑operations, it can distinguish them with the
syscall’s flags parameter instead of filenames (as securityfs_if.c does today) and reuse
the code already employed by its pseudo‑fs, as in the AppArmor patch. Supporting this
syscall would therefore require only minimal changes.

Line‑oriented ASCII text is not a barrier, either. The syscall can pass that format just
fine. Because a typical TOMOYO line is very small, the performance gains from using the
syscall are actually greater. A brief benchmark is available in [1].

Thanks,

Maxime

[1] https://gitlab.com/-/snippets/4840792

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07 10:40   ` Tetsuo Handa
  2025-05-07 15:37     ` Maxime Bélair
@ 2025-05-07 20:25     ` Paul Moore
  2025-05-08  8:29       ` John Johansen
  2025-05-08  8:25     ` John Johansen
  2 siblings, 1 reply; 36+ messages in thread
From: Paul Moore @ 2025-05-07 20:25 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: Tetsuo Handa, linux-security-module, john.johansen, jmorris,
	serge, mic, kees, stephen.smalley.work, casey, takedakn,
	linux-api, apparmor, linux-kernel

On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2025/05/06 23:32, Maxime Bélair wrote:
> > diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> > index dcaad8818679..b39e6635a7d5 100644
> > --- a/security/lsm_syscalls.c
> > +++ b/security/lsm_syscalls.c
> > @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
> >  SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
> >               __user *, size, u32, flags)
> >  {
> > -     return 0;
> > +     size_t usize;
> > +
> > +     if (get_user(usize, size))
> > +             return -EFAULT;
> > +
> > +     return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
> >  }
>
> syzbot will report user-controlled unbounded huge size memory allocation attempt. ;-)
>
> This interface might be fine for AppArmor, but TOMOYO won't use this interface because
> TOMOYO's policy is line-oriented ASCII text data where the destination is switched via
> pseudo‑filesystem's filename ...

While Tetsuo's comment is limited to TOMOYO, I believe the argument
applies to a number of other LSMs as well.  The reality is that there
is no one policy ideal shared across LSMs and that complicates things
like the lsm_manage_policy() proposal.  I'm intentionally saying
"complicates" and not "prevents" because I don't want to flat out
reject something like this, but I think there needs to be a larger
discussion among the different LSM groups about what such an API
should look like.  We may not need to get every LSM to support this
new API, but we need to get something that would work for a
significant majority and would be general/extensible enough that we
would expect it to work with the majority of future LSMs (as much as
we can predict the future anyway).

--
paul-moore.com

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-07 15:37     ` Maxime Bélair
@ 2025-05-07 22:04       ` Tetsuo Handa
  2025-05-08  7:52         ` John Johansen
  2025-05-08  6:06       ` Song Liu
  1 sibling, 1 reply; 36+ messages in thread
From: Tetsuo Handa @ 2025-05-07 22:04 UTC (permalink / raw)
  To: Maxime Bélair, Song Liu
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, linux-api, apparmor,
	linux-kernel

On 2025/05/08 0:37, Maxime Bélair wrote:
> Again, each module decides which operations to expose through this syscall. In many cases
> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
> that choose this interface remain secure while gaining its advantages.

If the interpretation of "flags" argument varies across LSMs, it sounds like ioctl()'s
"cmd" argument. Also, there is prctl() which can already carry string-ish parameters
without involving open(). Why can't we use prctl() instead of lsm_manage_policy() ?


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-07 15:37     ` Maxime Bélair
  2025-05-07 22:04       ` Tetsuo Handa
@ 2025-05-08  6:06       ` Song Liu
  2025-05-08  8:18         ` John Johansen
  1 sibling, 1 reply; 36+ messages in thread
From: Song Liu @ 2025-05-08  6:06 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: linux-security-module, john.johansen, paul, jmorris, serge, mic,
	kees, stephen.smalley.work, casey, takedakn, penguin-kernel,
	linux-api, apparmor, linux-kernel

On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
<maxime.belair@canonical.com> wrote:
[...]
> >
> > These two do not feel like real benefits:
> > - One syscall cannot fit all use cases well...
>
> This syscall is not intended to cover every case, nor to replace existing kernel
> interfaces.
>
> Each LSM can decide which operations it wants to support (if any). For example, when
> loading policies, an LSM may choose to allow only policies that further restrict
> privileges.
>
> > - Not working in containers is often not an issue, but a feature.
>
> Indeed, using this syscall requires appropriate capabilities and will not permit
> unprivileged containers to manage policies arbitrarily.
>
> With this syscall, capability checks remain the responsibility of each LSM.
>
> For instance, in the AppArmor patch, a profile can be loaded only if
> aa_policy_admin_capable() succeeds (which requires CAP_MAC_ADMIN). Moreover, by design,
> policies can be loaded only in the current namespace.
>
> I see this syscall as a middle point between exposing the entire sysfs, creating a large
> attack surface, and blocking everything.
>
> Landlock’s existing syscalls already improve security by allowing processes to further
> restrict their ambient rights while adding only a modest attack surface.
>
> This syscall is a further step in that direction: it lets LSMs add restrictive policies
> without requiring exposing every other interface.

I don't think a syscall makes the API more secure. If necessary, we can add
permission check to each pseudo file. The downside of the syscall, however,
is that all the permission checks are hard-coded in the kernel (except for
BPF LSM); while the sys admin can configure permissions of the pseudo
files in user space.

> Again, each module decides which operations to expose through this syscall. In many cases
> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
> that choose this interface remain secure while gaining its advantages.
>
> >>   - Avoids overhead of other kernel interfaces for better efficiency
> >
> > .. and it is is probably less efficient, because everything need to
> > fit in the same API.
>
> As shown below, the syscall can significantly improve the performance of policy management.
> A more detailed benchmark is available in [1].
>
> The following table presents the time required to load an AppArmor profile.
>
> For every cell, the first value is the total time taken by aa-load, and the value in
> parentheses is the time spent to load the policy in the kernel only (total - dry‑run).
>
> Results are in microseconds and are averaged over 10 000 runs to reduce variance.
>
>
> | t (µs)    | syscall     | pseudofs    | Speedup       |
> |-----------|-------------|-------------|---------------|
> | 1password | 4257 (1127) | 3333 (192)  | x1.28 (x5.86) |
> | Xorg      | 6099 (2961) | 5167 (2020) | x1.18 (x1.47) |
>

I am not sure the performance of loading security policies is on any
critical path.
The implementation calls the hook for each LSM, which is why I think the
syscall is not efficient.

Overall, I am still not convinced a syscall for all LSMs is needed. To
justify such
a syscall, I think we need to show that it is useful in multiple LSMs.
Also, if we
really want to have single set of APIs for all LSMs, we may also need
get_policy,
remove_policy, etc. This set as-is appears to be an incomplete design. The
implementation, with call_int_hook, is also problematic. It can easily
cause some
controversial behaviors.

Thanks,
Song

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-07  6:26   ` Song Liu
  2025-05-07 15:37     ` Maxime Bélair
@ 2025-05-08  7:12     ` John Johansen
  1 sibling, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-08  7:12 UTC (permalink / raw)
  To: Song Liu, Maxime Bélair
  Cc: linux-security-module, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel

On 5/6/25 23:26, Song Liu wrote:
> On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
> <maxime.belair@canonical.com> wrote:
>>
>> Add support for the new lsm_manage_policy syscall, providing a unified
>> API for loading and modifying LSM policies without requiring the LSM’s
>> pseudo-filesystem.
>>
>> Benefits:
>>    - Works even if the LSM pseudo-filesystem isn’t mounted or available
>>      (e.g. in containers)
>>    - Offers a logical and unified interface rather than multiple
>>      heterogeneous pseudo-filesystems.
> 
> These two do not feel like real benefits:
> - Not working in containers is often not an issue, but a feature.

and the LSM doesn't have to allow the syscall to function in a container
where appropriate. Its up to the LSM if the syscall is supported and
what kind of permissions are needed.

However having the ability to function in a container and not having to
mount securityfs, or procfs into a container. similar to what landlock
gets with its syscall can be beneficial.

> - One syscall cannot fit all use cases well...
> 
of course not, and for those other use cases new syscalls can be added.

>>    - Avoids overhead of other kernel interfaces for better efficiency
> 
> .. and it is is probably less efficient, because everything need to
> fit in the same API.
> 
no not everything, just what fits into the syscall. Nor does an LSM
have to use the syscall it is still use what works for it.

This could be a little more efficient than the current fs interface
used by apparmor/selinux/smack but I don't think efficiency is going
to be a huge win for this.


> Overall, this set doesn't feel like a good change to me.
> 
> Thanks,
> Song


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-07 22:04       ` Tetsuo Handa
@ 2025-05-08  7:52         ` John Johansen
  2025-05-09 10:25           ` Mickaël Salaün
  0 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-08  7:52 UTC (permalink / raw)
  To: Tetsuo Handa, Maxime Bélair, Song Liu
  Cc: linux-security-module, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, linux-api, apparmor,
	linux-kernel

On 5/7/25 15:04, Tetsuo Handa wrote:
> On 2025/05/08 0:37, Maxime Bélair wrote:
>> Again, each module decides which operations to expose through this syscall. In many cases
>> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
>> that choose this interface remain secure while gaining its advantages.
> 
> If the interpretation of "flags" argument varies across LSMs, it sounds like ioctl()'s

yes that does feel like ioctls(), on the other hand defining them at the LSM level won't
offer LSMs flexibility making it so the syscall covers fewer use cases. I am not opposed
to either, it just hashing out what people want, and what is acceptable.

> "cmd" argument. Also, there is prctl() which can already carry string-ish parameters
> without involving open(). Why can't we use prctl() instead of lsm_manage_policy() ?
> 

prctl() can be used, I used it for the unprivileged policy demo. It has its own set of
problems. While LSM policy could be associated with the process doing the load/replacement
or what ever operation, it isn't necessarily tied to it. A lot of LSM policy is not
process specific making prctl() a poor fit.

prctl() requires allocating a global prctl()

prctl() are already being filtered/controlled by LSMs making them a poort fit for
use by an LSM in a stacking situation as it requires updating the policy of other
LSMs on the system. Yes seccomp can filter the syscall but that still is an easier
barrier to overcome than having to have instruction for how to allow your LSMs
prctl() in multiple LSMs.


Mickaël already argued the need for landlock to have syscalls. See
https://lore.kernel.org/lkml/20200511192156.1618284-7-mic@digikod.net/
and the numerous iterations before that.

Ideally those could have been LSM syscalls, with landlock leveraging them. AppArmor
is getting to where it has similar needs to landlock. Yes we can use ioctls, prctls,
netlink, the fs, etc. it doesn't mean that those are the best interfaces to do so,
and ideally any interface we use will be of benefit to some other LSMs in the future.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-08  6:06       ` Song Liu
@ 2025-05-08  8:18         ` John Johansen
  2025-05-09 10:26           ` Mickaël Salaün
  0 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-08  8:18 UTC (permalink / raw)
  To: Song Liu, Maxime Bélair
  Cc: linux-security-module, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel

On 5/7/25 23:06, Song Liu wrote:
> On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
> <maxime.belair@canonical.com> wrote:
> [...]
>>>
>>> These two do not feel like real benefits:
>>> - One syscall cannot fit all use cases well...
>>
>> This syscall is not intended to cover every case, nor to replace existing kernel
>> interfaces.
>>
>> Each LSM can decide which operations it wants to support (if any). For example, when
>> loading policies, an LSM may choose to allow only policies that further restrict
>> privileges.
>>
>>> - Not working in containers is often not an issue, but a feature.
>>
>> Indeed, using this syscall requires appropriate capabilities and will not permit
>> unprivileged containers to manage policies arbitrarily.
>>
>> With this syscall, capability checks remain the responsibility of each LSM.
>>
>> For instance, in the AppArmor patch, a profile can be loaded only if
>> aa_policy_admin_capable() succeeds (which requires CAP_MAC_ADMIN). Moreover, by design,
>> policies can be loaded only in the current namespace.
>>
>> I see this syscall as a middle point between exposing the entire sysfs, creating a large
>> attack surface, and blocking everything.
>>
>> Landlock’s existing syscalls already improve security by allowing processes to further
>> restrict their ambient rights while adding only a modest attack surface.
>>
>> This syscall is a further step in that direction: it lets LSMs add restrictive policies
>> without requiring exposing every other interface.
> 
> I don't think a syscall makes the API more secure. If necessary, we can add

It exposes a different attack surface. Requiring mounting of the fs to where it is visible
in the container, provides attack surface, and requires additional external configuration.

Then there is the whole issue of getting the various LSMs to allow another LSM in the
stack to be able manage its own policy.

> permission check to each pseudo file. The downside of the syscall, however,
> is that all the permission checks are hard-coded in the kernel (except for

The permission checks don't have to be hard coded. Each LSM can define how it handles
or manages the syscall. The default is that it isn't supported, but if an lsm decides
to support it, there is now reason that its policy can't determine the use of the
syscall.

> BPF LSM); while the sys admin can configure permissions of the pseudo
> files in user space.
> 
Other LSMs also have policy that can control access to pseudo filesystems and
other resources. Again, the control doesn't have to be hard coded. And seccomp can
be used to block the syscall.



>> Again, each module decides which operations to expose through this syscall. In many cases
>> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
>> that choose this interface remain secure while gaining its advantages.
>>
>>>>    - Avoids overhead of other kernel interfaces for better efficiency
>>>
>>> .. and it is is probably less efficient, because everything need to
>>> fit in the same API.
>>
>> As shown below, the syscall can significantly improve the performance of policy management.
>> A more detailed benchmark is available in [1].
>>
>> The following table presents the time required to load an AppArmor profile.
>>
>> For every cell, the first value is the total time taken by aa-load, and the value in
>> parentheses is the time spent to load the policy in the kernel only (total - dry‑run).
>>
>> Results are in microseconds and are averaged over 10 000 runs to reduce variance.
>>
>>
>> | t (µs)    | syscall     | pseudofs    | Speedup       |
>> |-----------|-------------|-------------|---------------|
>> | 1password | 4257 (1127) | 3333 (192)  | x1.28 (x5.86) |
>> | Xorg      | 6099 (2961) | 5167 (2020) | x1.18 (x1.47) |
>>
> 
> I am not sure the performance of loading security policies is on any
> critical path.

generally speaking I agree, but I am also not going to turn down a
performance improvement either. Its a nice to have, but not a strong
argument for need.

> The implementation calls the hook for each LSM, which is why I think the
> syscall is not efficient.
> 
it should only call the LSM identified by the lsmid in the call.

> Overall, I am still not convinced a syscall for all LSMs is needed. To
> justify such

its not needed by all LSMs, just a subset of them, and some nebulous
subset of potentially future LSMs that is entirely undefinable.

If we had had appropriate LSM syscalls landlock wouldn't have needed
to have landlock specific syscalls. Having another LSM go that route
feels wrong especially now that we have some LSM syscalls. If a
syscall is needed by an LSM its better to try hashing something out
that might have utility for multiple LSMs or at the very least,
potentially have utility in the future.


> a syscall, I think we need to show that it is useful in multiple LSMs.
> Also, if we
> really want to have single set of APIs for all LSMs, we may also need
> get_policy,

We are never going to get a single set of APIs for all LSMs. I will
settle for an api that has utility for a subset

> remove_policy, etc. This set as-is appears to be an incomplete design. The

To have a complete design, there needs to be feedback and discussion
from multiple LSMs. This is a starting point.

> implementation, with call_int_hook, is also problematic. It can easily
> cause some> controversial behaviors.
> 
agreed it shouldn't be doing a straight call_int_hook, it should only
call it against the lsm identified by the lsmid


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07 15:37     ` Maxime Bélair
@ 2025-05-08  8:20       ` John Johansen
  0 siblings, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-08  8:20 UTC (permalink / raw)
  To: Maxime Bélair, Song Liu
  Cc: linux-security-module, paul, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, penguin-kernel, linux-api,
	apparmor, linux-kernel

On 5/7/25 08:37, Maxime Bélair wrote:
> 
> 
> On 5/7/25 08:19, Song Liu wrote:
>> On Tue, May 6, 2025 at 7:40 AM Maxime Bélair
>> <maxime.belair@canonical.com> wrote:
>>>
>>> Define a new LSM hook security_lsm_manage_policy and wire it into the
>>> lsm_manage_policy() syscall so that LSMs can register a unified interface
>>> for policy management. This initial, minimal implementation only supports
>>> the LSM_POLICY_LOAD operation to limit changes.
>>>
>>> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
>> [...]
>>> diff --git a/security/security.c b/security/security.c
>>> index fb57e8fddd91..256104e338b1 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -5883,6 +5883,27 @@ int security_bdev_setintegrity(struct block_device *bdev,
>>>   }
>>>   EXPORT_SYMBOL(security_bdev_setintegrity);
>>>
>>> +/**
>>> + * security_lsm_manage_policy() - Manage the policies of LSMs
>>> + * @lsm_id: id of the lsm to target
>>> + * @op: Operation to perform (one of the LSM_POLICY_XXX values)
>>> + * @buf:  userspace pointer to policy data
>>> + * @size: size of @buf
>>> + * @flags: lsm policy management flags
>>> + *
>>> + * Manage the policies of a LSM. This notably allows to update them even when
>>> + * the lsmfs is unavailable is restricted. Currently, only LSM_POLICY_LOAD is
>>> + * supported.
>>> + *
>>> + * Return: Returns 0 on success, error on failure.
>>> + */
>>> +int security_lsm_manage_policy(u32 lsm_id, u32 op, void __user *buf,
>>> +                              size_t size, u32 flags)
>>> +{
>>> +       return call_int_hook(lsm_manage_policy, lsm_id, op, buf, size, flags);
>>
>> If the LSM doesn't implement this hook, sys_lsm_manage_policy will return 0
>> for any inputs, right? This is gonna be so confusing for users.
> 
> Indeed, that was an oversight. It will return -EOPNOTSUPP in the next patch revision.
> 

I think it needs to do more than that. I don't think this should call each LSM, the
infrastructure should filter it and only send it to the LSM identified by the lsm_id


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07 10:40   ` Tetsuo Handa
  2025-05-07 15:37     ` Maxime Bélair
  2025-05-07 20:25     ` Paul Moore
@ 2025-05-08  8:25     ` John Johansen
  2025-05-08 12:55       ` Tetsuo Handa
  2 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-08  8:25 UTC (permalink / raw)
  To: Tetsuo Handa, Maxime Bélair, linux-security-module
  Cc: paul, jmorris, serge, mic, kees, stephen.smalley.work, casey,
	takedakn, linux-api, apparmor, linux-kernel

On 5/7/25 03:40, Tetsuo Handa wrote:
> On 2025/05/06 23:32, Maxime Bélair wrote:
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> index dcaad8818679..b39e6635a7d5 100644
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>>   SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
>>   		__user *, size, u32, flags)
>>   {
>> -	return 0;
>> +	size_t usize;
>> +
>> +	if (get_user(usize, size))
>> +		return -EFAULT;
>> +
>> +	return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
>>   }
> 
> syzbot will report user-controlled unbounded huge size memory allocation attempt. ;-)
> 
> This interface might be fine for AppArmor, but TOMOYO won't use this interface because
> TOMOYO's policy is line-oriented ASCII text data where the destination is switched via
> pseudo‑filesystem's filename; use of filename helps restricting which type of policy
> can be manipulated by which process.
> 

That is fine. But curious I am curious what the interface would look like to fit TOMOYO's
needs. I look at the current implementation as an opening discussion of what the syscall
should look like. I have no delusions that we are going to get something that will fit
all LSMs but without requirements, we won't be able to even attempt to hash something
better out.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-07 20:25     ` Paul Moore
@ 2025-05-08  8:29       ` John Johansen
  2025-05-08 16:54         ` Casey Schaufler
  0 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-08  8:29 UTC (permalink / raw)
  To: Paul Moore, Maxime Bélair
  Cc: Tetsuo Handa, linux-security-module, jmorris, serge, mic, kees,
	stephen.smalley.work, casey, takedakn, linux-api, apparmor,
	linux-kernel

On 5/7/25 13:25, Paul Moore wrote:
> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>> On 2025/05/06 23:32, Maxime Bélair wrote:
>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>>> index dcaad8818679..b39e6635a7d5 100644
>>> --- a/security/lsm_syscalls.c
>>> +++ b/security/lsm_syscalls.c
>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>>>   SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void __user *, buf, u32
>>>                __user *, size, u32, flags)
>>>   {
>>> -     return 0;
>>> +     size_t usize;
>>> +
>>> +     if (get_user(usize, size))
>>> +             return -EFAULT;
>>> +
>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize, flags);
>>>   }
>>
>> syzbot will report user-controlled unbounded huge size memory allocation attempt. ;-)
>>
>> This interface might be fine for AppArmor, but TOMOYO won't use this interface because
>> TOMOYO's policy is line-oriented ASCII text data where the destination is switched via
>> pseudo‑filesystem's filename ...
> 
> While Tetsuo's comment is limited to TOMOYO, I believe the argument
> applies to a number of other LSMs as well.  The reality is that there
> is no one policy ideal shared across LSMs and that complicates things
> like the lsm_manage_policy() proposal.  I'm intentionally saying
> "complicates" and not "prevents" because I don't want to flat out
> reject something like this, but I think there needs to be a larger
> discussion among the different LSM groups about what such an API
> should look like.  We may not need to get every LSM to support this
> new API, but we need to get something that would work for a
> significant majority and would be general/extensible enough that we
> would expect it to work with the majority of future LSMs (as much as
> we can predict the future anyway).
> 

yep, I look at this is just a starting point for discussion. There
isn't going to be any discussion without some code, so here is a v1
that supports a single LSM let the bike shedding begin.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08  8:25     ` John Johansen
@ 2025-05-08 12:55       ` Tetsuo Handa
  2025-05-08 14:44         ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Tetsuo Handa @ 2025-05-08 12:55 UTC (permalink / raw)
  To: John Johansen, Maxime Bélair, linux-security-module
  Cc: paul, jmorris, serge, mic, kees, stephen.smalley.work, casey,
	takedakn, linux-api, apparmor, linux-kernel

On 2025/05/08 17:25, John Johansen wrote:
> That is fine. But curious I am curious what the interface would look like to fit TOMOYO's
> needs.

Stream (like "FILE *") with restart from the beginning (like rewind(fp)) support.
That is, the caller can read/write at least one byte at a time, and written data
is processed upon encountering '\n'.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08 12:55       ` Tetsuo Handa
@ 2025-05-08 14:44         ` John Johansen
  2025-05-08 15:07           ` Tetsuo Handa
  0 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-08 14:44 UTC (permalink / raw)
  To: Tetsuo Handa, Maxime Bélair, linux-security-module
  Cc: paul, jmorris, serge, mic, kees, stephen.smalley.work, casey,
	takedakn, linux-api, apparmor, linux-kernel

On 5/8/25 05:55, Tetsuo Handa wrote:
> On 2025/05/08 17:25, John Johansen wrote:
>> That is fine. But curious I am curious what the interface would look like to fit TOMOYO's
>> needs.
> 
> Stream (like "FILE *") with restart from the beginning (like rewind(fp)) support.
> That is, the caller can read/write at least one byte at a time, and written data
> is processed upon encountering '\n'.
> 

that can be emulated within the current sycall, where the lsm maintains a buffer.
Are you asking to also read data back out as well, that could be added, but doing
a syscall per byte here or through the fs is going to have fairly high overhead.

Without understanding the requirement it would seem to me, that it would be
better to emulate that file buffer manipulation in userspace similar say C++
stringstreams, and then write the syscall when done.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08 14:44         ` John Johansen
@ 2025-05-08 15:07           ` Tetsuo Handa
  2025-05-09  3:25             ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Tetsuo Handa @ 2025-05-08 15:07 UTC (permalink / raw)
  To: John Johansen, Maxime Bélair, linux-security-module
  Cc: paul, jmorris, serge, mic, kees, stephen.smalley.work, casey,
	takedakn, linux-api, apparmor, linux-kernel

On 2025/05/08 23:44, John Johansen wrote:
> On 5/8/25 05:55, Tetsuo Handa wrote:
>> On 2025/05/08 17:25, John Johansen wrote:
>>> That is fine. But curious I am curious what the interface would look like to fit TOMOYO's
>>> needs.
>>
>> Stream (like "FILE *") with restart from the beginning (like rewind(fp)) support.
>> That is, the caller can read/write at least one byte at a time, and written data
>> is processed upon encountering '\n'.
>>
> 
> that can be emulated within the current sycall, where the lsm maintains a buffer.

That cannot be emulated, for there is no event that is automatically triggered when
the process terminates (i.e. implicit close() upon exit()) in order to release the
buffer the LSM maintains.

> Are you asking to also read data back out as well, that could be added, but doing
> a syscall per byte here or through the fs is going to have fairly high overhead.

At least one byte means arbitrary bytes; that is, the caller does not need to read
or write the whole policy at one syscall.

> 
> Without understanding the requirement it would seem to me, that it would be
> better to emulate that file buffer manipulation in userspace similar say C++
> stringstreams, and then write the syscall when done.

The size of the whole policy in byte varies a lot.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08  8:29       ` John Johansen
@ 2025-05-08 16:54         ` Casey Schaufler
  2025-05-09 10:26           ` Mickaël Salaün
  0 siblings, 1 reply; 36+ messages in thread
From: Casey Schaufler @ 2025-05-08 16:54 UTC (permalink / raw)
  To: John Johansen, Paul Moore, Maxime Bélair
  Cc: Tetsuo Handa, linux-security-module, jmorris, serge, mic, kees,
	stephen.smalley.work, takedakn, linux-api, apparmor, linux-kernel,
	Casey Schaufler

On 5/8/2025 1:29 AM, John Johansen wrote:
> On 5/7/25 13:25, Paul Moore wrote:
>> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
>> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>> On 2025/05/06 23:32, Maxime Bélair wrote:
>>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>>>> index dcaad8818679..b39e6635a7d5 100644
>>>> --- a/security/lsm_syscalls.c
>>>> +++ b/security/lsm_syscalls.c
>>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user
>>>> *, ids, u32 __user *, size,
>>>>   SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void
>>>> __user *, buf, u32
>>>>                __user *, size, u32, flags)
>>>>   {
>>>> -     return 0;
>>>> +     size_t usize;
>>>> +
>>>> +     if (get_user(usize, size))
>>>> +             return -EFAULT;
>>>> +
>>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize,
>>>> flags);
>>>>   }
>>>
>>> syzbot will report user-controlled unbounded huge size memory
>>> allocation attempt. ;-)
>>>
>>> This interface might be fine for AppArmor, but TOMOYO won't use this
>>> interface because
>>> TOMOYO's policy is line-oriented ASCII text data where the
>>> destination is switched via
>>> pseudo‑filesystem's filename ...
>>
>> While Tetsuo's comment is limited to TOMOYO, I believe the argument
>> applies to a number of other LSMs as well.  The reality is that there
>> is no one policy ideal shared across LSMs and that complicates things
>> like the lsm_manage_policy() proposal.  I'm intentionally saying
>> "complicates" and not "prevents" because I don't want to flat out
>> reject something like this, but I think there needs to be a larger
>> discussion among the different LSM groups about what such an API
>> should look like.  We may not need to get every LSM to support this
>> new API, but we need to get something that would work for a
>> significant majority and would be general/extensible enough that we
>> would expect it to work with the majority of future LSMs (as much as
>> we can predict the future anyway).
>>
>
> yep, I look at this is just a starting point for discussion. There
> isn't going to be any discussion without some code, so here is a v1
> that supports a single LSM let the bike shedding begin.

Aside from the issues with allocating a buffer for a big policy
I don't see a problem with this proposal. The system call looks
a lot like the other LSM interfaces, so any developer who likes
those ought to like this one. The infrastructure can easily check
the lsm_id and only call the appropriate LSM hook, so no one
is going to be interfering with other modules.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08 15:07           ` Tetsuo Handa
@ 2025-05-09  3:25             ` John Johansen
  0 siblings, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-09  3:25 UTC (permalink / raw)
  To: Tetsuo Handa, Maxime Bélair, linux-security-module
  Cc: paul, jmorris, serge, mic, kees, stephen.smalley.work, casey,
	takedakn, linux-api, apparmor, linux-kernel

On 5/8/25 08:07, Tetsuo Handa wrote:
> On 2025/05/08 23:44, John Johansen wrote:
>> On 5/8/25 05:55, Tetsuo Handa wrote:
>>> On 2025/05/08 17:25, John Johansen wrote:
>>>> That is fine. But curious I am curious what the interface would look like to fit TOMOYO's
>>>> needs.
>>>
>>> Stream (like "FILE *") with restart from the beginning (like rewind(fp)) support.
>>> That is, the caller can read/write at least one byte at a time, and written data
>>> is processed upon encountering '\n'.
>>>
>>
>> that can be emulated within the current sycall, where the lsm maintains a buffer.
> 
> That cannot be emulated, for there is no event that is automatically triggered when
> the process terminates (i.e. implicit close() upon exit()) in order to release the
> buffer the LSM maintains.
>

security_task_free()
  
>> Are you asking to also read data back out as well, that could be added, but doing
>> a syscall per byte here or through the fs is going to have fairly high overhead.
> 
> At least one byte means arbitrary bytes; that is, the caller does not need to read
> or write the whole policy at one syscall.
> 
got it

>>
>> Without understanding the requirement it would seem to me, that it would be
>> better to emulate that file buffer manipulation in userspace similar say C++
>> stringstreams, and then write the syscall when done.
> 
> The size of the whole policy in byte varies a lot.
> 
sure, buffers can be variable length. AppArmor policy also varies a lot in size.

More than anything I am trying to understand TOMOYO's requirements. They do
align better with using an fs interface. Can they be met sure, but it would
be more work for TOMOYO.

One of the big motivations for the syscall from the apparmor side is getting
away from the need to have the vfs present or having to pass an fd into the
environment.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-08  7:52         ` John Johansen
@ 2025-05-09 10:25           ` Mickaël Salaün
  2025-05-11 11:09             ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Mickaël Salaün @ 2025-05-09 10:25 UTC (permalink / raw)
  To: John Johansen
  Cc: Tetsuo Handa, Maxime Bélair, Song Liu, linux-security-module,
	paul, jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	linux-api, apparmor, linux-kernel, Arnd Bergmann

On Thu, May 08, 2025 at 12:52:55AM -0700, John Johansen wrote:
> On 5/7/25 15:04, Tetsuo Handa wrote:
> > On 2025/05/08 0:37, Maxime Bélair wrote:
> > > Again, each module decides which operations to expose through this syscall. In many cases
> > > the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
> > > that choose this interface remain secure while gaining its advantages.
> > 
> > If the interpretation of "flags" argument varies across LSMs, it sounds like ioctl()'s
> 
> yes that does feel like ioctls(), on the other hand defining them at the LSM level won't
> offer LSMs flexibility making it so the syscall covers fewer use cases. I am not opposed
> to either, it just hashing out what people want, and what is acceptable.
> 
> > "cmd" argument. Also, there is prctl() which can already carry string-ish parameters
> > without involving open(). Why can't we use prctl() instead of lsm_manage_policy() ?
> > 
> 
> prctl() can be used, I used it for the unprivileged policy demo. It has its own set of
> problems. While LSM policy could be associated with the process doing the load/replacement
> or what ever operation, it isn't necessarily tied to it. A lot of LSM policy is not
> process specific making prctl() a poor fit.
> 
> prctl() requires allocating a global prctl()
> 
> prctl() are already being filtered/controlled by LSMs making them a poort fit for
> use by an LSM in a stacking situation as it requires updating the policy of other
> LSMs on the system. Yes seccomp can filter the syscall but that still is an easier
> barrier to overcome than having to have instruction for how to allow your LSMs
> prctl() in multiple LSMs.
> 
> 
> Mickaël already argued the need for landlock to have syscalls. See

Landlock indeed requires syscalls mainly because of its unprivileged
nature.

> https://lore.kernel.org/lkml/20200511192156.1618284-7-mic@digikod.net/
> and the numerous iterations before that.

This link might be misleading though, it points to an initial version of
the syscall proposal (v17) and it was then decided to create one syscall
per operation (v34), which is why we ended with 3 syscalls.  See the
changelog:
https://lore.kernel.org/r/20210422154123.13086-9-mic@digikod.net

> 
> Ideally those could have been LSM syscalls, with landlock leveraging them.

I don't agree.  The Landlock syscalls have a well-defined semantic, with
documented security requirements, and they deal with specific kernel
objects identified with file descriptors, including a dedicated one:
[landlock-ruleset].  For the features provided by these Landlock
syscalls, it would not have been a good idea to reuse existing syscalls,
nor to rely on the syscall proposed in this series because the interface
is too specific to some of the current privileged LSMs (i.e. ingest a
policy blob).  Making this interface more generic would lead to even
less defined semantic though.

> AppArmor
> is getting to where it has similar needs to landlock. Yes we can use ioctls, prctls,
> netlink, the fs, etc. it doesn't mean that those are the best interfaces to do so,

I think it would make sense to propose AppArmor-specific syscalls.

> and ideally any interface we use will be of benefit to some other LSMs in the future.

The LSM syscalls may make sense to deal with LSM blobs managed by the
LSM framework (e.g. get/set properties) when the operations are
common/generic.

Security policies are specific to each LSM and they should implement
their own well-defined interface (e.g. filesystem, netlink, syscall).

The LSM framework doesn't provide nor manage any security policy, it
mainly provides a set of consistent and well-defined kernel hooks with
security blobs to enforce a security policy.  I don't think it makes
sense to add LSM syscalls to manage things not managed by the LSM
framework.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-08  8:18         ` John Johansen
@ 2025-05-09 10:26           ` Mickaël Salaün
  2025-05-11 10:47             ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Mickaël Salaün @ 2025-05-09 10:26 UTC (permalink / raw)
  To: John Johansen
  Cc: Song Liu, Maxime Bélair, linux-security-module, paul,
	jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	penguin-kernel, linux-api, apparmor, linux-kernel, Arnd Bergmann

On Thu, May 08, 2025 at 01:18:20AM -0700, John Johansen wrote:
> On 5/7/25 23:06, Song Liu wrote:
> > On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
> > <maxime.belair@canonical.com> wrote:
> > [...]
> > > > 
> > > > These two do not feel like real benefits:
> > > > - One syscall cannot fit all use cases well...
> > > 
> > > This syscall is not intended to cover every case, nor to replace existing kernel
> > > interfaces.
> > > 
> > > Each LSM can decide which operations it wants to support (if any). For example, when
> > > loading policies, an LSM may choose to allow only policies that further restrict
> > > privileges.
> > > 
> > > > - Not working in containers is often not an issue, but a feature.
> > > 
> > > Indeed, using this syscall requires appropriate capabilities and will not permit
> > > unprivileged containers to manage policies arbitrarily.
> > > 
> > > With this syscall, capability checks remain the responsibility of each LSM.
> > > 
> > > For instance, in the AppArmor patch, a profile can be loaded only if
> > > aa_policy_admin_capable() succeeds (which requires CAP_MAC_ADMIN). Moreover, by design,
> > > policies can be loaded only in the current namespace.
> > > 
> > > I see this syscall as a middle point between exposing the entire sysfs, creating a large
> > > attack surface, and blocking everything.
> > > 
> > > Landlock’s existing syscalls already improve security by allowing processes to further
> > > restrict their ambient rights while adding only a modest attack surface.
> > > 
> > > This syscall is a further step in that direction: it lets LSMs add restrictive policies
> > > without requiring exposing every other interface.
> > 
> > I don't think a syscall makes the API more secure. If necessary, we can add
> 
> It exposes a different attack surface. Requiring mounting of the fs to where it is visible
> in the container, provides attack surface, and requires additional external configuration.

We should also keep in mind that syscalls could be accessible from
everywhere, by everyone, which may increase the attack surface compared
to a privileged filesystem interface.  Adding a second interface may
also introduce issues.  Anyway, I'm definitely not against syscalls, but
I don't see why the filesystem interface would be "less secure" in this
context.

> 
> Then there is the whole issue of getting the various LSMs to allow another LSM in the
> stack to be able manage its own policy.

Right, and it's a similar issue with seccomp policies wrt syscalls.

> 
> > permission check to each pseudo file. The downside of the syscall, however,
> > is that all the permission checks are hard-coded in the kernel (except for
> 
> The permission checks don't have to be hard coded. Each LSM can define how it handles
> or manages the syscall. The default is that it isn't supported, but if an lsm decides
> to support it, there is now reason that its policy can't determine the use of the
> syscall.

From an interface design point of view, it would be better to clearly
specify the scope of a command (e.g. which components could be impacted
by a command), and make sure the documentation reflect that as well.
Even better, have a syscalls per required privileges and impact (e.g.
privileged or unprivileged).  Going this road, I'm not sure if a
privileged syscall would make sense given the existing filesystem
interface.

> 
> > BPF LSM); while the sys admin can configure permissions of the pseudo
> > files in user space.
> > 
> Other LSMs also have policy that can control access to pseudo filesystems and
> other resources. Again, the control doesn't have to be hard coded. And seccomp can
> be used to block the syscall.
> 
> 
> 
> > > Again, each module decides which operations to expose through this syscall. In many cases
> > > the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
> > > that choose this interface remain secure while gaining its advantages.
> > > 
> > > > >    - Avoids overhead of other kernel interfaces for better efficiency
> > > > 
> > > > .. and it is is probably less efficient, because everything need to
> > > > fit in the same API.
> > > 
> > > As shown below, the syscall can significantly improve the performance of policy management.
> > > A more detailed benchmark is available in [1].
> > > 
> > > The following table presents the time required to load an AppArmor profile.
> > > 
> > > For every cell, the first value is the total time taken by aa-load, and the value in
> > > parentheses is the time spent to load the policy in the kernel only (total - dry‑run).
> > > 
> > > Results are in microseconds and are averaged over 10 000 runs to reduce variance.
> > > 
> > > 
> > > | t (µs)    | syscall     | pseudofs    | Speedup       |
> > > |-----------|-------------|-------------|---------------|
> > > | 1password | 4257 (1127) | 3333 (192)  | x1.28 (x5.86) |
> > > | Xorg      | 6099 (2961) | 5167 (2020) | x1.18 (x1.47) |
> > > 
> > 
> > I am not sure the performance of loading security policies is on any
> > critical path.
> 
> generally speaking I agree, but I am also not going to turn down a
> performance improvement either. Its a nice to have, but not a strong
> argument for need.
> 
> > The implementation calls the hook for each LSM, which is why I think the
> > syscall is not efficient.
> > 
> it should only call the LSM identified by the lsmid in the call.
> 
> > Overall, I am still not convinced a syscall for all LSMs is needed. To
> > justify such
> 
> its not needed by all LSMs, just a subset of them, and some nebulous
> subset of potentially future LSMs that is entirely undefinable.
> 
> If we had had appropriate LSM syscalls landlock wouldn't have needed
> to have landlock specific syscalls. Having another LSM go that route
> feels wrong especially now that we have some LSM syscalls.

I don't agree.  Dedicated syscalls are a good thing.  See my other
reply.

> If a
> syscall is needed by an LSM its better to try hashing something out
> that might have utility for multiple LSMs or at the very least,
> potentially have utility in the future.
> 
> 
> > a syscall, I think we need to show that it is useful in multiple LSMs.
> > Also, if we
> > really want to have single set of APIs for all LSMs, we may also need
> > get_policy,
> 
> We are never going to get a single set of APIs for all LSMs. I will
> settle for an api that has utility for a subset
> 
> > remove_policy, etc. This set as-is appears to be an incomplete design. The
> 
> To have a complete design, there needs to be feedback and discussion
> from multiple LSMs. This is a starting point.
> 
> > implementation, with call_int_hook, is also problematic. It can easily
> > cause some> controversial behaviors.
> > 
> agreed it shouldn't be doing a straight call_int_hook, it should only
> call it against the lsm identified by the lsmid

Yes, but then, I don't see the point of a "generic" LSM syscall.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-08 16:54         ` Casey Schaufler
@ 2025-05-09 10:26           ` Mickaël Salaün
  2025-05-09 14:21             ` Casey Schaufler
  2025-05-11 11:20             ` John Johansen
  0 siblings, 2 replies; 36+ messages in thread
From: Mickaël Salaün @ 2025-05-09 10:26 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: John Johansen, Paul Moore, Maxime Bélair, Tetsuo Handa,
	linux-security-module, jmorris, serge, kees, stephen.smalley.work,
	takedakn, linux-api, apparmor, linux-kernel, Arnd Bergmann

On Thu, May 08, 2025 at 09:54:19AM -0700, Casey Schaufler wrote:
> On 5/8/2025 1:29 AM, John Johansen wrote:
> > On 5/7/25 13:25, Paul Moore wrote:
> >> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
> >> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >>> On 2025/05/06 23:32, Maxime Bélair wrote:
> >>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> >>>> index dcaad8818679..b39e6635a7d5 100644
> >>>> --- a/security/lsm_syscalls.c
> >>>> +++ b/security/lsm_syscalls.c
> >>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user
> >>>> *, ids, u32 __user *, size,
> >>>>   SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void
> >>>> __user *, buf, u32
> >>>>                __user *, size, u32, flags)
> >>>>   {
> >>>> -     return 0;
> >>>> +     size_t usize;
> >>>> +
> >>>> +     if (get_user(usize, size))
> >>>> +             return -EFAULT;
> >>>> +
> >>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize,
> >>>> flags);
> >>>>   }
> >>>
> >>> syzbot will report user-controlled unbounded huge size memory
> >>> allocation attempt. ;-)
> >>>
> >>> This interface might be fine for AppArmor, but TOMOYO won't use this
> >>> interface because
> >>> TOMOYO's policy is line-oriented ASCII text data where the
> >>> destination is switched via
> >>> pseudo‑filesystem's filename ...
> >>
> >> While Tetsuo's comment is limited to TOMOYO, I believe the argument
> >> applies to a number of other LSMs as well.  The reality is that there
> >> is no one policy ideal shared across LSMs and that complicates things
> >> like the lsm_manage_policy() proposal.  I'm intentionally saying
> >> "complicates" and not "prevents" because I don't want to flat out
> >> reject something like this, but I think there needs to be a larger
> >> discussion among the different LSM groups about what such an API
> >> should look like.  We may not need to get every LSM to support this
> >> new API, but we need to get something that would work for a
> >> significant majority and would be general/extensible enough that we
> >> would expect it to work with the majority of future LSMs (as much as
> >> we can predict the future anyway).
> >>
> >
> > yep, I look at this is just a starting point for discussion. There
> > isn't going to be any discussion without some code, so here is a v1
> > that supports a single LSM let the bike shedding begin.
> 
> Aside from the issues with allocating a buffer for a big policy
> I don't see a problem with this proposal. The system call looks
> a lot like the other LSM interfaces, so any developer who likes
> those ought to like this one. The infrastructure can easily check
> the lsm_id and only call the appropriate LSM hook, so no one
> is going to be interfering with other modules.

We may not want to only be able to load buffers containing policies, but
also to leverage file descriptors like Landlock does.  Getting a
property from a kernel object or updating it is mainly about dealing
with a buffer.  And the current LSM syscalls do just that.  Other kind
of operations may require more than that though.

I don't like multiplexer syscalls because they don't expose a clear
semantic and can be complex to manage and filter.  This new syscall is
kind of a multiplexer that redirect commands to an arbitrary set of
kernel parts, which can then define their own semantic.  I'd like to see
a clear set of well-defined operations and their required permission.
Even better, one syscall per operation should simplify their interface.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-09 10:26           ` Mickaël Salaün
@ 2025-05-09 14:21             ` Casey Schaufler
  2025-05-11 11:26               ` John Johansen
  2025-05-11 11:20             ` John Johansen
  1 sibling, 1 reply; 36+ messages in thread
From: Casey Schaufler @ 2025-05-09 14:21 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: John Johansen, Paul Moore, Maxime Bélair, Tetsuo Handa,
	linux-security-module, jmorris, serge, kees, stephen.smalley.work,
	takedakn, linux-api, apparmor, linux-kernel, Arnd Bergmann,
	Casey Schaufler

On 5/9/2025 3:26 AM, Mickaël Salaün wrote:
> On Thu, May 08, 2025 at 09:54:19AM -0700, Casey Schaufler wrote:
>> On 5/8/2025 1:29 AM, John Johansen wrote:
>>> On 5/7/25 13:25, Paul Moore wrote:
>>>> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
>>>> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>>>> On 2025/05/06 23:32, Maxime Bélair wrote:
>>>>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>>>>>> index dcaad8818679..b39e6635a7d5 100644
>>>>>> --- a/security/lsm_syscalls.c
>>>>>> +++ b/security/lsm_syscalls.c
>>>>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user
>>>>>> *, ids, u32 __user *, size,
>>>>>>   SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void
>>>>>> __user *, buf, u32
>>>>>>                __user *, size, u32, flags)
>>>>>>   {
>>>>>> -     return 0;
>>>>>> +     size_t usize;
>>>>>> +
>>>>>> +     if (get_user(usize, size))
>>>>>> +             return -EFAULT;
>>>>>> +
>>>>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize,
>>>>>> flags);
>>>>>>   }
>>>>> syzbot will report user-controlled unbounded huge size memory
>>>>> allocation attempt. ;-)
>>>>>
>>>>> This interface might be fine for AppArmor, but TOMOYO won't use this
>>>>> interface because
>>>>> TOMOYO's policy is line-oriented ASCII text data where the
>>>>> destination is switched via
>>>>> pseudo‑filesystem's filename ...
>>>> While Tetsuo's comment is limited to TOMOYO, I believe the argument
>>>> applies to a number of other LSMs as well.  The reality is that there
>>>> is no one policy ideal shared across LSMs and that complicates things
>>>> like the lsm_manage_policy() proposal.  I'm intentionally saying
>>>> "complicates" and not "prevents" because I don't want to flat out
>>>> reject something like this, but I think there needs to be a larger
>>>> discussion among the different LSM groups about what such an API
>>>> should look like.  We may not need to get every LSM to support this
>>>> new API, but we need to get something that would work for a
>>>> significant majority and would be general/extensible enough that we
>>>> would expect it to work with the majority of future LSMs (as much as
>>>> we can predict the future anyway).
>>>>
>>> yep, I look at this is just a starting point for discussion. There
>>> isn't going to be any discussion without some code, so here is a v1
>>> that supports a single LSM let the bike shedding begin.
>> Aside from the issues with allocating a buffer for a big policy
>> I don't see a problem with this proposal. The system call looks
>> a lot like the other LSM interfaces, so any developer who likes
>> those ought to like this one. The infrastructure can easily check
>> the lsm_id and only call the appropriate LSM hook, so no one
>> is going to be interfering with other modules.
> We may not want to only be able to load buffers containing policies, but
> also to leverage file descriptors like Landlock does.  Getting a
> property from a kernel object or updating it is mainly about dealing
> with a buffer.  And the current LSM syscalls do just that.  Other kind
> of operations may require more than that though.
>
> I don't like multiplexer syscalls because they don't expose a clear
> semantic and can be complex to manage and filter.  This new syscall is
> kind of a multiplexer that redirect commands to an arbitrary set of
> kernel parts, which can then define their own semantic.  I'd like to see
> a clear set of well-defined operations and their required permission.
> Even better, one syscall per operation should simplify their interface.

The development and maintenance of system calls is expensive in both
time and effort. LSM specific system calls frighten me. When I was
young adding system calls was just  not  done. A system call would
never be allowed for a specific sub-system or optional feature. True,
there are issues with the LSM specific filesystem approach. But I
like it, as it allows the LSM more freedom in its interfaces and
won't clutter the API if the LSM goes away or quits using it.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-09 10:26           ` Mickaël Salaün
@ 2025-05-11 10:47             ` John Johansen
  2025-05-12 10:20               ` Mickaël Salaün
  0 siblings, 1 reply; 36+ messages in thread
From: John Johansen @ 2025-05-11 10:47 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Song Liu, Maxime Bélair, linux-security-module, paul,
	jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	penguin-kernel, linux-api, apparmor, linux-kernel, Arnd Bergmann

On 5/9/25 03:26, Mickaël Salaün wrote:
> On Thu, May 08, 2025 at 01:18:20AM -0700, John Johansen wrote:
>> On 5/7/25 23:06, Song Liu wrote:
>>> On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
>>> <maxime.belair@canonical.com> wrote:
>>> [...]
>>>>>
>>>>> These two do not feel like real benefits:
>>>>> - One syscall cannot fit all use cases well...
>>>>
>>>> This syscall is not intended to cover every case, nor to replace existing kernel
>>>> interfaces.
>>>>
>>>> Each LSM can decide which operations it wants to support (if any). For example, when
>>>> loading policies, an LSM may choose to allow only policies that further restrict
>>>> privileges.
>>>>
>>>>> - Not working in containers is often not an issue, but a feature.
>>>>
>>>> Indeed, using this syscall requires appropriate capabilities and will not permit
>>>> unprivileged containers to manage policies arbitrarily.
>>>>
>>>> With this syscall, capability checks remain the responsibility of each LSM.
>>>>
>>>> For instance, in the AppArmor patch, a profile can be loaded only if
>>>> aa_policy_admin_capable() succeeds (which requires CAP_MAC_ADMIN). Moreover, by design,
>>>> policies can be loaded only in the current namespace.
>>>>
>>>> I see this syscall as a middle point between exposing the entire sysfs, creating a large
>>>> attack surface, and blocking everything.
>>>>
>>>> Landlock’s existing syscalls already improve security by allowing processes to further
>>>> restrict their ambient rights while adding only a modest attack surface.
>>>>
>>>> This syscall is a further step in that direction: it lets LSMs add restrictive policies
>>>> without requiring exposing every other interface.
>>>
>>> I don't think a syscall makes the API more secure. If necessary, we can add
>>
>> It exposes a different attack surface. Requiring mounting of the fs to where it is visible
>> in the container, provides attack surface, and requires additional external configuration.
> 
> We should also keep in mind that syscalls could be accessible from
> everywhere, by everyone, which may increase the attack surface compared
> to a privileged filesystem interface.  Adding a second interface may
> also introduce issues.  Anyway, I'm definitely not against syscalls, but
> I don't see why the filesystem interface would be "less secure" in this
> context.
> 

yes syscalls being accessible from everywhere is another form of attack
surface, that needs to be mediated.

the fs can be mediated, its expose is a multiple lsms with multiple
different interfaces on the files within it. What really is more
problematic is makng the fs available in the container. Yes a
container manager can do it but then you are dependent on the
container manager making your interface available.

Other wise you are looking at making mount available to your app
within the container.

>>
>> Then there is the whole issue of getting the various LSMs to allow another LSM in the
>> stack to be able manage its own policy.
> 
> Right, and it's a similar issue with seccomp policies wrt syscalls.
> 
yes, though seccomp I have found to be the easier one to deal with

>>
>>> permission check to each pseudo file. The downside of the syscall, however,
>>> is that all the permission checks are hard-coded in the kernel (except for
>>
>> The permission checks don't have to be hard coded. Each LSM can define how it handles
>> or manages the syscall. The default is that it isn't supported, but if an lsm decides
>> to support it, there is now reason that its policy can't determine the use of the
>> syscall.
> 
>  From an interface design point of view, it would be better to clearly
> specify the scope of a command (e.g. which components could be impacted
> by a command), and make sure the documentation reflect that as well.
> Even better, have a syscalls per required privileges and impact (e.g.
> privileged or unprivileged).  Going this road, I'm not sure if a
> privileged syscall would make sense given the existing filesystem
> interface.
> 

uhhhmmm, not just privileged. As you well know we are looking to use
this for unprivileged policy. The LSM can limit to privileged if it
wants but it doesn't have to limit it to privileged policy.

>>
>>> BPF LSM); while the sys admin can configure permissions of the pseudo
>>> files in user space.
>>>
>> Other LSMs also have policy that can control access to pseudo filesystems and
>> other resources. Again, the control doesn't have to be hard coded. And seccomp can
>> be used to block the syscall.
>>
>>
>>
>>>> Again, each module decides which operations to expose through this syscall. In many cases
>>>> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
>>>> that choose this interface remain secure while gaining its advantages.
>>>>
>>>>>>     - Avoids overhead of other kernel interfaces for better efficiency
>>>>>
>>>>> .. and it is is probably less efficient, because everything need to
>>>>> fit in the same API.
>>>>
>>>> As shown below, the syscall can significantly improve the performance of policy management.
>>>> A more detailed benchmark is available in [1].
>>>>
>>>> The following table presents the time required to load an AppArmor profile.
>>>>
>>>> For every cell, the first value is the total time taken by aa-load, and the value in
>>>> parentheses is the time spent to load the policy in the kernel only (total - dry‑run).
>>>>
>>>> Results are in microseconds and are averaged over 10 000 runs to reduce variance.
>>>>
>>>>
>>>> | t (µs)    | syscall     | pseudofs    | Speedup       |
>>>> |-----------|-------------|-------------|---------------|
>>>> | 1password | 4257 (1127) | 3333 (192)  | x1.28 (x5.86) |
>>>> | Xorg      | 6099 (2961) | 5167 (2020) | x1.18 (x1.47) |
>>>>
>>>
>>> I am not sure the performance of loading security policies is on any
>>> critical path.
>>
>> generally speaking I agree, but I am also not going to turn down a
>> performance improvement either. Its a nice to have, but not a strong
>> argument for need.
>>
>>> The implementation calls the hook for each LSM, which is why I think the
>>> syscall is not efficient.
>>>
>> it should only call the LSM identified by the lsmid in the call.
>>
>>> Overall, I am still not convinced a syscall for all LSMs is needed. To
>>> justify such
>>
>> its not needed by all LSMs, just a subset of them, and some nebulous
>> subset of potentially future LSMs that is entirely undefinable.
>>
>> If we had had appropriate LSM syscalls landlock wouldn't have needed
>> to have landlock specific syscalls. Having another LSM go that route
>> feels wrong especially now that we have some LSM syscalls.
> 
> I don't agree.  Dedicated syscalls are a good thing.  See my other
> reply.
> 

I think we can just disagree on this point.

>> If a
>> syscall is needed by an LSM its better to try hashing something out
>> that might have utility for multiple LSMs or at the very least,
>> potentially have utility in the future.
>>
>>
>>> a syscall, I think we need to show that it is useful in multiple LSMs.
>>> Also, if we
>>> really want to have single set of APIs for all LSMs, we may also need
>>> get_policy,
>>
>> We are never going to get a single set of APIs for all LSMs. I will
>> settle for an api that has utility for a subset
>>
>>> remove_policy, etc. This set as-is appears to be an incomplete design. The
>>
>> To have a complete design, there needs to be feedback and discussion
>> from multiple LSMs. This is a starting point.
>>
>>> implementation, with call_int_hook, is also problematic. It can easily
>>> cause some> controversial behaviors.
>>>
>> agreed it shouldn't be doing a straight call_int_hook, it should only
>> call it against the lsm identified by the lsmid
> 
> Yes, but then, I don't see the point of a "generic" LSM syscall.

its not a generic LSM syscall. Its a syscall or maybe a set of syscalls
for a specific scoped problem of loading/managing policy.

Can we come to something acceptable? I don't know but we are going to
look at it before trying for an apparmor specific syscall.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-09 10:25           ` Mickaël Salaün
@ 2025-05-11 11:09             ` John Johansen
  0 siblings, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-11 11:09 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Tetsuo Handa, Maxime Bélair, Song Liu, linux-security-module,
	paul, jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	linux-api, apparmor, linux-kernel, Arnd Bergmann

On 5/9/25 03:25, Mickaël Salaün wrote:
> On Thu, May 08, 2025 at 12:52:55AM -0700, John Johansen wrote:
>> On 5/7/25 15:04, Tetsuo Handa wrote:
>>> On 2025/05/08 0:37, Maxime Bélair wrote:
>>>> Again, each module decides which operations to expose through this syscall. In many cases
>>>> the operation will still require CAP_SYS_ADMIN or a similar capability, so environments
>>>> that choose this interface remain secure while gaining its advantages.
>>>
>>> If the interpretation of "flags" argument varies across LSMs, it sounds like ioctl()'s
>>
>> yes that does feel like ioctls(), on the other hand defining them at the LSM level won't
>> offer LSMs flexibility making it so the syscall covers fewer use cases. I am not opposed
>> to either, it just hashing out what people want, and what is acceptable.
>>
>>> "cmd" argument. Also, there is prctl() which can already carry string-ish parameters
>>> without involving open(). Why can't we use prctl() instead of lsm_manage_policy() ?
>>>
>>
>> prctl() can be used, I used it for the unprivileged policy demo. It has its own set of
>> problems. While LSM policy could be associated with the process doing the load/replacement
>> or what ever operation, it isn't necessarily tied to it. A lot of LSM policy is not
>> process specific making prctl() a poor fit.
>>
>> prctl() requires allocating a global prctl()
>>
>> prctl() are already being filtered/controlled by LSMs making them a poort fit for
>> use by an LSM in a stacking situation as it requires updating the policy of other
>> LSMs on the system. Yes seccomp can filter the syscall but that still is an easier
>> barrier to overcome than having to have instruction for how to allow your LSMs
>> prctl() in multiple LSMs.
>>
>>
>> Mickaël already argued the need for landlock to have syscalls. See
> 
> Landlock indeed requires syscalls mainly because of its unprivileged
> nature.
> 

yes that is the dominant reason

>> https://lore.kernel.org/lkml/20200511192156.1618284-7-mic@digikod.net/
>> and the numerous iterations before that.
> 
> This link might be misleading though, it points to an initial version of
> the syscall proposal (v17) and it was then decided to create one syscall
> per operation (v34), which is why we ended with 3 syscalls.  See the
> changelog:
> https://lore.kernel.org/r/20210422154123.13086-9-mic@digikod.net
> 

yes and no. I am well aware landlock's syscall got split into three syscalls.

All I was trying to do is reference to the start of the discussion on why
landlock needed a syscall(s). I thought the details of why you have three
etc, really didn't add to the discussion. But yeah not also pointing to
v34 could be considered misleading.


>>
>> Ideally those could have been LSM syscalls, with landlock leveraging them.
> 
> I don't agree.  The Landlock syscalls have a well-defined semantic, with

First I don't begrudge Landlock its syscalls, I think at the time it was
the only way forward.

> documented security requirements, and they deal with specific kernel
> objects identified with file descriptors, including a dedicated one:
> [landlock-ruleset].

I am aware. Those semantics could have been kept and documented, within
a set of LSM syscalls. Yes landlock's syscalls shouldn't have been done
behind a single LSM syscall, I am not advocating for that but maybe
behind several LSM syscalls.

>  For the features provided by these Landlock
> syscalls, it would not have been a good idea to reuse existing syscalls,
> nor to rely on the syscall proposed in this series because the interface
> is too specific to some of the current privileged LSMs (i.e. ingest a
> policy blob).  Making this interface more generic would lead to even
> less defined semantic though.

Right, so again not a generic LSM syscall. But "generic" LSM syscalls
for certain purposes. Let me walk my statement back a little, what I
find unfortunate was that the landlock LSM syscalls didn't get discussed
as a set of generic LSM syscall's with landlock being the first to
implement them.

The question is hashing out where the generic semantics are vs. the
individual LSMs. Having an LSM syscall to deal with specific kernel
objects idenetified with file descriptors, and allowing each LSMs
to deal with that if it needs is possible.

Its a matter of figuring something out. It could be it turns out it is
not worth it. And some individual LSM syscalls like landlocks are the
way to go, its that it wasn't explored. I don't fault you, and think
it really wasn't even an option at the time.

> 
>> AppArmor
>> is getting to where it has similar needs to landlock. Yes we can use ioctls, prctls,
>> netlink, the fs, etc. it doesn't mean that those are the best interfaces to do so,
> 
> I think it would make sense to propose AppArmor-specific syscalls.
> 

that may be the case, but I think we should explore providing a more
LSM generic interface first.

>> and ideally any interface we use will be of benefit to some other LSMs in the future.
> 
> The LSM syscalls may make sense to deal with LSM blobs managed by the
> LSM framework (e.g. get/set properties) when the operations are
> common/generic.
> 
> Security policies are specific to each LSM and they should implement
> their own well-defined interface (e.g. filesystem, netlink, syscall).
> 
policies at some level are just blobs too. It is worth at least
exploring whether there can be a common interface.

> The LSM framework doesn't provide nor manage any security policy, it
> mainly provides a set of consistent and well-defined kernel hooks with
> security blobs to enforce a security policy.  I don't think it makes
> sense to add LSM syscalls to manage things not managed by the LSM
> framework.

we aren't talking about the LSM framework managing security policy,
just whether it makes sense for it to provide a common interface that
an LSM can choose to use to provide it a blob of policy that it
can then manage.

Its just a mechanism. This isn't all that different than using the
filesystem, netlink, or other mechanisms to shuttle the blob
between userspace to the kernel, and then the LSM manages its
policy and data.

The big difference is that using the syscall opens unprivileged
policy up to the LSM more broadly. If we are going to go the syscall
route for apparmor, we might as well see if we can't make that
mechanism more broadly available, and make it easier for other
LSMs in the future.

Again, it might turn out its a fools errand, and we have to do
an apparmor specific syscall, but it is worth exploring first.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-09 10:26           ` Mickaël Salaün
  2025-05-09 14:21             ` Casey Schaufler
@ 2025-05-11 11:20             ` John Johansen
  1 sibling, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-11 11:20 UTC (permalink / raw)
  To: Mickaël Salaün, Casey Schaufler
  Cc: Paul Moore, Maxime Bélair, Tetsuo Handa,
	linux-security-module, jmorris, serge, kees, stephen.smalley.work,
	takedakn, linux-api, apparmor, linux-kernel, Arnd Bergmann

On 5/9/25 03:26, Mickaël Salaün wrote:
> On Thu, May 08, 2025 at 09:54:19AM -0700, Casey Schaufler wrote:
>> On 5/8/2025 1:29 AM, John Johansen wrote:
>>> On 5/7/25 13:25, Paul Moore wrote:
>>>> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
>>>> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>>>> On 2025/05/06 23:32, Maxime Bélair wrote:
>>>>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>>>>>> index dcaad8818679..b39e6635a7d5 100644
>>>>>> --- a/security/lsm_syscalls.c
>>>>>> +++ b/security/lsm_syscalls.c
>>>>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user
>>>>>> *, ids, u32 __user *, size,
>>>>>>    SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void
>>>>>> __user *, buf, u32
>>>>>>                 __user *, size, u32, flags)
>>>>>>    {
>>>>>> -     return 0;
>>>>>> +     size_t usize;
>>>>>> +
>>>>>> +     if (get_user(usize, size))
>>>>>> +             return -EFAULT;
>>>>>> +
>>>>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize,
>>>>>> flags);
>>>>>>    }
>>>>>
>>>>> syzbot will report user-controlled unbounded huge size memory
>>>>> allocation attempt. ;-)
>>>>>
>>>>> This interface might be fine for AppArmor, but TOMOYO won't use this
>>>>> interface because
>>>>> TOMOYO's policy is line-oriented ASCII text data where the
>>>>> destination is switched via
>>>>> pseudo‑filesystem's filename ...
>>>>
>>>> While Tetsuo's comment is limited to TOMOYO, I believe the argument
>>>> applies to a number of other LSMs as well.  The reality is that there
>>>> is no one policy ideal shared across LSMs and that complicates things
>>>> like the lsm_manage_policy() proposal.  I'm intentionally saying
>>>> "complicates" and not "prevents" because I don't want to flat out
>>>> reject something like this, but I think there needs to be a larger
>>>> discussion among the different LSM groups about what such an API
>>>> should look like.  We may not need to get every LSM to support this
>>>> new API, but we need to get something that would work for a
>>>> significant majority and would be general/extensible enough that we
>>>> would expect it to work with the majority of future LSMs (as much as
>>>> we can predict the future anyway).
>>>>
>>>
>>> yep, I look at this is just a starting point for discussion. There
>>> isn't going to be any discussion without some code, so here is a v1
>>> that supports a single LSM let the bike shedding begin.
>>
>> Aside from the issues with allocating a buffer for a big policy
>> I don't see a problem with this proposal. The system call looks
>> a lot like the other LSM interfaces, so any developer who likes
>> those ought to like this one. The infrastructure can easily check
>> the lsm_id and only call the appropriate LSM hook, so no one
>> is going to be interfering with other modules.
> 
> We may not want to only be able to load buffers containing policies, but
> also to leverage file descriptors like Landlock does.  Getting a

I am not opposed to a syscall that leverages file desriptors like landlock
but that would be a different syscall with different semantics, and
something for an lsm that wants that semantic to introduce.

> property from a kernel object or updating it is mainly about dealing
> with a buffer.  And the current LSM syscalls do just that.  Other kind
> of operations may require more than that though.
> 
sure but they don't do it for the semantic of loading/managing policy.

> I don't like multiplexer syscalls because they don't expose a clear
> semantic and can be complex to manage and filter.  This new syscall is
> kind of a multiplexer that redirect commands to an arbitrary set of
> kernel parts, which can then define their own semantic.  I'd like to see
> a clear set of well-defined operations and their required permission.
> Even better, one syscall per operation should simplify their interface.

I am not opposed to that approach. This can be multiple syscalls. Its
a v1 to try and see if we can come to any agreement on a set of semantics


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook
  2025-05-09 14:21             ` Casey Schaufler
@ 2025-05-11 11:26               ` John Johansen
  0 siblings, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-11 11:26 UTC (permalink / raw)
  To: Casey Schaufler, Mickaël Salaün
  Cc: Paul Moore, Maxime Bélair, Tetsuo Handa,
	linux-security-module, jmorris, serge, kees, stephen.smalley.work,
	takedakn, linux-api, apparmor, linux-kernel, Arnd Bergmann

On 5/9/25 07:21, Casey Schaufler wrote:
> On 5/9/2025 3:26 AM, Mickaël Salaün wrote:
>> On Thu, May 08, 2025 at 09:54:19AM -0700, Casey Schaufler wrote:
>>> On 5/8/2025 1:29 AM, John Johansen wrote:
>>>> On 5/7/25 13:25, Paul Moore wrote:
>>>>> On Wed, May 7, 2025 at 6:41 AM Tetsuo Handa
>>>>> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>>>>> On 2025/05/06 23:32, Maxime Bélair wrote:
>>>>>>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>>>>>>> index dcaad8818679..b39e6635a7d5 100644
>>>>>>> --- a/security/lsm_syscalls.c
>>>>>>> +++ b/security/lsm_syscalls.c
>>>>>>> @@ -122,5 +122,10 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user
>>>>>>> *, ids, u32 __user *, size,
>>>>>>>    SYSCALL_DEFINE5(lsm_manage_policy, u32, lsm_id, u32, op, void
>>>>>>> __user *, buf, u32
>>>>>>>                 __user *, size, u32, flags)
>>>>>>>    {
>>>>>>> -     return 0;
>>>>>>> +     size_t usize;
>>>>>>> +
>>>>>>> +     if (get_user(usize, size))
>>>>>>> +             return -EFAULT;
>>>>>>> +
>>>>>>> +     return security_lsm_manage_policy(lsm_id, op, buf, usize,
>>>>>>> flags);
>>>>>>>    }
>>>>>> syzbot will report user-controlled unbounded huge size memory
>>>>>> allocation attempt. ;-)
>>>>>>
>>>>>> This interface might be fine for AppArmor, but TOMOYO won't use this
>>>>>> interface because
>>>>>> TOMOYO's policy is line-oriented ASCII text data where the
>>>>>> destination is switched via
>>>>>> pseudo‑filesystem's filename ...
>>>>> While Tetsuo's comment is limited to TOMOYO, I believe the argument
>>>>> applies to a number of other LSMs as well.  The reality is that there
>>>>> is no one policy ideal shared across LSMs and that complicates things
>>>>> like the lsm_manage_policy() proposal.  I'm intentionally saying
>>>>> "complicates" and not "prevents" because I don't want to flat out
>>>>> reject something like this, but I think there needs to be a larger
>>>>> discussion among the different LSM groups about what such an API
>>>>> should look like.  We may not need to get every LSM to support this
>>>>> new API, but we need to get something that would work for a
>>>>> significant majority and would be general/extensible enough that we
>>>>> would expect it to work with the majority of future LSMs (as much as
>>>>> we can predict the future anyway).
>>>>>
>>>> yep, I look at this is just a starting point for discussion. There
>>>> isn't going to be any discussion without some code, so here is a v1
>>>> that supports a single LSM let the bike shedding begin.
>>> Aside from the issues with allocating a buffer for a big policy
>>> I don't see a problem with this proposal. The system call looks
>>> a lot like the other LSM interfaces, so any developer who likes
>>> those ought to like this one. The infrastructure can easily check
>>> the lsm_id and only call the appropriate LSM hook, so no one
>>> is going to be interfering with other modules.
>> We may not want to only be able to load buffers containing policies, but
>> also to leverage file descriptors like Landlock does.  Getting a
>> property from a kernel object or updating it is mainly about dealing
>> with a buffer.  And the current LSM syscalls do just that.  Other kind
>> of operations may require more than that though.
>>
>> I don't like multiplexer syscalls because they don't expose a clear
>> semantic and can be complex to manage and filter.  This new syscall is
>> kind of a multiplexer that redirect commands to an arbitrary set of
>> kernel parts, which can then define their own semantic.  I'd like to see
>> a clear set of well-defined operations and their required permission.
>> Even better, one syscall per operation should simplify their interface.
> 
> The development and maintenance of system calls is expensive in both
> time and effort. LSM specific system calls frighten me. When I was
> young adding system calls was just  not  done. A system call would
> never be allowed for a specific sub-system or optional feature. True,
> there are issues with the LSM specific filesystem approach. But I
> like it, as it allows the LSM more freedom in its interfaces and
> won't clutter the API if the LSM goes away or quits using it.
> 
I get the reticence on adding syscalls. Indeed its part of why I
want to explore LSM syscalls before going with an apparmor specific
syscall.

The current LSM specific fs approach has limitations that just can't
be reasonably worked around for some use cases, so that leaves going
with an alternate mechanism. For this use case, ioctls are problematic
like the fs. prctl could work for a subset and abused for the whole,
but a syscall feels cleaner.

I am open to other options.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-11 10:47             ` John Johansen
@ 2025-05-12 10:20               ` Mickaël Salaün
  2025-05-17  7:59                 ` John Johansen
  0 siblings, 1 reply; 36+ messages in thread
From: Mickaël Salaün @ 2025-05-12 10:20 UTC (permalink / raw)
  To: John Johansen
  Cc: Song Liu, Maxime Bélair, linux-security-module, paul,
	jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	penguin-kernel, linux-api, apparmor, linux-kernel, Arnd Bergmann

On Sun, May 11, 2025 at 03:47:21AM -0700, John Johansen wrote:
> On 5/9/25 03:26, Mickaël Salaün wrote:
> > On Thu, May 08, 2025 at 01:18:20AM -0700, John Johansen wrote:
> > > On 5/7/25 23:06, Song Liu wrote:
> > > > On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
> > > > <maxime.belair@canonical.com> wrote:
> > > > [...]

> > > > permission check to each pseudo file. The downside of the syscall, however,
> > > > is that all the permission checks are hard-coded in the kernel (except for
> > > 
> > > The permission checks don't have to be hard coded. Each LSM can define how it handles
> > > or manages the syscall. The default is that it isn't supported, but if an lsm decides
> > > to support it, there is now reason that its policy can't determine the use of the
> > > syscall.
> > 
> >  From an interface design point of view, it would be better to clearly
> > specify the scope of a command (e.g. which components could be impacted
> > by a command), and make sure the documentation reflect that as well.
> > Even better, have a syscalls per required privileges and impact (e.g.
> > privileged or unprivileged).  Going this road, I'm not sure if a
> > privileged syscall would make sense given the existing filesystem
> > interface.
> > 
> 
> uhhhmmm, not just privileged. As you well know we are looking to use
> this for unprivileged policy. The LSM can limit to privileged if it
> wants but it doesn't have to limit it to privileged policy.

Yes, I meant to say having a syscall for unprivileged actions, and maybe
another one for privileged ones, but this might be a hard sell. :)

To say it another way, for your use case, do you need this syscall(s)
for privileged operations?  Do you plan to drop (or stop extending) the
filesystem interface or do you think it would be good for (AppArmor)
privileged operations too?  I know syscalls might be attractive and
could be used for everything, but it's good to have a well-defined plan
and semantic to avoid using such syscall as another multiplexer with
unrelated operations and required privileges.

If this syscall should also be a way to do privileged operations, should
we also agree on a common set of permissions (e.g. global CAP_MAC_ADMIN
or user namespace one)?

[...]

> > > > Overall, I am still not convinced a syscall for all LSMs is needed. To
> > > > justify such
> > > 
> > > its not needed by all LSMs, just a subset of them, and some nebulous
> > > subset of potentially future LSMs that is entirely undefinable.
> > > 
> > > If we had had appropriate LSM syscalls landlock wouldn't have needed
> > > to have landlock specific syscalls. Having another LSM go that route
> > > feels wrong especially now that we have some LSM syscalls.
> > 
> > I don't agree.  Dedicated syscalls are a good thing.  See my other
> > reply.
> > 
> 
> I think we can just disagree on this point.
> 
> > > If a
> > > syscall is needed by an LSM its better to try hashing something out
> > > that might have utility for multiple LSMs or at the very least,
> > > potentially have utility in the future.
> > > 
> > > 
> > > > a syscall, I think we need to show that it is useful in multiple LSMs.
> > > > Also, if we
> > > > really want to have single set of APIs for all LSMs, we may also need
> > > > get_policy,
> > > 
> > > We are never going to get a single set of APIs for all LSMs. I will
> > > settle for an api that has utility for a subset
> > > 
> > > > remove_policy, etc. This set as-is appears to be an incomplete design. The
> > > 
> > > To have a complete design, there needs to be feedback and discussion
> > > from multiple LSMs. This is a starting point.
> > > 
> > > > implementation, with call_int_hook, is also problematic. It can easily
> > > > cause some> controversial behaviors.
> > > > 
> > > agreed it shouldn't be doing a straight call_int_hook, it should only
> > > call it against the lsm identified by the lsmid
> > 
> > Yes, but then, I don't see the point of a "generic" LSM syscall.
> 
> its not a generic LSM syscall. Its a syscall or maybe a set of syscalls
> for a specific scoped problem of loading/managing policy.
> 
> Can we come to something acceptable? I don't know but we are going to
> look at it before trying for an apparmor specific syscall.

I understand and it's good to have this discussion.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/3] Wire up the lsm_manage_policy syscall
  2025-05-12 10:20               ` Mickaël Salaün
@ 2025-05-17  7:59                 ` John Johansen
  0 siblings, 0 replies; 36+ messages in thread
From: John Johansen @ 2025-05-17  7:59 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Song Liu, Maxime Bélair, linux-security-module, paul,
	jmorris, serge, kees, stephen.smalley.work, casey, takedakn,
	penguin-kernel, linux-api, apparmor, linux-kernel, Arnd Bergmann

On 5/12/25 03:20, Mickaël Salaün wrote:
> On Sun, May 11, 2025 at 03:47:21AM -0700, John Johansen wrote:
>> On 5/9/25 03:26, Mickaël Salaün wrote:
>>> On Thu, May 08, 2025 at 01:18:20AM -0700, John Johansen wrote:
>>>> On 5/7/25 23:06, Song Liu wrote:
>>>>> On Wed, May 7, 2025 at 8:37 AM Maxime Bélair
>>>>> <maxime.belair@canonical.com> wrote:
>>>>> [...]
> 
>>>>> permission check to each pseudo file. The downside of the syscall, however,
>>>>> is that all the permission checks are hard-coded in the kernel (except for
>>>>
>>>> The permission checks don't have to be hard coded. Each LSM can define how it handles
>>>> or manages the syscall. The default is that it isn't supported, but if an lsm decides
>>>> to support it, there is now reason that its policy can't determine the use of the
>>>> syscall.
>>>
>>>   From an interface design point of view, it would be better to clearly
>>> specify the scope of a command (e.g. which components could be impacted
>>> by a command), and make sure the documentation reflect that as well.
>>> Even better, have a syscalls per required privileges and impact (e.g.
>>> privileged or unprivileged).  Going this road, I'm not sure if a
>>> privileged syscall would make sense given the existing filesystem
>>> interface.
>>>
>>
>> uhhhmmm, not just privileged. As you well know we are looking to use
>> this for unprivileged policy. The LSM can limit to privileged if it
>> wants but it doesn't have to limit it to privileged policy.
> 
> Yes, I meant to say having a syscall for unprivileged actions, and maybe
> another one for privileged ones, but this might be a hard sell. :)
> 
indeed, in the apparmor case context would be important. Just exactly
what is privileged. It may be a privileged operation to load policy to one
namespace, but not to another that you are setting up for a child.

> To say it another way, for your use case, do you need this syscall(s)
> for privileged operations?  Do you plan to drop (or stop extending) the

need, probably. That is to say, loading of policy have varying levels
of privilege. root within the container has privilege to load policy
to its namespace, but it might have authority to setup a child namespace
that does not require privilege for it to load policy into, and it
will determine if the child has privilege or unprivleged policy within
it.

Ideally we won't have to use the fs interface within the "privileged"
container, as there are cases where this is currently not done or
undesirable.

> filesystem interface or do you think it would be good for (AppArmor)
> privileged operations too?  I know syscalls might be attractive and
> could be used for everything, but it's good to have a well-defined plan
> and semantic to avoid using such syscall as another multiplexer with
> unrelated operations and required privileges.
> 
sure. But the privilege level is use case dependent, to which policy
namespace is policy being loaded, replaced, ...  The privilege level
very much will depend on what is in the stack/bounding of policy.

> If this syscall should also be a way to do privileged operations, should
> we also agree on a common set of permissions (e.g. global CAP_MAC_ADMIN
> or user namespace one)?
> 
I think requiring something like CAP_MAC_ADMIN would be a per LSM
decision.


> [...]
> 
>>>>> Overall, I am still not convinced a syscall for all LSMs is needed. To
>>>>> justify such
>>>>
>>>> its not needed by all LSMs, just a subset of them, and some nebulous
>>>> subset of potentially future LSMs that is entirely undefinable.
>>>>
>>>> If we had had appropriate LSM syscalls landlock wouldn't have needed
>>>> to have landlock specific syscalls. Having another LSM go that route
>>>> feels wrong especially now that we have some LSM syscalls.
>>>
>>> I don't agree.  Dedicated syscalls are a good thing.  See my other
>>> reply.
>>>
>>
>> I think we can just disagree on this point.
>>
>>>> If a
>>>> syscall is needed by an LSM its better to try hashing something out
>>>> that might have utility for multiple LSMs or at the very least,
>>>> potentially have utility in the future.
>>>>
>>>>
>>>>> a syscall, I think we need to show that it is useful in multiple LSMs.
>>>>> Also, if we
>>>>> really want to have single set of APIs for all LSMs, we may also need
>>>>> get_policy,
>>>>
>>>> We are never going to get a single set of APIs for all LSMs. I will
>>>> settle for an api that has utility for a subset
>>>>
>>>>> remove_policy, etc. This set as-is appears to be an incomplete design. The
>>>>
>>>> To have a complete design, there needs to be feedback and discussion
>>>> from multiple LSMs. This is a starting point.
>>>>
>>>>> implementation, with call_int_hook, is also problematic. It can easily
>>>>> cause some> controversial behaviors.
>>>>>
>>>> agreed it shouldn't be doing a straight call_int_hook, it should only
>>>> call it against the lsm identified by the lsmid
>>>
>>> Yes, but then, I don't see the point of a "generic" LSM syscall.
>>
>> its not a generic LSM syscall. Its a syscall or maybe a set of syscalls
>> for a specific scoped problem of loading/managing policy.
>>
>> Can we come to something acceptable? I don't know but we are going to
>> look at it before trying for an apparmor specific syscall.
> 
> I understand and it's good to have this discussion.


^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2025-05-17  7:59 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-06 14:32 [PATCH 0/3] lsm: introduce lsm_manage_policy() syscall Maxime Bélair
2025-05-06 14:32 ` [PATCH 1/3] Wire up the lsm_manage_policy syscall Maxime Bélair
2025-05-07  6:26   ` Song Liu
2025-05-07 15:37     ` Maxime Bélair
2025-05-07 22:04       ` Tetsuo Handa
2025-05-08  7:52         ` John Johansen
2025-05-09 10:25           ` Mickaël Salaün
2025-05-11 11:09             ` John Johansen
2025-05-08  6:06       ` Song Liu
2025-05-08  8:18         ` John Johansen
2025-05-09 10:26           ` Mickaël Salaün
2025-05-11 10:47             ` John Johansen
2025-05-12 10:20               ` Mickaël Salaün
2025-05-17  7:59                 ` John Johansen
2025-05-08  7:12     ` John Johansen
2025-05-07 13:58   ` kernel test robot
2025-05-06 14:32 ` [PATCH 2/3] lsm: introduce security_lsm_manage_policy hook Maxime Bélair
2025-05-07  6:19   ` Song Liu
2025-05-07 15:37     ` Maxime Bélair
2025-05-08  8:20       ` John Johansen
2025-05-07 10:40   ` Tetsuo Handa
2025-05-07 15:37     ` Maxime Bélair
2025-05-07 20:25     ` Paul Moore
2025-05-08  8:29       ` John Johansen
2025-05-08 16:54         ` Casey Schaufler
2025-05-09 10:26           ` Mickaël Salaün
2025-05-09 14:21             ` Casey Schaufler
2025-05-11 11:26               ` John Johansen
2025-05-11 11:20             ` John Johansen
2025-05-08  8:25     ` John Johansen
2025-05-08 12:55       ` Tetsuo Handa
2025-05-08 14:44         ` John Johansen
2025-05-08 15:07           ` Tetsuo Handa
2025-05-09  3:25             ` John Johansen
2025-05-07 12:04   ` kernel test robot
2025-05-06 14:32 ` [PATCH 3/3] AppArmor: add support for lsm_manage_policy Maxime Bélair

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).