[v7,02/11] LSM: Maintain a table of LSM attribute data
Commit Message
As LSMs are registered add their lsm_id pointers to a table.
This will be used later for attribute reporting.
Determine the number of possible security modules based on
their respective CONFIG options. This allows the number to be
known at build time. This allows data structures and tables
to use the constant.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/security.h | 2 ++
security/security.c | 44 ++++++++++++++++++++++++++++++++--------
2 files changed, 38 insertions(+), 8 deletions(-)
Comments
Greeting,
FYI, we noticed WARNING:at_security/security.c:#append_ordered_lsm due to commit (built with gcc-11):
commit: c7e8233da73a24636e9c1d2a7114ebc9da924fe0 ("[PATCH v7 02/11] LSM: Maintain a table of LSM attribute data")
url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230316-074751
base: https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
patch link: https://lore.kernel.org/all/20230315224704.2672-3-casey@schaufler-ca.com/
patch subject: [PATCH v7 02/11] LSM: Maintain a table of LSM attribute data
in testcase: trinity
version: trinity-static-i386-x86_64-1c734c75-1_2020-01-06
with following parameters:
runtime: 300s
group: group-02
test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Link: https://lore.kernel.org/oe-lkp/202303222245.c0b1af97-oliver.sang@intel.com
[ 1.821776][ T0] ------------[ cut here ]------------
[ 1.822708][ T0] builtin: out of LSM slots!?
[ 1.823230][ T0] WARNING: CPU: 0 PID: 0 at security/security.c:173 append_ordered_lsm (security.c:?)
[ 1.823709][ T0] Modules linked in:
[ 1.824708][ T0] CPU: 0 PID: 0 Comm: swapper Not tainted 6.3.0-rc1-00003-gc7e8233da73a #1
[ 1.825708][ T0] EIP: append_ordered_lsm (security.c:?)
[ 1.826307][ T0] Code: c3 55 89 e5 57 89 d7 56 53 89 c3 e8 7b ff ff ff 84 c0 75 7a 8b 35 7c 99 12 c3 83 fe 01 75 11 57 68 3e e7 7c c2 e8 b4 a6 f4 fd <0f> 0b 58 5a eb 5e 83 7b 0c 00 75 07 c7 43 0c 84 99 12 c3 8d 46 01
All code
========
0: c3 retq
1: 55 push %rbp
2: 89 e5 mov %esp,%ebp
4: 57 push %rdi
5: 89 d7 mov %edx,%edi
7: 56 push %rsi
8: 53 push %rbx
9: 89 c3 mov %eax,%ebx
b: e8 7b ff ff ff callq 0xffffffffffffff8b
10: 84 c0 test %al,%al
12: 75 7a jne 0x8e
14: 8b 35 7c 99 12 c3 mov -0x3ced6684(%rip),%esi # 0xffffffffc3129996
1a: 83 fe 01 cmp $0x1,%esi
1d: 75 11 jne 0x30
1f: 57 push %rdi
20: 68 3e e7 7c c2 pushq $0xffffffffc27ce73e
25: e8 b4 a6 f4 fd callq 0xfffffffffdf4a6de
2a:* 0f 0b ud2 <-- trapping instruction
2c: 58 pop %rax
2d: 5a pop %rdx
2e: eb 5e jmp 0x8e
30: 83 7b 0c 00 cmpl $0x0,0xc(%rbx)
34: 75 07 jne 0x3d
36: c7 43 0c 84 99 12 c3 movl $0xc3129984,0xc(%rbx)
3d: 8d 46 01 lea 0x1(%rsi),%eax
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 58 pop %rax
3: 5a pop %rdx
4: eb 5e jmp 0x64
6: 83 7b 0c 00 cmpl $0x0,0xc(%rbx)
a: 75 07 jne 0x13
c: c7 43 0c 84 99 12 c3 movl $0xc3129984,0xc(%rbx)
13: 8d 46 01 lea 0x1(%rsi),%eax
[ 1.826710][ T0] EAX: 00000000 EBX: c313d090 ECX: 00000000 EDX: 00000000
[ 1.827500][ T0] ESI: 00000001 EDI: c27ce8d5 EBP: c29b7f44 ESP: c29b7f30
[ 1.827709][ T0] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00210246
[ 1.828712][ T0] CR0: 80050033 CR2: ffd99000 CR3: 0314f000 CR4: 00040690
[ 1.829710][ T0] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 1.830708][ T0] DR6: fffe0ff0 DR7: 00000400
[ 1.831708][ T0] Call Trace:
[ 1.832104][ T0] ordered_lsm_parse (security.c:?)
[ 1.832712][ T0] ordered_lsm_init (security.c:?)
[ 1.833264][ T0] security_init (??:?)
[ 1.833710][ T0] start_kernel (??:?)
[ 1.834247][ T0] i386_start_kernel (??:?)
[ 1.834709][ T0] startup_32_smp (??:?)
[ 1.835279][ T0] irq event stamp: 1363
[ 1.835709][ T0] hardirqs last enabled at (1373): __up_console_sem (printk.c:?)
[ 1.836709][ T0] hardirqs last disabled at (1382): __up_console_sem (printk.c:?)
[ 1.837708][ T0] softirqs last enabled at (0): 0x0
[ 1.838708][ T0] softirqs last disabled at (0): 0x0
[ 1.839708][ T0] ---[ end trace 0000000000000000 ]---
[ 1.840359][ T0] LSM: initializing lsm=capability
To reproduce:
# build kernel
cd linux
cp config-6.3.0-rc1-00003-gc7e8233da73a .config
make HOSTCC=gcc-11 CC=gcc-11 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-11 CC=gcc-11 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
On Wed, Mar 15, 2023 at 6:47 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> As LSMs are registered add their lsm_id pointers to a table.
> This will be used later for attribute reporting.
>
> Determine the number of possible security modules based on
> their respective CONFIG options. This allows the number to be
> known at build time. This allows data structures and tables
> to use the constant.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 2 ++
> security/security.c | 44 ++++++++++++++++++++++++++++++++--------
> 2 files changed, 38 insertions(+), 8 deletions(-)
...
> diff --git a/security/security.c b/security/security.c
> index 58828a326024..aa84b1cf4253 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -513,6 +531,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> {
> int i;
>
> + /*
> + * A security module may call security_add_hooks() more
> + * than once. Landlock is one such case.
> + */
> + if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid)
> + lsm_idlist[lsm_active_cnt++] = lsmid;
> +
> + if (lsm_active_cnt > LSM_COUNT)
> + panic("%s Too many LSMs registered.\n", __func__);
In addition to the fixes needed to resolve the bug identified by the
kernel test robot, I think it might be wise to do the @lsm_active_cnt
check *before* potentially adding it to the @lsm_idlist array.
> for (i = 0; i < count; i++) {
> hooks[i].lsmid = lsmid;
> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> --
> 2.39.2
--
paul-moore.com
@@ -138,6 +138,8 @@ enum lockdown_reason {
};
extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
+extern u32 lsm_active_cnt;
+extern struct lsm_id *lsm_idlist[];
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
@@ -28,12 +28,29 @@
#include <linux/backing-dev.h>
#include <linux/string.h>
#include <linux/msg.h>
+#include <uapi/linux/lsm.h>
#include <net/flow.h>
#define MAX_LSM_EVM_XATTR 2
-/* How many LSMs were built into the kernel? */
-#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
+/*
+ * How many LSMs are built into the kernel as determined at
+ * build time. Used to determine fixed array sizes.
+ * The capability module is accounted for by CONFIG_SECURITY
+ */
+#define LSM_COUNT ( \
+ (IS_ENABLED(CONFIG_SECURITY) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_IMA) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN_LSM) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0))
/*
* These are descriptions of the reasons that can be passed to the
@@ -90,7 +107,7 @@ static __initdata const char *chosen_major_lsm;
static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
/* Ordered list of LSMs to initialize. */
-static __initdata struct lsm_info **ordered_lsms;
+static __initdata struct lsm_info *ordered_lsms[LSM_COUNT];
static __initdata struct lsm_info *exclusive;
static __initdata bool debug;
@@ -341,13 +358,16 @@ static void __init report_lsm_order(void)
pr_cont("\n");
}
+/*
+ * Current index to use while initializing the lsm id list.
+ */
+u32 lsm_active_cnt __lsm_ro_after_init;
+struct lsm_id *lsm_idlist[LSM_COUNT] __lsm_ro_after_init;
+
static void __init ordered_lsm_init(void)
{
struct lsm_info **lsm;
- ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
- GFP_KERNEL);
-
if (chosen_lsm_order) {
if (chosen_major_lsm) {
pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
@@ -387,8 +407,6 @@ static void __init ordered_lsm_init(void)
lsm_early_task(current);
for (lsm = ordered_lsms; *lsm; lsm++)
initialize_lsm(*lsm);
-
- kfree(ordered_lsms);
}
int __init early_security_init(void)
@@ -513,6 +531,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
{
int i;
+ /*
+ * A security module may call security_add_hooks() more
+ * than once. Landlock is one such case.
+ */
+ if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid)
+ lsm_idlist[lsm_active_cnt++] = lsmid;
+
+ if (lsm_active_cnt > LSM_COUNT)
+ panic("%s Too many LSMs registered.\n", __func__);
+
for (i = 0; i < count; i++) {
hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);