2023-04-21 17:45:04

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 00/11] LSM: Three basic syscalls

Add three system calls for the Linux Security Module ABI.

lsm_get_self_attr() provides the security module specific attributes
that have previously been visible in the /proc/self/attr directory.
For each security module that uses the specified attribute on the
current process the system call will return an LSM identifier and
the value of the attribute. The LSM and attribute identifier values
are defined in include/uapi/linux/lsm.h

LSM identifiers are simple integers and reflect the order in which
the LSM was added to the mainline kernel. This is a convention, not
a promise of the API. LSM identifiers below the value of 100 are
reserved for unspecified future uses. That could include information
about the security infrastructure itself, or about how multiple LSMs
might interact with each other.

A new LSM hook security_getselfattr() is introduced to get the
required information from the security modules. This is similar
to the existing security_getprocattr() hook, but specifies the
format in which string data is returned and requires the module
to put the information into a userspace destination.

lsm_set_self_attr() changes the specified LSM attribute. Only one
attribute can be changed at a time, and then only if the specified
security module allows the change.

A new LSM hook security_setselfattr() is introduced to set the
required information in the security modules. This is similar
to the existing security_setprocattr() hook, but specifies the
format in which string data is presented and requires the module
to get the information from a userspace destination.

lsm_list_modules() provides the LSM identifiers, in order, of the
security modules that are active on the system. This has been
available in the securityfs file /sys/kernel/security/lsm.

Patch 0001 changes the LSM registration from passing the name
of the module to passing a lsm_id structure that contains the
name of the module, an LSM identifier number and an attribute
identifier.
Patch 0002 adds the registered lsm_ids to a table.
Patch 0003 changes security_[gs]etprocattr() to use LSM IDs instead
of LSM names.
Patch 0004 implements lsm_get_self_attr() and lsm_set_self_attr().
New LSM hooks security_getselfattr() and security_setselfattr() are
defined.
Patch 0005 implements lsm_list_modules().
Patch 0006 wires up the syscalls.
Patch 0007 implements helper functions to make it easier for
security modules to use lsm_ctx structures.
Patch 0008 provides the Smack implementation for [gs]etselfattr().
Patch 0009 provides the AppArmor implementation for [gs]etselfattr().
Patch 0010 provides the SELinux implementation for [gs]etselfattr().
Patch 0011 implements selftests for the three new syscalls.

https://github.com/cschaufler/lsm-stacking.git#lsm-syscalls-6.3-rc7-v9

v9: Support a flag LSM_FLAG_SINGLE in lsm_get_self_attr() that
instructs the call to provide only the attribute for the LSM
identified in the referenced lsm_ctx structure.
Fix a typing error.
Change some coding style.
v8: Allow an LSM to provide more than one instance of an attribute,
even though none of the existing modules do so.
Pad the data returned by lsm_get_self_attr() to the size of
the struct lsm_ctx.
Change some displeasing varilable names.
v7: Pass the attribute desired to lsm_[gs]et_self_attr in its own
parameter rather than encoding it in the flags.
Change the flags parameters to u32.
Don't shortcut out of calling LSM specific code in the
infrastructure, let the LSM report that doesn't support an
attribute instead. With that it is not necessary to maintain
a set of supported attributes in the lsm_id structure.
Fix a typing error.
v6: Switch from reusing security_[gs]procattr() to using new
security_[gs]selfattr() hooks. Use explicit sized data types
in the lsm_ctx structure.

v5: Correct syscall parameter data types.

v4: Restore "reserved" LSM ID values. Add explaination.
Squash patches that introduce fields in lsm_id.
Correct a wireup error.

v3: Add lsm_set_self_attr().
Rename lsm_self_attr() to lsm_get_self_attr().
Provide the values only for a specifed attribute in
lsm_get_self_attr().
Add selftests for the three new syscalls.
Correct some parameter checking.

v2: Use user-interface safe data types.
Remove "reserved" LSM ID values.
Improve kerneldoc comments
Include copyright dates
Use more descriptive name for LSM counter
Add documentation
Correct wireup errors


Casey Schaufler (11):
LSM: Identify modules by more than name
LSM: Maintain a table of LSM attribute data
proc: Use lsmids instead of lsm names for attrs
LSM: syscalls for current process attributes
LSM: Create lsm_list_modules system call
LSM: wireup Linux Security Module syscalls
LSM: Helpers for attribute names and filling an lsm_ctx
Smack: implement setselfattr and getselfattr hooks
AppArmor: Add selfattr hooks
SELinux: Add selfattr hooks
LSM: selftests for Linux Security Module syscalls

Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/lsm.rst | 73 +++++
MAINTAINERS | 1 +
arch/alpha/kernel/syscalls/syscall.tbl | 3 +
arch/arm/tools/syscall.tbl | 3 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 6 +
arch/ia64/kernel/syscalls/syscall.tbl | 3 +
arch/m68k/kernel/syscalls/syscall.tbl | 3 +
arch/microblaze/kernel/syscalls/syscall.tbl | 3 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 3 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 3 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 3 +
arch/parisc/kernel/syscalls/syscall.tbl | 3 +
arch/powerpc/kernel/syscalls/syscall.tbl | 3 +
arch/s390/kernel/syscalls/syscall.tbl | 3 +
arch/sh/kernel/syscalls/syscall.tbl | 3 +
arch/sparc/kernel/syscalls/syscall.tbl | 3 +
arch/x86/entry/syscalls/syscall_32.tbl | 3 +
arch/x86/entry/syscalls/syscall_64.tbl | 3 +
arch/xtensa/kernel/syscalls/syscall.tbl | 3 +
fs/proc/base.c | 29 +-
fs/proc/internal.h | 2 +-
include/linux/lsm_hook_defs.h | 4 +
include/linux/lsm_hooks.h | 25 +-
include/linux/security.h | 45 ++-
include/linux/syscalls.h | 6 +
include/uapi/asm-generic/unistd.h | 11 +-
include/uapi/linux/lsm.h | 90 ++++++
kernel/sys_ni.c | 5 +
security/Makefile | 1 +
security/apparmor/include/procattr.h | 2 +-
security/apparmor/lsm.c | 104 ++++++-
security/apparmor/procattr.c | 11 +-
security/bpf/hooks.c | 9 +-
security/commoncap.c | 8 +-
security/landlock/cred.c | 2 +-
security/landlock/fs.c | 2 +-
security/landlock/ptrace.c | 2 +-
security/landlock/setup.c | 6 +
security/landlock/setup.h | 1 +
security/loadpin/loadpin.c | 9 +-
security/lockdown/lockdown.c | 8 +-
security/lsm_syscalls.c | 118 ++++++++
security/safesetid/lsm.c | 9 +-
security/security.c | 215 ++++++++++++--
security/selinux/hooks.c | 162 +++++++++--
security/smack/smack_lsm.c | 113 +++++++-
security/tomoyo/tomoyo.c | 9 +-
security/yama/yama_lsm.c | 8 +-
.../arch/mips/entry/syscalls/syscall_n64.tbl | 3 +
.../arch/powerpc/entry/syscalls/syscall.tbl | 3 +
.../perf/arch/s390/entry/syscalls/syscall.tbl | 3 +
.../arch/x86/entry/syscalls/syscall_64.tbl | 3 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/lsm/Makefile | 12 +
tools/testing/selftests/lsm/config | 2 +
.../selftests/lsm/lsm_get_self_attr_test.c | 267 ++++++++++++++++++
.../selftests/lsm/lsm_list_modules_test.c | 149 ++++++++++
.../selftests/lsm/lsm_set_self_attr_test.c | 70 +++++
60 files changed, 1559 insertions(+), 101 deletions(-)
create mode 100644 Documentation/userspace-api/lsm.rst
create mode 100644 include/uapi/linux/lsm.h
create mode 100644 security/lsm_syscalls.c
create mode 100644 tools/testing/selftests/lsm/Makefile
create mode 100644 tools/testing/selftests/lsm/config
create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_list_modules_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c

--
2.39.2


2023-04-21 17:45:21

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 01/11] LSM: Identify modules by more than name

Create a struct lsm_id to contain identifying information
about Linux Security Modules (LSMs). At inception this contains
the name of the module, an identifier associated with the security
module and an integer member "attrs" which identifies the API
related data associated with each security module. The initial set
of features maps to information that has traditionaly been available
in /proc/self/attr. They are documented in a new userspace-api file.
Change the security_add_hooks() interface to use this structure.
Change the individual modules to maintain their own struct lsm_id
and pass it to security_add_hooks().

The values are for LSM identifiers are defined in a new UAPI
header file linux/lsm.h. Each existing LSM has been updated to
include it's LSMID in the lsm_id.

The LSM ID values are sequential, with the oldest module
LSM_ID_CAPABILITY being the lowest value and the existing modules
numbered in the order they were included in the main line kernel.
This is an arbitrary convention for assigning the values, but
none better presents itself. The value 0 is defined as being invalid.
The values 1-99 are reserved for any special case uses which may
arise in the future. This may include attributes of the LSM
infrastructure itself, possibly related to namespacing or network
attribute management. A special range is identified for such attributes
to help reduce confusion for developers unfamiliar with LSMs.

LSM attribute values are defined for the attributes presented by
modules that are available today. As with the LSM IDs, The value 0
is defined as being invalid. The values 1-99 are reserved for any
special case uses which may arise in the future.

Signed-off-by: Casey Schaufler <[email protected]>
Cc: linux-security-module <[email protected]>
---
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/lsm.rst | 55 +++++++++++++++++++++++++++
MAINTAINERS | 1 +
include/linux/lsm_hooks.h | 16 +++++++-
include/uapi/linux/lsm.h | 54 ++++++++++++++++++++++++++
security/apparmor/lsm.c | 8 +++-
security/bpf/hooks.c | 9 ++++-
security/commoncap.c | 8 +++-
security/landlock/cred.c | 2 +-
security/landlock/fs.c | 2 +-
security/landlock/ptrace.c | 2 +-
security/landlock/setup.c | 6 +++
security/landlock/setup.h | 1 +
security/loadpin/loadpin.c | 9 ++++-
security/lockdown/lockdown.c | 8 +++-
security/safesetid/lsm.c | 9 ++++-
security/security.c | 12 +++---
security/selinux/hooks.c | 9 ++++-
security/smack/smack_lsm.c | 8 +++-
security/tomoyo/tomoyo.c | 9 ++++-
security/yama/yama_lsm.c | 8 +++-
21 files changed, 216 insertions(+), 21 deletions(-)
create mode 100644 Documentation/userspace-api/lsm.rst
create mode 100644 include/uapi/linux/lsm.h

diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index f16337bdb852..54c0f54cde89 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -31,6 +31,7 @@ place where this information is gathered.
sysfs-platform_profile
vduse
futex2
+ lsm

.. only:: subproject and html

diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
new file mode 100644
index 000000000000..6ddf5506110b
--- /dev/null
+++ b/Documentation/userspace-api/lsm.rst
@@ -0,0 +1,55 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. Copyright (C) 2022 Casey Schaufler <[email protected]>
+.. Copyright (C) 2022 Intel Corporation
+
+=====================================
+Linux Security Modules
+=====================================
+
+:Author: Casey Schaufler
+:Date: November 2022
+
+Linux security modules (LSM) provide a mechanism to implement
+additional access controls to the Linux security policies.
+
+The various security modules may support any of these attributes:
+
+``LSM_ATTR_CURRENT`` is the current, active security context of the
+process.
+The proc filesystem provides this value in ``/proc/self/attr/current``.
+This is supported by the SELinux, Smack and AppArmor security modules.
+Smack also provides this value in ``/proc/self/attr/smack/current``.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/current``.
+
+``LSM_ATTR_EXEC`` is the security context of the process at the time the
+current image was executed.
+The proc filesystem provides this value in ``/proc/self/attr/exec``.
+This is supported by the SELinux and AppArmor security modules.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/exec``.
+
+``LSM_ATTR_FSCREATE`` is the security context of the process used when
+creating file system objects.
+The proc filesystem provides this value in ``/proc/self/attr/fscreate``.
+This is supported by the SELinux security module.
+
+``LSM_ATTR_KEYCREATE`` is the security context of the process used when
+creating key objects.
+The proc filesystem provides this value in ``/proc/self/attr/keycreate``.
+This is supported by the SELinux security module.
+
+``LSM_ATTR_PREV`` is the security context of the process at the time the
+current security context was set.
+The proc filesystem provides this value in ``/proc/self/attr/prev``.
+This is supported by the SELinux and AppArmor security modules.
+AppArmor also provides this value in ``/proc/self/attr/apparmor/prev``.
+
+``LSM_ATTR_SOCKCREATE`` is the security context of the process used when
+creating socket objects.
+The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
+This is supported by the SELinux security module.
+
+Additional documentation
+========================
+
+* Documentation/security/lsm.rst
+* Documentation/security/lsm-development.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index 0e64787aace8..25d09f6eb3ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18810,6 +18810,7 @@ S: Supported
W: http://kernsec.org/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
F: security/
+F: include/uapi/linux/lsm.h
X: security/selinux/

SELINUX SECURITY MODULE
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 6e156d2acffc..8e6ba0a9896e 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1665,6 +1665,18 @@ struct security_hook_heads {
#undef LSM_HOOK
} __randomize_layout;

+/**
+ * struct lsm_id - Identify a Linux Security Module.
+ * @lsm: name of the LSM, must be approved by the LSM maintainers
+ * @id: LSM ID number from uapi/linux/lsm.h
+ *
+ * Contains the information that identifies the LSM.
+ */
+struct lsm_id {
+ const u8 *lsm;
+ u64 id;
+};
+
/*
* Security module hook list structure.
* For use with generic list macros for common operations.
@@ -1673,7 +1685,7 @@ struct security_hook_list {
struct hlist_node list;
struct hlist_head *head;
union security_list_options hook;
- const char *lsm;
+ struct lsm_id *lsmid;
} __randomize_layout;

/*
@@ -1708,7 +1720,7 @@ extern struct security_hook_heads security_hook_heads;
extern char *lsm_names;

extern void security_add_hooks(struct security_hook_list *hooks, int count,
- const char *lsm);
+ struct lsm_id *lsmid);

#define LSM_FLAG_LEGACY_MAJOR BIT(0)
#define LSM_FLAG_EXCLUSIVE BIT(1)
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
new file mode 100644
index 000000000000..f27c9a9cc376
--- /dev/null
+++ b/include/uapi/linux/lsm.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Linux Security Modules (LSM) - User space API
+ *
+ * Copyright (C) 2022 Casey Schaufler <[email protected]>
+ * Copyright (C) 2022 Intel Corporation
+ */
+
+#ifndef _UAPI_LINUX_LSM_H
+#define _UAPI_LINUX_LSM_H
+
+/*
+ * ID tokens to identify Linux Security Modules (LSMs)
+ *
+ * These token values are used to uniquely identify specific LSMs
+ * in the kernel as well as in the kernel's LSM userspace API.
+ *
+ * A value of zero/0 is considered undefined and should not be used
+ * outside the kernel. Values 1-99 are reserved for potential
+ * future use.
+ */
+#define LSM_ID_UNDEF 0
+#define LSM_ID_CAPABILITY 100
+#define LSM_ID_SELINUX 101
+#define LSM_ID_SMACK 102
+#define LSM_ID_TOMOYO 103
+#define LSM_ID_IMA 104
+#define LSM_ID_APPARMOR 105
+#define LSM_ID_YAMA 106
+#define LSM_ID_LOADPIN 107
+#define LSM_ID_SAFESETID 108
+#define LSM_ID_LOCKDOWN 109
+#define LSM_ID_BPF 110
+#define LSM_ID_LANDLOCK 111
+
+/*
+ * LSM_ATTR_XXX definitions identify different LSM attributes
+ * which are used in the kernel's LSM userspace API. Support
+ * for these attributes vary across the different LSMs. None
+ * are required.
+ *
+ * A value of zero/0 is considered undefined and should not be used
+ * outside the kernel. Values 1-99 are reserved for potential
+ * future use.
+ */
+#define LSM_ATTR_UNDEF 0
+#define LSM_ATTR_CURRENT 100
+#define LSM_ATTR_EXEC 101
+#define LSM_ATTR_FSCREATE 102
+#define LSM_ATTR_KEYCREATE 103
+#define LSM_ATTR_PREV 104
+#define LSM_ATTR_SOCKCREATE 105
+
+#endif /* _UAPI_LINUX_LSM_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index d6cc4812ca53..ce6ccb7e06ec 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -24,6 +24,7 @@
#include <linux/zstd.h>
#include <net/sock.h>
#include <uapi/linux/mount.h>
+#include <uapi/linux/lsm.h>

#include "include/apparmor.h"
#include "include/apparmorfs.h"
@@ -1215,6 +1216,11 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
.lbs_task = sizeof(struct aa_task_ctx),
};

+static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
+ .lsm = "apparmor",
+ .id = LSM_ID_APPARMOR,
+};
+
static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
@@ -1910,7 +1916,7 @@ static int __init apparmor_init(void)
goto buffers_out;
}
security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
- "apparmor");
+ &apparmor_lsmid);

/* Report that AppArmor successfully initialized */
apparmor_initialized = 1;
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index e5971fa74fd7..5232c80be5b3 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -5,6 +5,7 @@
*/
#include <linux/lsm_hooks.h>
#include <linux/bpf_lsm.h>
+#include <uapi/linux/lsm.h>

static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
@@ -15,9 +16,15 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_free, bpf_task_storage_free),
};

+static struct lsm_id bpf_lsmid __lsm_ro_after_init = {
+ .lsm = "bpf",
+ .id = LSM_ID_BPF,
+};
+
static int __init bpf_lsm_init(void)
{
- security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks), "bpf");
+ security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks),
+ &bpf_lsmid);
pr_info("LSM support for eBPF active\n");
return 0;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 5bb7d1e96277..bbc0a210506a 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -25,6 +25,7 @@
#include <linux/binfmts.h>
#include <linux/personality.h>
#include <linux/mnt_idmapping.h>
+#include <uapi/linux/lsm.h>

/*
* If a non-root user executes a setuid-root binary in
@@ -1440,6 +1441,11 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,

#ifdef CONFIG_SECURITY

+static struct lsm_id capability_lsmid __lsm_ro_after_init = {
+ .lsm = "capability",
+ .id = LSM_ID_CAPABILITY,
+};
+
static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(capable, cap_capable),
LSM_HOOK_INIT(settime, cap_settime),
@@ -1464,7 +1470,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
static int __init capability_init(void)
{
security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
- "capability");
+ &capability_lsmid);
return 0;
}

diff --git a/security/landlock/cred.c b/security/landlock/cred.c
index ec6c37f04a19..2eb1d65f10d6 100644
--- a/security/landlock/cred.c
+++ b/security/landlock/cred.c
@@ -42,5 +42,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_cred_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index adcea0fe7e68..fa0e6e76991c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1307,5 +1307,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_fs_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/ptrace.c b/security/landlock/ptrace.c
index 4c5b9cd71286..eab35808f395 100644
--- a/security/landlock/ptrace.c
+++ b/security/landlock/ptrace.c
@@ -116,5 +116,5 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
__init void landlock_add_ptrace_hooks(void)
{
security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
- LANDLOCK_NAME);
+ &landlock_lsmid);
}
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index 3f196d2ce4f9..9104133d04ca 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -8,6 +8,7 @@

#include <linux/init.h>
#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>

#include "common.h"
#include "cred.h"
@@ -24,6 +25,11 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
.lbs_superblock = sizeof(struct landlock_superblock_security),
};

+struct lsm_id landlock_lsmid __lsm_ro_after_init = {
+ .lsm = LANDLOCK_NAME,
+ .id = LSM_ID_LANDLOCK,
+};
+
static int __init landlock_init(void)
{
landlock_add_cred_hooks();
diff --git a/security/landlock/setup.h b/security/landlock/setup.h
index 1daffab1ab4b..38bce5b172dc 100644
--- a/security/landlock/setup.h
+++ b/security/landlock/setup.h
@@ -14,5 +14,6 @@
extern bool landlock_initialized;

extern struct lsm_blob_sizes landlock_blob_sizes;
+extern struct lsm_id landlock_lsmid;

#endif /* _SECURITY_LANDLOCK_SETUP_H */
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index d73a281adf86..556d43e37177 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -20,6 +20,7 @@
#include <linux/string_helpers.h>
#include <linux/dm-verity-loadpin.h>
#include <uapi/linux/loadpin.h>
+#include <uapi/linux/lsm.h>

#define VERITY_DIGEST_FILE_HEADER "# LOADPIN_TRUSTED_VERITY_ROOT_DIGESTS"

@@ -214,6 +215,11 @@ static int loadpin_load_data(enum kernel_load_data_id id, bool contents)
return loadpin_check(NULL, (enum kernel_read_file_id) id);
}

+static struct lsm_id loadpin_lsmid __lsm_ro_after_init = {
+ .lsm = "loadpin",
+ .id = LSM_ID_LOADPIN,
+};
+
static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
@@ -265,7 +271,8 @@ static int __init loadpin_init(void)
if (!register_sysctl_paths(loadpin_sysctl_path, loadpin_sysctl_table))
pr_notice("sysctl registration failed!\n");
#endif
- security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
+ security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks),
+ &loadpin_lsmid);

return 0;
}
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index a79b985e917e..e8c41a0caf7d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -13,6 +13,7 @@
#include <linux/security.h>
#include <linux/export.h>
#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>

static enum lockdown_reason kernel_locked_down;

@@ -75,6 +76,11 @@ static struct security_hook_list lockdown_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(locked_down, lockdown_is_locked_down),
};

+static struct lsm_id lockdown_lsmid __lsm_ro_after_init = {
+ .lsm = "lockdown",
+ .id = LSM_ID_LOCKDOWN,
+};
+
static int __init lockdown_lsm_init(void)
{
#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
@@ -83,7 +89,7 @@ static int __init lockdown_lsm_init(void)
lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY_MAX);
#endif
security_add_hooks(lockdown_hooks, ARRAY_SIZE(lockdown_hooks),
- "lockdown");
+ &lockdown_lsmid);
return 0;
}

diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index e806739f7868..8d0742ba045d 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -19,6 +19,7 @@
#include <linux/ptrace.h>
#include <linux/sched/task_stack.h>
#include <linux/security.h>
+#include <uapi/linux/lsm.h>
#include "lsm.h"

/* Flag indicating whether initialization completed */
@@ -261,6 +262,11 @@ static int safesetid_task_fix_setgroups(struct cred *new, const struct cred *old
return 0;
}

+static struct lsm_id safesetid_lsmid __lsm_ro_after_init = {
+ .lsm = "safesetid",
+ .id = LSM_ID_SAFESETID,
+};
+
static struct security_hook_list safesetid_security_hooks[] = {
LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
LSM_HOOK_INIT(task_fix_setgid, safesetid_task_fix_setgid),
@@ -271,7 +277,8 @@ static struct security_hook_list safesetid_security_hooks[] = {
static int __init safesetid_security_init(void)
{
security_add_hooks(safesetid_security_hooks,
- ARRAY_SIZE(safesetid_security_hooks), "safesetid");
+ ARRAY_SIZE(safesetid_security_hooks),
+ &safesetid_lsmid);

/* Report that SafeSetID successfully initialized */
safesetid_initialized = 1;
diff --git a/security/security.c b/security/security.c
index cf6cc576736f..58828a326024 100644
--- a/security/security.c
+++ b/security/security.c
@@ -504,17 +504,17 @@ static int lsm_append(const char *new, char **result)
* security_add_hooks - Add a modules hooks to the hook lists.
* @hooks: the hooks to add
* @count: the number of hooks to add
- * @lsm: the name of the security module
+ * @lsmid: the identification information for the security module
*
* Each LSM has to register its hooks with the infrastructure.
*/
void __init security_add_hooks(struct security_hook_list *hooks, int count,
- const char *lsm)
+ struct lsm_id *lsmid)
{
int i;

for (i = 0; i < count; i++) {
- hooks[i].lsm = lsm;
+ hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
}

@@ -523,7 +523,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
* and fix this up afterwards.
*/
if (slab_is_available()) {
- if (lsm_append(lsm, &lsm_names) < 0)
+ if (lsm_append(lsmid->lsm, &lsm_names) < 0)
panic("%s - Cannot get early memory.\n", __func__);
}
}
@@ -2146,7 +2146,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
struct security_hook_list *hp;

hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.getprocattr(p, name, value);
}
@@ -2159,7 +2159,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
struct security_hook_list *hp;

hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.setprocattr(name, value, size);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9a5bdfc21314..9403aee75981 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -92,6 +92,7 @@
#include <linux/fsnotify.h>
#include <linux/fanotify.h>
#include <linux/io_uring.h>
+#include <uapi/linux/lsm.h>

#include "avc.h"
#include "objsec.h"
@@ -7032,6 +7033,11 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */

+static struct lsm_id selinux_lsmid __lsm_ro_after_init = {
+ .lsm = "selinux",
+ .id = LSM_ID_SELINUX,
+};
+
/*
* IMPORTANT NOTE: When adding new hooks, please be careful to keep this order:
* 1. any hooks that don't belong to (2.) or (3.) below,
@@ -7355,7 +7361,8 @@ static __init int selinux_init(void)

hashtab_cache_init();

- security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+ security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks),
+ &selinux_lsmid);

if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
panic("SELinux: Unable to register AVC netcache callback\n");
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index cfcbb748da25..3cf862fcbe08 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -43,6 +43,7 @@
#include <linux/fs_parser.h>
#include <linux/watch_queue.h>
#include <linux/io_uring.h>
+#include <uapi/linux/lsm.h>
#include "smack.h"

#define TRANS_TRUE "TRUE"
@@ -4856,6 +4857,11 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_superblock = sizeof(struct superblock_smack),
};

+static struct lsm_id smack_lsmid __lsm_ro_after_init = {
+ .lsm = "smack",
+ .id = LSM_ID_SMACK,
+};
+
static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
@@ -5062,7 +5068,7 @@ static __init int smack_init(void)
/*
* Register with LSM
*/
- security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+ security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), &smack_lsmid);
smack_enabled = 1;

pr_info("Smack: Initializing.\n");
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index af04a7b7eb28..a4658fb5ef0e 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -6,6 +6,7 @@
*/

#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
#include "common.h"

/**
@@ -542,6 +543,11 @@ static void tomoyo_task_free(struct task_struct *task)
}
}

+static struct lsm_id tomoyo_lsmid __lsm_ro_after_init = {
+ .lsm = "tomoyo",
+ .id = LSM_ID_TOMOYO,
+};
+
/*
* tomoyo_security_ops is a "struct security_operations" which is used for
* registering TOMOYO.
@@ -595,7 +601,8 @@ static int __init tomoyo_init(void)
struct tomoyo_task *s = tomoyo_task(current);

/* register ourselves with the security framework */
- security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
+ security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks),
+ &tomoyo_lsmid);
pr_info("TOMOYO Linux initialized\n");
s->domain_info = &tomoyo_kernel_domain;
atomic_inc(&tomoyo_kernel_domain.users);
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 06e226166aab..2487b8f847f3 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -18,6 +18,7 @@
#include <linux/task_work.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
+#include <uapi/linux/lsm.h>

#define YAMA_SCOPE_DISABLED 0
#define YAMA_SCOPE_RELATIONAL 1
@@ -421,6 +422,11 @@ static int yama_ptrace_traceme(struct task_struct *parent)
return rc;
}

+static struct lsm_id yama_lsmid __lsm_ro_after_init = {
+ .lsm = "yama",
+ .id = LSM_ID_YAMA,
+};
+
static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
@@ -477,7 +483,7 @@ static inline void yama_init_sysctl(void) { }
static int __init yama_init(void)
{
pr_info("Yama: becoming mindful.\n");
- security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
+ security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), &yama_lsmid);
yama_init_sysctl();
return 0;
}
--
2.39.2

2023-04-21 17:45:32

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 02/11] LSM: Maintain a table of LSM attribute data

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 <[email protected]>
---
include/linux/security.h | 2 ++
security/security.c | 43 ++++++++++++++++++++++++++++++++--------
2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 5984d0d550b4..e70fc863b04a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -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,
diff --git a/security/security.c b/security/security.c
index 58828a326024..3f98e5171176 100644
--- a/security/security.c
+++ b/security/security.c
@@ -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 + 1];
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,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
{
int i;

+ if (lsm_active_cnt >= LSM_COUNT)
+ panic("%s Too many LSMs registered.\n", __func__);
+ /*
+ * 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;
+
for (i = 0; i < count; i++) {
hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
--
2.39.2

2023-04-21 17:45:51

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 03/11] proc: Use lsmids instead of lsm names for attrs

Use the LSM ID number instead of the LSM name to identify which
security module's attibute data should be shown in /proc/self/attr.
The security_[gs]etprocattr() functions have been changed to expect
the LSM ID. The change from a string comparison to an integer comparison
in these functions will provide a minor performance improvement.

Signed-off-by: Casey Schaufler <[email protected]>
Cc: [email protected]
---
fs/proc/base.c | 29 +++++++++++++++--------------
fs/proc/internal.h | 2 +-
include/linux/security.h | 11 +++++------
security/security.c | 11 +++++------
4 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 5e0e0ccd47aa..cb6dec7473fe 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -96,6 +96,7 @@
#include <linux/time_namespace.h>
#include <linux/resctrl.h>
#include <linux/cn_proc.h>
+#include <uapi/linux/lsm.h>
#include <trace/events/oom.h>
#include "internal.h"
#include "fd.h"
@@ -145,10 +146,10 @@ struct pid_entry {
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_single_file_operations, \
{ .proc_show = show } )
-#define ATTR(LSM, NAME, MODE) \
+#define ATTR(LSMID, NAME, MODE) \
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_pid_attr_operations, \
- { .lsm = LSM })
+ { .lsmid = LSMID })

/*
* Count the number of hardlinks for the pid_entry table, excluding the .
@@ -2730,7 +2731,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
if (!task)
return -ESRCH;

- length = security_getprocattr(task, PROC_I(inode)->op.lsm,
+ length = security_getprocattr(task, PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name,
&p);
put_task_struct(task);
@@ -2788,7 +2789,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;

- rv = security_setprocattr(PROC_I(inode)->op.lsm,
+ rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
mutex_unlock(&current->signal->cred_guard_mutex);
@@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \

#ifdef CONFIG_SECURITY_SMACK
static const struct pid_entry smack_attr_dir_stuff[] = {
- ATTR("smack", "current", 0666),
+ ATTR(LSM_ID_SMACK, "current", 0666),
};
LSM_DIR_OPS(smack);
#endif

#ifdef CONFIG_SECURITY_APPARMOR
static const struct pid_entry apparmor_attr_dir_stuff[] = {
- ATTR("apparmor", "current", 0666),
- ATTR("apparmor", "prev", 0444),
- ATTR("apparmor", "exec", 0666),
+ ATTR(LSM_ID_APPARMOR, "current", 0666),
+ ATTR(LSM_ID_APPARMOR, "prev", 0444),
+ ATTR(LSM_ID_APPARMOR, "exec", 0666),
};
LSM_DIR_OPS(apparmor);
#endif

static const struct pid_entry attr_dir_stuff[] = {
- ATTR(NULL, "current", 0666),
- ATTR(NULL, "prev", 0444),
- ATTR(NULL, "exec", 0666),
- ATTR(NULL, "fscreate", 0666),
- ATTR(NULL, "keycreate", 0666),
- ATTR(NULL, "sockcreate", 0666),
+ ATTR(LSM_ID_UNDEF, "current", 0666),
+ ATTR(LSM_ID_UNDEF, "prev", 0444),
+ ATTR(LSM_ID_UNDEF, "exec", 0666),
+ ATTR(LSM_ID_UNDEF, "fscreate", 0666),
+ ATTR(LSM_ID_UNDEF, "keycreate", 0666),
+ ATTR(LSM_ID_UNDEF, "sockcreate", 0666),
#ifdef CONFIG_SECURITY_SMACK
DIR("smack", 0555,
proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 9dda7e54b2d0..a889d9ef9584 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -92,7 +92,7 @@ union proc_op {
int (*proc_show)(struct seq_file *m,
struct pid_namespace *ns, struct pid *pid,
struct task_struct *task);
- const char *lsm;
+ int lsmid;
};

struct proc_inode {
diff --git a/include/linux/security.h b/include/linux/security.h
index e70fc863b04a..8faed81fc3b4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -473,10 +473,9 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
unsigned nsops, int alter);
void security_d_instantiate(struct dentry *dentry, struct inode *inode);
-int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
char **value);
-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size);
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
@@ -1344,14 +1343,14 @@ static inline void security_d_instantiate(struct dentry *dentry,
struct inode *inode)
{ }

-static inline int security_getprocattr(struct task_struct *p, const char *lsm,
+static inline int security_getprocattr(struct task_struct *p, int lsmid,
const char *name, char **value)
{
return -EINVAL;
}

-static inline int security_setprocattr(const char *lsm, char *name,
- void *value, size_t size)
+static inline int security_setprocattr(int lsmid, char *name, void *value,
+ size_t size)
{
return -EINVAL;
}
diff --git a/security/security.c b/security/security.c
index 3f98e5171176..38ca0e646cac 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2167,26 +2167,25 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
}
EXPORT_SYMBOL(security_d_instantiate);

-int security_getprocattr(struct task_struct *p, const char *lsm,
- const char *name, char **value)
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
+ char **value)
{
struct security_hook_list *hp;

hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.getprocattr(p, name, value);
}
return LSM_RET_DEFAULT(getprocattr);
}

-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size)
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size)
{
struct security_hook_list *hp;

hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.setprocattr(name, value, size);
}
--
2.39.2

2023-04-21 17:47:00

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 05/11] LSM: Create lsm_list_modules system call

Create a system call to report the list of Linux Security Modules
that are active on the system. The list is provided as an array
of LSM ID numbers.

The calling application can use this list determine what LSM
specific actions it might take. That might include choosing an
output format, determining required privilege or bypassing
security module specific behavior.

Signed-off-by: Casey Schaufler <[email protected]>
---
Documentation/userspace-api/lsm.rst | 3 +++
include/linux/syscalls.h | 1 +
kernel/sys_ni.c | 1 +
security/lsm_syscalls.c | 39 +++++++++++++++++++++++++++++
4 files changed, 44 insertions(+)

diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
index b45e402302b3..a86e3817f062 100644
--- a/Documentation/userspace-api/lsm.rst
+++ b/Documentation/userspace-api/lsm.rst
@@ -63,6 +63,9 @@ Get the specified security attributes of the current process
.. kernel-doc:: security/lsm_syscalls.c
:identifiers: sys_lsm_get_self_attr

+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_list_modules
+
Additional documentation
========================

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 9a94c31bf6b6..ddbcc333f3c3 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1063,6 +1063,7 @@ asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
size_t *size, __u32 flags);
asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
size_t size, __u32 flags);
+asmlinkage long sys_lsm_list_modules(u64 *ids, size_t *size, u32 flags);

/*
* Architecture-specific system calls
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index d03c78ef1562..ceb3d21a62d0 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -265,6 +265,7 @@ COND_SYSCALL(mremap);
/* security/lsm_syscalls.c */
COND_SYSCALL(lsm_get_self_attr);
COND_SYSCALL(lsm_set_self_attr);
+COND_SYSCALL(lsm_list_modules);

/* security/keys/keyctl.c */
COND_SYSCALL(add_key);
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index feee31600219..6efbe244d304 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -53,3 +53,42 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
{
return security_getselfattr(attr, ctx, size, flags);
}
+
+/**
+ * sys_lsm_list_modules - Return a list of the active security modules
+ * @ids: the LSM module ids
+ * @size: size of @ids, updated on return
+ * @flags: reserved for future use, must be zero
+ *
+ * Returns a list of the active LSM ids. On success this function
+ * returns the number of @ids array elements. This value may be zero
+ * if there are no LSMs active. If @size is insufficient to contain
+ * the return data -E2BIG is returned and @size is set to the minimum
+ * required size. In all other cases a negative value indicating the
+ * error is returned.
+ */
+SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size,
+ u32, flags)
+{
+ size_t total_size = lsm_active_cnt * sizeof(*ids);
+ size_t usize;
+ int i;
+
+ if (flags)
+ return -EINVAL;
+
+ if (get_user(usize, size))
+ return -EFAULT;
+
+ if (put_user(total_size, size) != 0)
+ return -EFAULT;
+
+ if (usize < total_size)
+ return -E2BIG;
+
+ for (i = 0; i < lsm_active_cnt; i++)
+ if (put_user(lsm_idlist[i]->id, ids++))
+ return -EFAULT;
+
+ return lsm_active_cnt;
+}
--
2.39.2

2023-04-21 17:47:02

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 04/11] LSM: syscalls for current process attributes

Create a system call lsm_get_self_attr() to provide the security
module maintained attributes of the current process.
Create a system call lsm_set_self_attr() to set a security
module maintained attribute of the current process.
Historically these attributes have been exposed to user space via
entries in procfs under /proc/self/attr.

The attribute value is provided in a lsm_ctx structure. The structure
identifies the size of the attribute, and the attribute value. The format
of the attribute value is defined by the security module. A flags field
is included for LSM specific information. It is currently unused and must
be 0. The total size of the data, including the lsm_ctx structure and any
padding, is maintained as well.

struct lsm_ctx {
__u64 id;
__u64 flags;
__u64 len;
__u64 ctx_len;
__u8 ctx[];
};

Two new LSM hooks are used to interface with the LSMs.
security_getselfattr() collects the lsm_ctx values from the
LSMs that support the hook, accounting for space requirements.
security_setselfattr() identifies which LSM the attribute is
intended for and passes it along.

Signed-off-by: Casey Schaufler <[email protected]>
---
Documentation/userspace-api/lsm.rst | 15 ++++
include/linux/lsm_hook_defs.h | 4 +
include/linux/lsm_hooks.h | 9 +++
include/linux/security.h | 19 +++++
include/linux/syscalls.h | 5 ++
include/uapi/linux/lsm.h | 36 +++++++++
kernel/sys_ni.c | 4 +
security/Makefile | 1 +
security/lsm_syscalls.c | 55 ++++++++++++++
security/security.c | 110 ++++++++++++++++++++++++++++
10 files changed, 258 insertions(+)
create mode 100644 security/lsm_syscalls.c

diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
index 6ddf5506110b..b45e402302b3 100644
--- a/Documentation/userspace-api/lsm.rst
+++ b/Documentation/userspace-api/lsm.rst
@@ -48,6 +48,21 @@ creating socket objects.
The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
This is supported by the SELinux security module.

+Kernel interface
+================
+
+Set a security attribute of the current process
+--------------------------------------------------
+
+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_set_self_attr
+
+Get the specified security attributes of the current process
+--------------------------------------------------
+
+.. kernel-doc:: security/lsm_syscalls.c
+ :identifiers: sys_lsm_get_self_attr
+
Additional documentation
========================

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 094b76dc7164..7177d9554f4a 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -261,6 +261,10 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops,
LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb)
LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry,
struct inode *inode)
+LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t *size, u32 __user flags)
+LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t size, u32 __user flags)
LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
char **value)
LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8e6ba0a9896e..ed38ad5eb444 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -25,6 +25,7 @@
#ifndef __LINUX_LSM_HOOKS_H
#define __LINUX_LSM_HOOKS_H

+#include <uapi/linux/lsm.h>
#include <linux/security.h>
#include <linux/init.h>
#include <linux/rculist.h>
@@ -503,6 +504,14 @@
* and writing the xattrs as this hook is merely a filter.
* @d_instantiate:
* Fill in @inode security information for a @dentry if allowed.
+ * @getselfattr:
+ * Read attribute @attr for the current process and store it into @ctx.
+ * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
+ * or another negative value otherwise.
+ * @setselfattr:
+ * Set attribute @attr for the current process.
+ * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
+ * or another negative value otherwise.
* @getprocattr:
* Read attribute @name for process @p and store it into @value if allowed.
* Return the length of @value on success, a negative value otherwise.
diff --git a/include/linux/security.h b/include/linux/security.h
index 8faed81fc3b4..f7292890b6a2 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -60,6 +60,7 @@ struct fs_parameter;
enum fs_value_type;
struct watch;
struct watch_notification;
+struct lsm_ctx;

/* Default (no) options for the capable function */
#define CAP_OPT_NONE 0x0
@@ -473,6 +474,10 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
unsigned nsops, int alter);
void security_d_instantiate(struct dentry *dentry, struct inode *inode);
+int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
+ size_t __user *size, u32 __user flags);
+int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
+ size_t __user size, u32 __user flags);
int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
char **value);
int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
@@ -1343,6 +1348,20 @@ static inline void security_d_instantiate(struct dentry *dentry,
struct inode *inode)
{ }

+static inline int security_getselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx,
+ size_t __user *size, u32 __user flags)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int security_setselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx,
+ size_t __user size, u32 __user flags)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int security_getprocattr(struct task_struct *p, int lsmid,
const char *name, char **value)
{
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 33a0ee3bcb2e..9a94c31bf6b6 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -71,6 +71,7 @@ struct clone_args;
struct open_how;
struct mount_attr;
struct landlock_ruleset_attr;
+struct lsm_ctx;
enum landlock_rule_type;

#include <linux/types.h>
@@ -1058,6 +1059,10 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
unsigned long home_node,
unsigned long flags);
+asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
+ size_t *size, __u32 flags);
+asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
+ size_t size, __u32 flags);

/*
* Architecture-specific system calls
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
index f27c9a9cc376..eeda59a77c02 100644
--- a/include/uapi/linux/lsm.h
+++ b/include/uapi/linux/lsm.h
@@ -9,6 +9,36 @@
#ifndef _UAPI_LINUX_LSM_H
#define _UAPI_LINUX_LSM_H

+#include <linux/types.h>
+#include <linux/unistd.h>
+
+/**
+ * struct lsm_ctx - LSM context information
+ * @id: the LSM id number, see LSM_ID_XXX
+ * @flags: LSM specific flags
+ * @len: length of the lsm_ctx struct, @ctx and any other data or padding
+ * @ctx_len: the size of @ctx
+ * @ctx: the LSM context value
+ *
+ * The @len field MUST be equal to the size of the lsm_ctx struct
+ * plus any additional padding and/or data placed after @ctx.
+ *
+ * In all cases @ctx_len MUST be equal to the length of @ctx.
+ * If @ctx is a string value it should be nul terminated with
+ * @ctx_len equal to `strlen(@ctx) + 1`. Binary values are
+ * supported.
+ *
+ * The @flags and @ctx fields SHOULD only be interpreted by the
+ * LSM specified by @id; they MUST be set to zero/0 when not used.
+ */
+struct lsm_ctx {
+ __u64 id;
+ __u64 flags;
+ __u64 len;
+ __u64 ctx_len;
+ __u8 ctx[];
+};
+
/*
* ID tokens to identify Linux Security Modules (LSMs)
*
@@ -51,4 +81,10 @@
#define LSM_ATTR_PREV 104
#define LSM_ATTR_SOCKCREATE 105

+/*
+ * LSM_FLAG_XXX definitions identify special handling instructions
+ * for the API.
+ */
+#define LSM_FLAG_SINGLE 0x0001
+
#endif /* _UAPI_LINUX_LSM_H */
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 860b2dcf3ac4..d03c78ef1562 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -262,6 +262,10 @@ COND_SYSCALL_COMPAT(recvmsg);
/* mm/nommu.c, also with MMU */
COND_SYSCALL(mremap);

+/* security/lsm_syscalls.c */
+COND_SYSCALL(lsm_get_self_attr);
+COND_SYSCALL(lsm_set_self_attr);
+
/* security/keys/keyctl.c */
COND_SYSCALL(add_key);
COND_SYSCALL(request_key);
diff --git a/security/Makefile b/security/Makefile
index 18121f8f85cd..59f238490665 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/

# always enable default capabilities
obj-y += commoncap.o
+obj-$(CONFIG_SECURITY) += lsm_syscalls.o
obj-$(CONFIG_MMU) += min_addr.o

# Object file lists
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
new file mode 100644
index 000000000000..feee31600219
--- /dev/null
+++ b/security/lsm_syscalls.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * System calls implementing the Linux Security Module API.
+ *
+ * Copyright (C) 2022 Casey Schaufler <[email protected]>
+ * Copyright (C) 2022 Intel Corporation
+ */
+
+#include <asm/current.h>
+#include <linux/compiler_types.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/security.h>
+#include <linux/stddef.h>
+#include <linux/syscalls.h>
+#include <linux/types.h>
+#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
+
+/**
+ * sys_lsm_set_self_attr - Set current task's security module attribute
+ * @attr: which attribute to set
+ * @ctx: the LSM contexts
+ * @size: size of @ctx
+ * @flags: reserved for future use
+ *
+ * Sets the calling task's LSM context. On success this function
+ * returns 0. If the attribute specified cannot be set a negative
+ * value indicating the reason for the error is returned.
+ */
+SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *,
+ ctx, size_t __user, size, u32, flags)
+{
+ return security_setselfattr(attr, ctx, size, flags);
+}
+
+/**
+ * sys_lsm_get_self_attr - Return current task's security module attributes
+ * @attr: which attribute to set
+ * @ctx: the LSM contexts
+ * @size: size of @ctx, updated on return
+ * @flags: reserved for future use
+ *
+ * Returns the calling task's LSM contexts. On success this
+ * function returns the number of @ctx array elements. This value
+ * may be zero if there are no LSM contexts assigned. If @size is
+ * insufficient to contain the return data -E2BIG is returned and
+ * @size is set to the minimum required size. In all other cases
+ * a negative value indicating the error is returned.
+ */
+SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
+ ctx, size_t __user *, size, u32, flags)
+{
+ return security_getselfattr(attr, ctx, size, flags);
+}
diff --git a/security/security.c b/security/security.c
index 38ca0e646cac..bc3f166b4bff 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2167,6 +2167,116 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
}
EXPORT_SYMBOL(security_d_instantiate);

+/**
+ * security_getselfattr - Read an LSM attribute of the current process.
+ * @attr: which attribute to return
+ * @ctx: the user-space destination for the information, or NULL
+ * @size: the size of space available to receive the data
+ * @flags: special handling options. LSM_FLAG_SINGLE indicates that only
+ * attributes associated with the LSM identified in the passed @ctx be
+ * reported
+ *
+ * Returns the number of attributes found on success, negative value
+ * on error. @size is reset to the total size of the data.
+ * If @size is insufficient to contain the data -E2BIG is returned.
+ */
+int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
+ size_t __user *size, u32 __user flags)
+{
+ struct security_hook_list *hp;
+ struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, };
+ u8 __user *base = (u8 __user *)ctx;
+ size_t total = 0;
+ size_t entrysize;
+ size_t left;
+ bool toobig = false;
+ int count = 0;
+ int rc;
+
+ if (attr == 0)
+ return -EINVAL;
+ if (size == NULL)
+ return -EINVAL;
+ if (get_user(left, size))
+ return -EFAULT;
+
+ if ((flags & LSM_FLAG_SINGLE) == LSM_FLAG_SINGLE) {
+ if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
+ return -EFAULT;
+ if (lctx.id == LSM_ID_UNDEF)
+ return -EINVAL;
+ } else if (flags) {
+ return -EINVAL;
+ }
+
+ hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) {
+ if (lctx.id != LSM_ID_UNDEF && lctx.id != hp->lsmid->id)
+ continue;
+ entrysize = left;
+ if (base)
+ ctx = (struct lsm_ctx __user *)(base + total);
+ rc = hp->hook.getselfattr(attr, ctx, &entrysize, flags);
+ if (rc == -EOPNOTSUPP) {
+ rc = 0;
+ continue;
+ }
+ if (rc == -E2BIG) {
+ toobig = true;
+ left = 0;
+ continue;
+ }
+ if (rc < 0)
+ return rc;
+
+ left -= entrysize;
+ total += entrysize;
+ count += rc;
+ }
+ if (put_user(total, size))
+ return -EFAULT;
+ if (toobig)
+ return -E2BIG;
+ if (count == 0)
+ return LSM_RET_DEFAULT(getselfattr);
+ return count;
+}
+
+/**
+ * security_setselfattr - Set an LSM attribute on the current process.
+ * @attr: which attribute to set
+ * @ctx: the user-space source for the information
+ * @size: the size of the data
+ * @flags: reserved for future use, must be 0
+ *
+ * Set an LSM attribute for the current process. The LSM, attribute
+ * and new value are included in @ctx.
+ *
+ * Returns 0 on success, -EINVAL if the input is inconsistent, -EFAULT
+ * if the user buffer is inaccessible or an LSM specific failure.
+ */
+int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
+ size_t __user size, u32 __user flags)
+{
+ struct security_hook_list *hp;
+ struct lsm_ctx lctx;
+
+ if (flags)
+ return -EINVAL;
+ if (size < sizeof(*ctx))
+ return -EINVAL;
+ if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
+ return -EFAULT;
+ if (size < lctx.len || size < lctx.ctx_len + sizeof(ctx) ||
+ lctx.len < lctx.ctx_len + sizeof(ctx))
+ return -EINVAL;
+
+ hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list)
+ if ((hp->lsmid->id) == lctx.id)
+ return hp->hook.setselfattr(attr, ctx, size, flags);
+
+ return LSM_RET_DEFAULT(setselfattr);
+}
+
int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
char **value)
{
--
2.39.2

2023-04-21 17:47:09

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 06/11] LSM: wireup Linux Security Module syscalls

Wireup lsm_get_self_attr, lsm_set_self_attr and lsm_list_modules
system calls.

Signed-off-by: Casey Schaufler <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Cc: [email protected]
---
arch/alpha/kernel/syscalls/syscall.tbl | 3 +++
arch/arm/tools/syscall.tbl | 3 +++
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 6 ++++++
arch/ia64/kernel/syscalls/syscall.tbl | 3 +++
arch/m68k/kernel/syscalls/syscall.tbl | 3 +++
arch/microblaze/kernel/syscalls/syscall.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n32.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_n64.tbl | 3 +++
arch/mips/kernel/syscalls/syscall_o32.tbl | 3 +++
arch/parisc/kernel/syscalls/syscall.tbl | 3 +++
arch/powerpc/kernel/syscalls/syscall.tbl | 3 +++
arch/s390/kernel/syscalls/syscall.tbl | 3 +++
arch/sh/kernel/syscalls/syscall.tbl | 3 +++
arch/sparc/kernel/syscalls/syscall.tbl | 3 +++
arch/x86/entry/syscalls/syscall_32.tbl | 3 +++
arch/x86/entry/syscalls/syscall_64.tbl | 3 +++
arch/xtensa/kernel/syscalls/syscall.tbl | 3 +++
include/uapi/asm-generic/unistd.h | 11 ++++++++++-
tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl | 3 +++
tools/perf/arch/powerpc/entry/syscalls/syscall.tbl | 3 +++
tools/perf/arch/s390/entry/syscalls/syscall.tbl | 3 +++
tools/perf/arch/x86/entry/syscalls/syscall_64.tbl | 3 +++
23 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 8ebacf37a8cf..178e2792c251 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -490,3 +490,6 @@
558 common process_mrelease sys_process_mrelease
559 common futex_waitv sys_futex_waitv
560 common set_mempolicy_home_node sys_ni_syscall
+561 common lsm_get_self_attr sys_lsm_get_self_attr
+562 common lsm_list_modules sys_lsm_list_modules
+563 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index ac964612d8b0..9cda144f9631 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -464,3 +464,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
index 037feba03a51..6a28fb91b85d 100644
--- a/arch/arm64/include/asm/unistd.h
+++ b/arch/arm64/include/asm/unistd.h
@@ -39,7 +39,7 @@
#define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5)
#define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800)

-#define __NR_compat_syscalls 451
+#define __NR_compat_syscalls 454
#endif

#define __ARCH_WANT_SYS_CLONE
diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
index 604a2053d006..72022ffd5faa 100644
--- a/arch/arm64/include/asm/unistd32.h
+++ b/arch/arm64/include/asm/unistd32.h
@@ -907,6 +907,12 @@ __SYSCALL(__NR_process_mrelease, sys_process_mrelease)
__SYSCALL(__NR_futex_waitv, sys_futex_waitv)
#define __NR_set_mempolicy_home_node 450
__SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node)
+#define __NR_lsm_get_self_attr 451
+__SYSCALL(__NR_lsm_get_self_attr, sys_lsm_get_self_attr)
+#define __NR_lsm_list_modules 452
+__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+#define __NR_lsm_set_self_attr 453
+__SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)

/*
* Please add new compat syscalls above this comment and update
diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
index 72c929d9902b..c52e9d87f47d 100644
--- a/arch/ia64/kernel/syscalls/syscall.tbl
+++ b/arch/ia64/kernel/syscalls/syscall.tbl
@@ -371,3 +371,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index b1f3940bc298..31eac3c99d84 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -450,3 +450,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 820145e47350..5037fa1f74b8 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -456,3 +456,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 253ff994ed2e..29545b3ec587 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -389,3 +389,6 @@
448 n32 process_mrelease sys_process_mrelease
449 n32 futex_waitv sys_futex_waitv
450 n32 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n32 lsm_get_self_attr sys_lsm_get_self_attr
+452 n32 lsm_list_modules sys_lsm_list_modules
+453 n32 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 3f1886ad9d80..8492aa4a771f 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -365,3 +365,6 @@
448 n64 process_mrelease sys_process_mrelease
449 n64 futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n64 lsm_get_self_attr sys_lsm_get_self_attr
+452 n64 lsm_list_modules sys_lsm_list_modules
+453 n64 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 8f243e35a7b2..d74fd86de2a2 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -438,3 +438,6 @@
448 o32 process_mrelease sys_process_mrelease
449 o32 futex_waitv sys_futex_waitv
450 o32 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 o32 lsm_get_self_attr sys_lsm_get_self_attr
+452 o32 lsm_list_modules sys_lsm_list_modules
+453 032 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 0e42fceb2d5e..d1a5f3120d6c 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -448,3 +448,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index a0be127475b1..a414fe8c069b 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -537,3 +537,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 nospu set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 799147658dee..96b7e6b72747 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index 2de85c977f54..1a75a599bb55 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 4398cc6fb68d..80b165091f6f 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -496,3 +496,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 320480a8db4f..130f9feb9eb9 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -455,3 +455,6 @@
448 i386 process_mrelease sys_process_mrelease
449 i386 futex_waitv sys_futex_waitv
450 i386 set_mempolicy_home_node sys_set_mempolicy_home_node
+451 i386 lsm_get_self_attr sys_lsm_get_self_attr
+452 i386 lsm_list_modules sys_lsm_list_modules
+453 i386 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index c84d12608cd2..96dd45bc5988 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -372,6 +372,9 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr

#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 52c94ab5c205..2610aba19802 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -421,3 +421,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 45fa180cc56a..93f89fb06ef5 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -886,8 +886,17 @@ __SYSCALL(__NR_futex_waitv, sys_futex_waitv)
#define __NR_set_mempolicy_home_node 450
__SYSCALL(__NR_set_mempolicy_home_node, sys_set_mempolicy_home_node)

+#define __NR_lsm_get_self_attr 451
+__SYSCALL(__NR_lsm_get_self_attr, sys_lsm_get_self_attr)
+
+#define __NR_lsm_list_modules 452
+__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+
+#define __NR_lsm_set_self_attr 453
+__SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
+
#undef __NR_syscalls
-#define __NR_syscalls 451
+#define __NR_syscalls 454

/*
* 32 bit systems traditionally used different
diff --git a/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl b/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
index 3f1886ad9d80..8492aa4a771f 100644
--- a/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
+++ b/tools/perf/arch/mips/entry/syscalls/syscall_n64.tbl
@@ -365,3 +365,6 @@
448 n64 process_mrelease sys_process_mrelease
449 n64 futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 n64 lsm_get_self_attr sys_lsm_get_self_attr
+452 n64 lsm_list_modules sys_lsm_list_modules
+453 n64 lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
index a0be127475b1..a414fe8c069b 100644
--- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
@@ -537,3 +537,6 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 nospu set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/s390/entry/syscalls/syscall.tbl b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
index 799147658dee..f9257e040109 100644
--- a/tools/perf/arch/s390/entry/syscalls/syscall.tbl
+++ b/tools/perf/arch/s390/entry/syscalls/syscall.tbl
@@ -453,3 +453,6 @@
448 common process_mrelease sys_process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr sys_lsm_set_self_attr
diff --git a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
index c84d12608cd2..96dd45bc5988 100644
--- a/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
@@ -372,6 +372,9 @@
448 common process_mrelease sys_process_mrelease
449 common futex_waitv sys_futex_waitv
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
+451 common lsm_get_self_attr sys_lsm_get_self_attr
+452 common lsm_list_modules sys_lsm_list_modules
+453 common lsm_set_self_attr sys_lsm_set_self_attr

#
# Due to a historical design error, certain syscalls are numbered differently
--
2.39.2

2023-04-21 17:53:15

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 08/11] Smack: implement setselfattr and getselfattr hooks

Implement Smack support for security_[gs]etselfattr.
Refactor the setprocattr hook to avoid code duplication.

Signed-off-by: Casey Schaufler <[email protected]>
---
security/smack/smack_lsm.c | 105 +++++++++++++++++++++++++++++++++++--
1 file changed, 100 insertions(+), 5 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3cf862fcbe08..902b39c187bf 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3552,6 +3552,41 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
return;
}

+/**
+ * smack_getselfattr - Smack current process attribute
+ * @attr: which attribute to fetch
+ * @ctx: buffer to receive the result
+ * @size: available size in, actual size out
+ * @flags: unused
+ *
+ * Fill the passed user space @ctx with the details of the requested
+ * attribute.
+ *
+ * Returns 1, the number of attributes, on success, an error code otherwise.
+ */
+static int smack_getselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t *size,
+ u32 __user flags)
+{
+ struct smack_known *skp = smk_of_current();
+ int total;
+ int slen;
+ int rc = 1;
+
+ if (attr != LSM_ATTR_CURRENT)
+ return -EOPNOTSUPP;
+
+ slen = strlen(skp->smk_known) + 1;
+ total = ALIGN(slen + sizeof(*ctx), 8);
+ if (total > *size)
+ rc = -E2BIG;
+ else
+ lsm_fill_user_ctx(ctx, skp->smk_known, slen, LSM_ID_SMACK, 0);
+
+ *size = total;
+ return rc;
+}
+
/**
* smack_getprocattr - Smack process attribute access
* @p: the object task
@@ -3581,8 +3616,8 @@ static int smack_getprocattr(struct task_struct *p, const char *name, char **val
}

/**
- * smack_setprocattr - Smack process attribute setting
- * @name: the name of the attribute in /proc/.../attr
+ * do_setattr - Smack process attribute setting
+ * @attr: the ID of the attribute
* @value: the value to set
* @size: the size of the value
*
@@ -3591,7 +3626,7 @@ static int smack_getprocattr(struct task_struct *p, const char *name, char **val
*
* Returns the length of the smack label or an error code
*/
-static int smack_setprocattr(const char *name, void *value, size_t size)
+static int do_setattr(u64 attr, void *value, size_t size)
{
struct task_smack *tsp = smack_cred(current_cred());
struct cred *new;
@@ -3605,8 +3640,8 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
if (value == NULL || size == 0 || size >= SMK_LONGLABEL)
return -EINVAL;

- if (strcmp(name, "current") != 0)
- return -EINVAL;
+ if (attr != LSM_ATTR_CURRENT)
+ return -EOPNOTSUPP;

skp = smk_import_entry(value, size);
if (IS_ERR(skp))
@@ -3645,6 +3680,64 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
return size;
}

+/**
+ * smack_setselfattr - Set a Smack process attribute
+ * @attr: which attribute to set
+ * @ctx: buffer containing the data
+ * @size: size of @ctx
+ * @flags: unused
+ *
+ * Fill the passed user space @ctx with the details of the requested
+ * attribute.
+ *
+ * Returns 0 on success, an error code otherwise.
+ */
+static int smack_setselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t __user size,
+ u32 __user flags)
+{
+ struct lsm_ctx *lctx;
+ void *context;
+ int rc;
+
+ context = kmalloc(size, GFP_KERNEL);
+ if (context == NULL)
+ return -ENOMEM;
+
+ lctx = (struct lsm_ctx *)context;
+ if (copy_from_user(context, ctx, size))
+ rc = -EFAULT;
+ else if (lctx->ctx_len > size)
+ rc = -EINVAL;
+ else
+ rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
+
+ kfree(context);
+ if (rc > 0)
+ return 0;
+ return rc;
+}
+
+/**
+ * smack_setprocattr - Smack process attribute setting
+ * @name: the name of the attribute in /proc/.../attr
+ * @value: the value to set
+ * @size: the size of the value
+ *
+ * Sets the Smack value of the task. Only setting self
+ * is permitted and only with privilege
+ *
+ * Returns the length of the smack label or an error code
+ */
+static int smack_setprocattr(const char *name, void *value, size_t size)
+{
+ int attr = lsm_name_to_attr(name);
+
+ if (attr)
+ return do_setattr(attr, value, size);
+ return -EINVAL;
+}
+
/**
* smack_unix_stream_connect - Smack access on UDS
* @sock: one sock
@@ -4955,6 +5048,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {

LSM_HOOK_INIT(d_instantiate, smack_d_instantiate),

+ LSM_HOOK_INIT(getselfattr, smack_getselfattr),
+ LSM_HOOK_INIT(setselfattr, smack_setselfattr),
LSM_HOOK_INIT(getprocattr, smack_getprocattr),
LSM_HOOK_INIT(setprocattr, smack_setprocattr),

--
2.39.2

2023-04-21 17:53:17

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 07/11] LSM: Helpers for attribute names and filling an lsm_ctx

Add lsm_name_to_attr(), which translates a text string to a
LSM_ATTR value if one is available.

Add lsm_fill_user_ctx(), which fills a struct lsm_ctx, including
the trailing attribute value. The .len value is padded to a multiple
of 64 bits

All are used in module specific components of LSM system calls.

Signed-off-by: Casey Schaufler <[email protected]>
---
include/linux/security.h | 13 ++++++++++++
security/lsm_syscalls.c | 24 ++++++++++++++++++++++
security/security.c | 43 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index f7292890b6a2..c96fb56159e3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -263,6 +263,7 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb);
/* prototypes */
extern int security_init(void);
extern int early_security_init(void);
+extern u64 lsm_name_to_attr(const char *name);

/* Security operations */
int security_binder_set_context_mgr(const struct cred *mgr);
@@ -491,6 +492,8 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
int security_locked_down(enum lockdown_reason what);
+int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
+ size_t context_size, u64 id, u64 flags);
#else /* CONFIG_SECURITY */

static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -508,6 +511,11 @@ static inline int unregister_blocking_lsm_notifier(struct notifier_block *nb)
return 0;
}

+static inline u64 lsm_name_to_attr(const char *name)
+{
+ return 0;
+}
+
static inline void security_free_mnt_opts(void **mnt_opts)
{
}
@@ -1420,6 +1428,11 @@ static inline int security_locked_down(enum lockdown_reason what)
{
return 0;
}
+static inline int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
+ size_t context_size, u64 id, u64 flags)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY */

#if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
index 6efbe244d304..67106f642422 100644
--- a/security/lsm_syscalls.c
+++ b/security/lsm_syscalls.c
@@ -17,6 +17,30 @@
#include <linux/lsm_hooks.h>
#include <uapi/linux/lsm.h>

+/**
+ * lsm_name_to_attr - map an LSM attribute name to its ID
+ * @name: name of the attribute
+ *
+ * Returns the LSM attribute value associated with @name, or 0 if
+ * there is no mapping.
+ */
+u64 lsm_name_to_attr(const char *name)
+{
+ if (!strcmp(name, "current"))
+ return LSM_ATTR_CURRENT;
+ if (!strcmp(name, "exec"))
+ return LSM_ATTR_EXEC;
+ if (!strcmp(name, "fscreate"))
+ return LSM_ATTR_FSCREATE;
+ if (!strcmp(name, "keycreate"))
+ return LSM_ATTR_KEYCREATE;
+ if (!strcmp(name, "prev"))
+ return LSM_ATTR_PREV;
+ if (!strcmp(name, "sockcreate"))
+ return LSM_ATTR_SOCKCREATE;
+ return 0;
+}
+
/**
* sys_lsm_set_self_attr - Set current task's security module attribute
* @attr: which attribute to set
diff --git a/security/security.c b/security/security.c
index bc3f166b4bff..759f3d977d2e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -752,6 +752,49 @@ static int lsm_superblock_alloc(struct super_block *sb)
return 0;
}

+/**
+ * lsm_fill_user_ctx - Fill a user space lsm_ctx structure
+ * @ctx: an LSM context to be filled
+ * @context: the new context value
+ * @context_size: the size of the new context value
+ * @id: LSM id
+ * @flags: LSM defined flags
+ *
+ * Fill all of the fields in a user space lsm_ctx structure.
+ * Caller is assumed to have verified that @ctx has enough space
+ * for @context.
+ *
+ * The total length is padded to a multiple of 64 bits.
+ *
+ * Returns 0 on success, -EFAULT on a copyout error, -ENOMEM
+ * if memory can't be allocated.
+ */
+int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
+ size_t context_size, u64 id, u64 flags)
+{
+ struct lsm_ctx *lctx;
+ size_t locallen = ALIGN(context_size + sizeof(*lctx), 8);
+ int rc = 0;
+
+ lctx = kzalloc(locallen, GFP_KERNEL);
+ if (lctx == NULL)
+ return -ENOMEM;
+
+ lctx->id = id;
+ lctx->flags = flags;
+ lctx->ctx_len = context_size;
+ lctx->len = locallen;
+
+ memcpy(&lctx[1], context, context_size);
+
+ if (copy_to_user(ctx, lctx, locallen))
+ rc = -EFAULT;
+
+ kfree(lctx);
+
+ return rc;
+}
+
/*
* The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
* can be accessed with:
--
2.39.2

2023-04-21 18:00:14

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Add hooks for setselfattr and getselfattr. These hooks are not very
different from their setprocattr and getprocattr equivalents, and
much of the code is shared.

Signed-off-by: Casey Schaufler <[email protected]>
Cc: John Johansen <[email protected]>
---
security/apparmor/include/procattr.h | 2 +-
security/apparmor/lsm.c | 96 ++++++++++++++++++++++++++--
security/apparmor/procattr.c | 11 +++-
3 files changed, 99 insertions(+), 10 deletions(-)

diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
index 31689437e0e1..03dbfdb2f2c0 100644
--- a/security/apparmor/include/procattr.h
+++ b/security/apparmor/include/procattr.h
@@ -11,7 +11,7 @@
#ifndef __AA_PROCATTR_H
#define __AA_PROCATTR_H

-int aa_getprocattr(struct aa_label *label, char **string);
+int aa_getprocattr(struct aa_label *label, char **string, bool newline);
int aa_setprocattr_changehat(char *args, size_t size, int flags);

#endif /* __AA_PROCATTR_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index ce6ccb7e06ec..bdaa8bac0404 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -630,6 +630,45 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
return error;
}

+static int apparmor_getselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *lx, size_t *size,
+ u32 __user flags)
+{
+ int error = -ENOENT;
+ struct aa_task_ctx *ctx = task_ctx(current);
+ struct aa_label *label = NULL;
+ size_t total_len;
+ char *value;
+
+ if (attr == LSM_ATTR_CURRENT)
+ label = aa_get_newest_label(cred_label(current_cred()));
+ else if (attr == LSM_ATTR_PREV && ctx->previous)
+ label = aa_get_newest_label(ctx->previous);
+ else if (attr == LSM_ATTR_EXEC && ctx->onexec)
+ label = aa_get_newest_label(ctx->onexec);
+ else
+ error = -EOPNOTSUPP;
+
+ if (label) {
+ error = aa_getprocattr(label, &value, false);
+ if (error > 0) {
+ total_len = ALIGN(error + sizeof(*ctx), 8);
+ if (total_len > *size)
+ error = -E2BIG;
+ else
+ lsm_fill_user_ctx(lx, value, error,
+ LSM_ID_APPARMOR, 0);
+ }
+ }
+
+ aa_put_label(label);
+
+ *size = total_len;
+ if (error > 0)
+ return 1;
+ return error;
+}
+
static int apparmor_getprocattr(struct task_struct *task, const char *name,
char **value)
{
@@ -649,7 +688,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
error = -EINVAL;

if (label)
- error = aa_getprocattr(label, value);
+ error = aa_getprocattr(label, value, true);

aa_put_label(label);
put_cred(cred);
@@ -657,8 +696,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
return error;
}

-static int apparmor_setprocattr(const char *name, void *value,
- size_t size)
+static int do_setattr(u64 attr, void *value, size_t size)
{
char *command, *largs = NULL, *args = value;
size_t arg_size;
@@ -689,7 +727,7 @@ static int apparmor_setprocattr(const char *name, void *value,
goto out;

arg_size = size - (args - (largs ? largs : (char *) value));
- if (strcmp(name, "current") == 0) {
+ if (attr == LSM_ATTR_CURRENT) {
if (strcmp(command, "changehat") == 0) {
error = aa_setprocattr_changehat(args, arg_size,
AA_CHANGE_NOFLAGS);
@@ -704,7 +742,7 @@ static int apparmor_setprocattr(const char *name, void *value,
error = aa_change_profile(args, AA_CHANGE_STACK);
} else
goto fail;
- } else if (strcmp(name, "exec") == 0) {
+ } else if (attr == LSM_ATTR_EXEC) {
if (strcmp(command, "exec") == 0)
error = aa_change_profile(args, AA_CHANGE_ONEXEC);
else if (strcmp(command, "stack") == 0)
@@ -724,13 +762,57 @@ static int apparmor_setprocattr(const char *name, void *value,

fail:
aad(&sa)->label = begin_current_label_crit_section();
- aad(&sa)->info = name;
+ if (attr == LSM_ATTR_CURRENT)
+ aad(&sa)->info = "current";
+ else if (attr == LSM_ATTR_EXEC)
+ aad(&sa)->info = "exec";
+ else
+ aad(&sa)->info = "invalid";
aad(&sa)->error = error = -EINVAL;
aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
end_current_label_crit_section(aad(&sa)->label);
goto out;
}

+static int apparmor_setselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t __user size,
+ u32 __user flags)
+{
+ struct lsm_ctx *lctx;
+ void *context;
+ int rc;
+
+ if (attr != LSM_ATTR_CURRENT && attr != LSM_ATTR_EXEC)
+ return -EOPNOTSUPP;
+
+ context = kmalloc(size, GFP_KERNEL);
+ if (context == NULL)
+ return -ENOMEM;
+
+ lctx = (struct lsm_ctx *)context;
+ if (copy_from_user(context, ctx, size))
+ rc = -EFAULT;
+ else if (lctx->ctx_len > size)
+ rc = -EINVAL;
+ else
+ rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
+
+ kfree(context);
+ if (rc > 0)
+ return 0;
+ return rc;
+}
+
+static int apparmor_setprocattr(const char *name, void *value,
+ size_t size)
+{
+ int attr = lsm_name_to_attr(name);
+
+ if (attr)
+ return do_setattr(attr, value, size);
+ return -EINVAL;
+}
+
/**
* apparmor_bprm_committing_creds - do task cleanup on committing new creds
* @bprm: binprm for the exec (NOT NULL)
@@ -1253,6 +1335,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(file_lock, apparmor_file_lock),
LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),

+ LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
+ LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),

diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
index 197d41f9c32b..196f319aa3b2 100644
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -20,6 +20,7 @@
* aa_getprocattr - Return the label information for @label
* @label: the label to print label info about (NOT NULL)
* @string: Returns - string containing the label info (NOT NULL)
+ * @newline: indicates that a newline should be added
*
* Requires: label != NULL && string != NULL
*
@@ -27,7 +28,7 @@
*
* Returns: size of string placed in @string else error code on failure
*/
-int aa_getprocattr(struct aa_label *label, char **string)
+int aa_getprocattr(struct aa_label *label, char **string, bool newline)
{
struct aa_ns *ns = labels_ns(label);
struct aa_ns *current_ns = aa_get_current_ns();
@@ -57,10 +58,14 @@ int aa_getprocattr(struct aa_label *label, char **string)
return len;
}

- (*string)[len] = '\n';
- (*string)[len + 1] = 0;
+ if (newline)
+ (*string)[len++] = '\n';
+ (*string)[len] = 0;

aa_put_ns(current_ns);
+
+ if (newline)
+ return len;
return len + 1;
}

--
2.39.2

2023-04-21 18:01:10

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 11/11] LSM: selftests for Linux Security Module syscalls

Add selftests for the three system calls supporting the LSM
infrastructure.

Signed-off-by: Casey Schaufler <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/lsm/Makefile | 12 +
tools/testing/selftests/lsm/config | 2 +
.../selftests/lsm/lsm_get_self_attr_test.c | 267 ++++++++++++++++++
.../selftests/lsm/lsm_list_modules_test.c | 149 ++++++++++
.../selftests/lsm/lsm_set_self_attr_test.c | 70 +++++
6 files changed, 501 insertions(+)
create mode 100644 tools/testing/selftests/lsm/Makefile
create mode 100644 tools/testing/selftests/lsm/config
create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_list_modules_test.c
create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 13a6837a0c6b..b18d133a1141 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -38,6 +38,7 @@ TARGETS += landlock
TARGETS += lib
TARGETS += livepatch
TARGETS += lkdtm
+TARGETS += lsm
TARGETS += membarrier
TARGETS += memfd
TARGETS += memory-hotplug
diff --git a/tools/testing/selftests/lsm/Makefile b/tools/testing/selftests/lsm/Makefile
new file mode 100644
index 000000000000..f39a75212b78
--- /dev/null
+++ b/tools/testing/selftests/lsm/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# First run: make -C ../../../.. headers_install
+
+CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
+
+TEST_GEN_PROGS := lsm_get_self_attr_test lsm_list_modules_test \
+ lsm_set_self_attr_test
+
+include ../lib.mk
+
+$(TEST_GEN_PROGS):
diff --git a/tools/testing/selftests/lsm/config b/tools/testing/selftests/lsm/config
new file mode 100644
index 000000000000..afb887715f64
--- /dev/null
+++ b/tools/testing/selftests/lsm/config
@@ -0,0 +1,2 @@
+CONFIG_SYSFS=y
+CONFIG_SECURITY=y
diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
new file mode 100644
index 000000000000..71c2b1a8a44e
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
@@ -0,0 +1,267 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_get_self_attr system call
+ *
+ * Copyright © 2022 Casey Schaufler <[email protected]>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+#define PROCATTR "/proc/self/attr/"
+
+static int read_proc_attr(const char *attr, char *value, __kernel_size_t size)
+{
+ int fd;
+ int len;
+ char *path;
+
+ len = strlen(PROCATTR) + strlen(attr) + 1;
+ path = calloc(len, 1);
+ if (path == NULL)
+ return -1;
+ sprintf(path, "%s%s", PROCATTR, attr);
+
+ fd = open(path, O_RDONLY);
+ free(path);
+
+ if (fd < 0)
+ return -1;
+ len = read(fd, value, size);
+ if (len <= 0)
+ return -1;
+fprintf(stderr, "len=%d\n", len);
+ close(fd);
+
+ path = strchr(value, '\n');
+ if (path)
+ *path = '\0';
+
+ return 0;
+}
+
+static struct lsm_ctx *next_ctx(struct lsm_ctx *ctxp)
+{
+ void *vp;
+
+ vp = (void *)ctxp + sizeof(*ctxp) + ctxp->ctx_len;
+ return (struct lsm_ctx *)vp;
+}
+
+TEST(size_null_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ NULL, 0));
+ ASSERT_EQ(EINVAL, errno);
+
+ free(ctx);
+}
+
+TEST(ctx_null_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, NULL,
+ &size, 0));
+ ASSERT_NE(1, size);
+}
+
+TEST(size_too_small_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = 1;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ &size, 0));
+ ASSERT_EQ(E2BIG, errno);
+ ASSERT_NE(1, size);
+
+ free(ctx);
+}
+
+TEST(flags_zero_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ &size, 1));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(page_size, size);
+
+ free(ctx);
+}
+
+TEST(flags_overset_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr,
+ LSM_ATTR_CURRENT | LSM_ATTR_PREV, ctx, &size, 0));
+ ASSERT_EQ(EOPNOTSUPP, errno);
+
+ free(ctx);
+}
+
+TEST(basic_lsm_get_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+ struct lsm_ctx *ctx = calloc(page_size, 1);
+ struct lsm_ctx *tctx = NULL;
+ __u64 *syscall_lsms = calloc(page_size, 1);
+ char *attr = calloc(page_size, 1);
+ int cnt_current = 0;
+ int cnt_exec = 0;
+ int cnt_fscreate = 0;
+ int cnt_keycreate = 0;
+ int cnt_prev = 0;
+ int cnt_sockcreate = 0;
+ int lsmcount;
+ int count;
+ int i;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_NE(NULL, syscall_lsms);
+
+ lsmcount = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
+ ASSERT_LE(1, lsmcount);
+
+ for (i = 0; i < lsmcount; i++) {
+ switch (syscall_lsms[i]) {
+ case LSM_ID_SELINUX:
+ cnt_current++;
+ cnt_exec++;
+ cnt_fscreate++;
+ cnt_keycreate++;
+ cnt_prev++;
+ cnt_sockcreate++;
+ break;
+ case LSM_ID_SMACK:
+ cnt_current++;
+ break;
+ case LSM_ID_APPARMOR:
+ cnt_current++;
+ cnt_exec++;
+ cnt_prev++;
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (cnt_current) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ &size, 0);
+ ASSERT_EQ(cnt_current, count);
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("current", attr, page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_exec) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_EXEC, ctx,
+ &size, 0);
+ ASSERT_GE(cnt_exec, count);
+ if (count > 0) {
+ tctx = ctx;
+ if (read_proc_attr("exec", attr, page_size) == 0)
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_fscreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_FSCREATE, ctx,
+ &size, 0);
+ ASSERT_GE(cnt_fscreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ if (read_proc_attr("fscreate", attr, page_size) == 0)
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_keycreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_KEYCREATE, ctx,
+ &size, 0);
+ ASSERT_GE(cnt_keycreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ if (read_proc_attr("keycreate", attr, page_size) == 0)
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ if (cnt_prev) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_PREV, ctx,
+ &size, 0);
+ ASSERT_GE(cnt_prev, count);
+ if (count > 0) {
+ tctx = ctx;
+ ASSERT_EQ(0, read_proc_attr("prev", attr, page_size));
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+ }
+ if (cnt_sockcreate) {
+ size = page_size;
+ count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_SOCKCREATE,
+ ctx, &size, 0);
+ ASSERT_GE(cnt_sockcreate, count);
+ if (count > 0) {
+ tctx = ctx;
+ if (read_proc_attr("sockcreate", attr, page_size) == 0)
+ ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
+ }
+ for (i = 1; i < count; i++) {
+ tctx = next_ctx(tctx);
+ ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
+ }
+ }
+
+ free(ctx);
+ free(attr);
+ free(syscall_lsms);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c
new file mode 100644
index 000000000000..3ec814002710
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_list_modules system call
+ *
+ * Copyright © 2022 Casey Schaufler <[email protected]>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+static int read_sysfs_lsms(char *lsms, __kernel_size_t size)
+{
+ FILE *fp;
+
+ fp = fopen("/sys/kernel/security/lsm", "r");
+ if (fp == NULL)
+ return -1;
+ if (fread(lsms, 1, size, fp) <= 0)
+ return -1;
+ fclose(fp);
+ return 0;
+}
+
+TEST(size_null_lsm_list_modules)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, NULL, 0));
+ ASSERT_EQ(EFAULT, errno);
+
+ free(syscall_lsms);
+}
+
+TEST(ids_null_lsm_list_modules)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+
+ ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, NULL, &size, 0));
+ ASSERT_EQ(EFAULT, errno);
+ ASSERT_NE(1, size);
+}
+
+TEST(size_too_small_lsm_list_modules)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+ __kernel_size_t size = 1;
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0));
+ ASSERT_EQ(E2BIG, errno);
+ ASSERT_NE(1, size);
+
+ free(syscall_lsms);
+}
+
+TEST(flags_set_lsm_list_modules)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *syscall_lsms = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 7));
+ ASSERT_EQ(EINVAL, errno);
+ ASSERT_EQ(page_size, size);
+
+ free(syscall_lsms);
+}
+
+TEST(correct_lsm_list_modules)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ __kernel_size_t size = page_size;
+ __u64 *syscall_lsms = calloc(page_size, 1);
+ char *sysfs_lsms = calloc(page_size, 1);
+ char *name;
+ char *cp;
+ int count;
+ int i;
+
+ ASSERT_NE(NULL, sysfs_lsms);
+ ASSERT_NE(NULL, syscall_lsms);
+ ASSERT_EQ(0, read_sysfs_lsms(sysfs_lsms, page_size));
+
+ count = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
+ ASSERT_LE(1, count);
+ cp = sysfs_lsms;
+ for (i = 0; i < count; i++) {
+ switch (syscall_lsms[i]) {
+ case LSM_ID_CAPABILITY:
+ name = "capability";
+ break;
+ case LSM_ID_SELINUX:
+ name = "selinux";
+ break;
+ case LSM_ID_SMACK:
+ name = "smack";
+ break;
+ case LSM_ID_TOMOYO:
+ name = "tomoyo";
+ break;
+ case LSM_ID_IMA:
+ name = "ima";
+ break;
+ case LSM_ID_APPARMOR:
+ name = "apparmor";
+ break;
+ case LSM_ID_YAMA:
+ name = "yama";
+ break;
+ case LSM_ID_LOADPIN:
+ name = "loadpin";
+ break;
+ case LSM_ID_SAFESETID:
+ name = "safesetid";
+ break;
+ case LSM_ID_LOCKDOWN:
+ name = "lockdown";
+ break;
+ case LSM_ID_BPF:
+ name = "bpf";
+ break;
+ case LSM_ID_LANDLOCK:
+ name = "landlock";
+ break;
+ default:
+ name = "INVALID";
+ break;
+ }
+ ASSERT_EQ(0, strncmp(cp, name, strlen(name)));
+ cp += strlen(name) + 1;
+ }
+
+ free(sysfs_lsms);
+ free(syscall_lsms);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
new file mode 100644
index 000000000000..ca538a703168
--- /dev/null
+++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lsm_set_self_attr system call
+ *
+ * Copyright © 2022 Casey Schaufler <[email protected]>
+ * Copyright © 2022 Intel Corporation
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "../kselftest_harness.h"
+
+TEST(ctx_null_lsm_set_self_attr)
+{
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, NULL,
+ sizeof(struct lsm_ctx), 0));
+}
+
+TEST(size_too_small_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ struct lsm_ctx *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ &size, 0));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx, 1,
+ 0));
+
+ free(ctx);
+}
+
+TEST(flags_zero_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
+ &size, 0));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx,
+ size, 1));
+
+ free(ctx);
+}
+
+TEST(flags_overset_lsm_set_self_attr)
+{
+ const long page_size = sysconf(_SC_PAGESIZE);
+ char *ctx = calloc(page_size, 1);
+ __kernel_size_t size = page_size;
+ struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
+
+ ASSERT_NE(NULL, ctx);
+ ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, tctx,
+ &size, 0));
+ ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr,
+ LSM_ATTR_CURRENT | LSM_ATTR_PREV, tctx, size, 0));
+
+ free(ctx);
+}
+
+TEST_HARNESS_MAIN
--
2.39.2

2023-04-21 18:05:56

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v9 10/11] SELinux: Add selfattr hooks

Add hooks for setselfattr and getselfattr. These hooks are not very
different from their setprocattr and getprocattr equivalents, and
much of the code is shared.

Signed-off-by: Casey Schaufler <[email protected]>
Cc: [email protected]
Cc: Paul Moore <[email protected]>
---
security/selinux/hooks.c | 153 +++++++++++++++++++++++++++++++--------
1 file changed, 123 insertions(+), 30 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9403aee75981..9bc6206fb1ef 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6348,8 +6348,8 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
inode_doinit_with_dentry(inode, dentry);
}

-static int selinux_getprocattr(struct task_struct *p,
- const char *name, char **value)
+static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p,
+ char **value)
{
const struct task_security_struct *__tsec;
u32 sid;
@@ -6367,20 +6367,27 @@ static int selinux_getprocattr(struct task_struct *p,
goto bad;
}

- if (!strcmp(name, "current"))
+ switch (attr) {
+ case LSM_ATTR_CURRENT:
sid = __tsec->sid;
- else if (!strcmp(name, "prev"))
+ break;
+ case LSM_ATTR_PREV:
sid = __tsec->osid;
- else if (!strcmp(name, "exec"))
+ break;
+ case LSM_ATTR_EXEC:
sid = __tsec->exec_sid;
- else if (!strcmp(name, "fscreate"))
+ break;
+ case LSM_ATTR_FSCREATE:
sid = __tsec->create_sid;
- else if (!strcmp(name, "keycreate"))
+ break;
+ case LSM_ATTR_KEYCREATE:
sid = __tsec->keycreate_sid;
- else if (!strcmp(name, "sockcreate"))
+ break;
+ case LSM_ATTR_SOCKCREATE:
sid = __tsec->sockcreate_sid;
- else {
- error = -EINVAL;
+ break;
+ default:
+ error = -EOPNOTSUPP;
goto bad;
}
rcu_read_unlock();
@@ -6398,7 +6405,7 @@ static int selinux_getprocattr(struct task_struct *p,
return error;
}

-static int selinux_setprocattr(const char *name, void *value, size_t size)
+static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
{
struct task_security_struct *tsec;
struct cred *new;
@@ -6409,28 +6416,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
/*
* Basic control over ability to set these attributes at all.
*/
- if (!strcmp(name, "exec"))
+ switch (attr) {
+ case LSM_ATTR_CURRENT:
+ error = avc_has_perm(&selinux_state,
+ mysid, mysid, SECCLASS_PROCESS,
+ PROCESS__SETCURRENT, NULL);
+ break;
+ case LSM_ATTR_EXEC:
error = avc_has_perm(&selinux_state,
mysid, mysid, SECCLASS_PROCESS,
PROCESS__SETEXEC, NULL);
- else if (!strcmp(name, "fscreate"))
+ break;
+ case LSM_ATTR_FSCREATE:
error = avc_has_perm(&selinux_state,
mysid, mysid, SECCLASS_PROCESS,
PROCESS__SETFSCREATE, NULL);
- else if (!strcmp(name, "keycreate"))
+ break;
+ case LSM_ATTR_KEYCREATE:
error = avc_has_perm(&selinux_state,
mysid, mysid, SECCLASS_PROCESS,
PROCESS__SETKEYCREATE, NULL);
- else if (!strcmp(name, "sockcreate"))
+ break;
+ case LSM_ATTR_SOCKCREATE:
error = avc_has_perm(&selinux_state,
mysid, mysid, SECCLASS_PROCESS,
PROCESS__SETSOCKCREATE, NULL);
- else if (!strcmp(name, "current"))
- error = avc_has_perm(&selinux_state,
- mysid, mysid, SECCLASS_PROCESS,
- PROCESS__SETCURRENT, NULL);
- else
- error = -EINVAL;
+ break;
+ default:
+ error = -EOPNOTSUPP;
+ break;
+ }
if (error)
return error;

@@ -6442,13 +6457,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
}
error = security_context_to_sid(&selinux_state, value, size,
&sid, GFP_KERNEL);
- if (error == -EINVAL && !strcmp(name, "fscreate")) {
+ if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) {
if (!has_cap_mac_admin(true)) {
struct audit_buffer *ab;
size_t audit_size;

- /* We strip a nul only if it is at the end, otherwise the
- * context contains a nul and we should audit that */
+ /* We strip a nul only if it is at the end,
+ * otherwise the context contains a nul and
+ * we should audit that */
if (str[size - 1] == '\0')
audit_size = size - 1;
else
@@ -6459,7 +6475,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
if (!ab)
return error;
audit_log_format(ab, "op=fscreate invalid_context=");
- audit_log_n_untrustedstring(ab, value, audit_size);
+ audit_log_n_untrustedstring(ab, value,
+ audit_size);
audit_log_end(ab);

return error;
@@ -6483,11 +6500,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
checks and may_create for the file creation checks. The
operation will then fail if the context is not permitted. */
tsec = selinux_cred(new);
- if (!strcmp(name, "exec")) {
+ if (attr == LSM_ATTR_EXEC) {
tsec->exec_sid = sid;
- } else if (!strcmp(name, "fscreate")) {
+ } else if (attr == LSM_ATTR_FSCREATE) {
tsec->create_sid = sid;
- } else if (!strcmp(name, "keycreate")) {
+ } else if (attr == LSM_ATTR_KEYCREATE) {
if (sid) {
error = avc_has_perm(&selinux_state, mysid, sid,
SECCLASS_KEY, KEY__CREATE, NULL);
@@ -6495,9 +6512,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
goto abort_change;
}
tsec->keycreate_sid = sid;
- } else if (!strcmp(name, "sockcreate")) {
+ } else if (attr == LSM_ATTR_SOCKCREATE) {
tsec->sockcreate_sid = sid;
- } else if (!strcmp(name, "current")) {
+ } else if (attr == LSM_ATTR_CURRENT) {
error = -EINVAL;
if (sid == 0)
goto abort_change;
@@ -6542,6 +6559,80 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
return error;
}

+static int selinux_getselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t *size,
+ u32 __user flags)
+{
+ char *value;
+ size_t total_len;
+ int len;
+ int rc = 1;
+
+ len = selinux_lsm_getattr(attr, current, &value);
+ if (len < 0)
+ return len;
+
+ total_len = ALIGN(len + sizeof(*ctx), 8);
+
+ if (total_len > *size)
+ rc = -E2BIG;
+ else
+ lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
+
+ *size = total_len;
+ return rc;
+}
+
+static int selinux_setselfattr(unsigned int __user attr,
+ struct lsm_ctx __user *ctx, size_t __user size,
+ u32 __user flags)
+{
+ struct lsm_ctx *lctx;
+ void *context;
+ int rc;
+
+ context = kmalloc(size, GFP_KERNEL);
+ if (context == NULL)
+ return -ENOMEM;
+
+ lctx = (struct lsm_ctx *)context;
+ if (copy_from_user(context, ctx, size))
+ rc = -EFAULT;
+ else if (lctx->ctx_len > size)
+ rc = -EINVAL;
+ else
+ rc = selinux_lsm_setattr(attr, lctx + 1, lctx->ctx_len);
+
+ kfree(context);
+ if (rc > 0)
+ return 0;
+ return rc;
+}
+
+static int selinux_getprocattr(struct task_struct *p,
+ const char *name, char **value)
+{
+ unsigned int attr = lsm_name_to_attr(name);
+ int rc;
+
+ if (attr) {
+ rc = selinux_lsm_getattr(attr, p, value);
+ if (rc != -EOPNOTSUPP)
+ return rc;
+ }
+
+ return -EINVAL;
+}
+
+static int selinux_setprocattr(const char *name, void *value, size_t size)
+{
+ int attr = lsm_name_to_attr(name);
+
+ if (attr)
+ return selinux_lsm_setattr(attr, value, size);
+ return -EINVAL;
+}
+
static int selinux_ismaclabel(const char *name)
{
return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
@@ -7183,6 +7274,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {

LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate),

+ LSM_HOOK_INIT(getselfattr, selinux_getselfattr),
+ LSM_HOOK_INIT(setselfattr, selinux_setselfattr),
LSM_HOOK_INIT(getprocattr, selinux_getprocattr),
LSM_HOOK_INIT(setprocattr, selinux_setprocattr),

--
2.39.2

2023-04-21 19:25:52

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 01/11] LSM: Identify modules by more than name

On Fri, Apr 21, 2023 at 10:42:49AM -0700, Casey Schaufler wrote:
> Create a struct lsm_id to contain identifying information
> about Linux Security Modules (LSMs). At inception this contains
> the name of the module, an identifier associated with the security
> module and an integer member "attrs" which identifies the API
> related data associated with each security module. The initial set
> of features maps to information that has traditionaly been available
> in /proc/self/attr. They are documented in a new userspace-api file.
> Change the security_add_hooks() interface to use this structure.
> Change the individual modules to maintain their own struct lsm_id
> and pass it to security_add_hooks().
>
> The values are for LSM identifiers are defined in a new UAPI
> header file linux/lsm.h. Each existing LSM has been updated to
> include it's LSMID in the lsm_id.
>
> The LSM ID values are sequential, with the oldest module
> LSM_ID_CAPABILITY being the lowest value and the existing modules
> numbered in the order they were included in the main line kernel.
> This is an arbitrary convention for assigning the values, but
> none better presents itself. The value 0 is defined as being invalid.
> The values 1-99 are reserved for any special case uses which may
> arise in the future. This may include attributes of the LSM
> infrastructure itself, possibly related to namespacing or network
> attribute management. A special range is identified for such attributes
> to help reduce confusion for developers unfamiliar with LSMs.
>
> LSM attribute values are defined for the attributes presented by
> modules that are available today. As with the LSM IDs, The value 0
> is defined as being invalid. The values 1-99 are reserved for any
> special case uses which may arise in the future.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> Cc: linux-security-module <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

Some nit-picks below...

> [...]
> +/**
> + * struct lsm_id - Identify a Linux Security Module.
> + * @lsm: name of the LSM, must be approved by the LSM maintainers
> + * @id: LSM ID number from uapi/linux/lsm.h
> + *
> + * Contains the information that identifies the LSM.
> + */
> +struct lsm_id {
> + const u8 *lsm;

Since this is a %NUL-terminated string, I'd keep the convention of
leaving this as "char", and perhaps even const. And "name" or "lsm_name"
seems more descriptive:

const char *const name;

> + u64 id;

if this is "id", "name" makes sense above.

--
Kees Cook

2023-04-21 19:27:00

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 02/11] LSM: Maintain a table of LSM attribute data

On Fri, Apr 21, 2023 at 10:42:50AM -0700, Casey Schaufler 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 <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

Nit below...

> [...]
> @@ -513,6 +531,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> {
> int i;
>
> + if (lsm_active_cnt >= LSM_COUNT)
> + panic("%s Too many LSMs registered.\n", __func__);
> + /*
> + * 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;
> +

I find this logic hard to parse. I think this might be better, since
lsm_idlist will be entirely initialized to LSM_UNDEF, yes?

/*
* A security module may call security_add_hooks() more
* than once during initialization, and LSM initialization
* is serialized. Landlock is one such case.
*/
if (lsm_idlist[lsm_active_cnt] != lsmid)
lsm_idlist[lsm_active_cnt++] = lsmid;


--
Kees Cook

2023-04-21 19:29:05

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 03/11] proc: Use lsmids instead of lsm names for attrs

On Fri, Apr 21, 2023 at 10:42:51AM -0700, Casey Schaufler wrote:
> Use the LSM ID number instead of the LSM name to identify which
> security module's attibute data should be shown in /proc/self/attr.
> The security_[gs]etprocattr() functions have been changed to expect
> the LSM ID. The change from a string comparison to an integer comparison
> in these functions will provide a minor performance improvement.
>
> Signed-off-by: Casey Schaufler <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-04-21 19:43:47

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 05/11] LSM: Create lsm_list_modules system call

On Fri, Apr 21, 2023 at 10:42:53AM -0700, Casey Schaufler wrote:
> Create a system call to report the list of Linux Security Modules
> that are active on the system. The list is provided as an array
> of LSM ID numbers.
>
> The calling application can use this list determine what LSM
> specific actions it might take. That might include choosing an
> output format, determining required privilege or bypassing
> security module specific behavior.
>
> Signed-off-by: Casey Schaufler <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-04-21 19:46:01

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 04/11] LSM: syscalls for current process attributes

On Fri, Apr 21, 2023 at 10:42:52AM -0700, Casey Schaufler wrote:
> Create a system call lsm_get_self_attr() to provide the security
> module maintained attributes of the current process.
> Create a system call lsm_set_self_attr() to set a security
> module maintained attribute of the current process.
> Historically these attributes have been exposed to user space via
> entries in procfs under /proc/self/attr.
>
> The attribute value is provided in a lsm_ctx structure. The structure
> identifies the size of the attribute, and the attribute value. The format
> of the attribute value is defined by the security module. A flags field
> is included for LSM specific information. It is currently unused and must
> be 0. The total size of the data, including the lsm_ctx structure and any
> padding, is maintained as well.
>
> struct lsm_ctx {
> __u64 id;
> __u64 flags;
> __u64 len;
> __u64 ctx_len;
> __u8 ctx[];
> };
>
> Two new LSM hooks are used to interface with the LSMs.
> security_getselfattr() collects the lsm_ctx values from the
> LSMs that support the hook, accounting for space requirements.
> security_setselfattr() identifies which LSM the attribute is
> intended for and passes it along.
>
> Signed-off-by: Casey Schaufler <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

Nits/questions below...

> ---
> Documentation/userspace-api/lsm.rst | 15 ++++
> include/linux/lsm_hook_defs.h | 4 +
> include/linux/lsm_hooks.h | 9 +++
> include/linux/security.h | 19 +++++
> include/linux/syscalls.h | 5 ++
> include/uapi/linux/lsm.h | 36 +++++++++
> kernel/sys_ni.c | 4 +
> security/Makefile | 1 +
> security/lsm_syscalls.c | 55 ++++++++++++++
> security/security.c | 110 ++++++++++++++++++++++++++++
> 10 files changed, 258 insertions(+)
> create mode 100644 security/lsm_syscalls.c
>
> diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
> index 6ddf5506110b..b45e402302b3 100644
> --- a/Documentation/userspace-api/lsm.rst
> +++ b/Documentation/userspace-api/lsm.rst
> @@ -48,6 +48,21 @@ creating socket objects.
> The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
> This is supported by the SELinux security module.
>
> +Kernel interface
> +================
> +
> +Set a security attribute of the current process
> +--------------------------------------------------
> +
> +.. kernel-doc:: security/lsm_syscalls.c
> + :identifiers: sys_lsm_set_self_attr
> +
> +Get the specified security attributes of the current process
> +--------------------------------------------------
> +
> +.. kernel-doc:: security/lsm_syscalls.c
> + :identifiers: sys_lsm_get_self_attr
> +
> Additional documentation
> ========================
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 094b76dc7164..7177d9554f4a 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -261,6 +261,10 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops,
> LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb)
> LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry,
> struct inode *inode)
> +LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t *size, u32 __user flags)
> +LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t size, u32 __user flags)
> LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
> char **value)
> LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8e6ba0a9896e..ed38ad5eb444 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -25,6 +25,7 @@
> #ifndef __LINUX_LSM_HOOKS_H
> #define __LINUX_LSM_HOOKS_H
>
> +#include <uapi/linux/lsm.h>
> #include <linux/security.h>
> #include <linux/init.h>
> #include <linux/rculist.h>
> @@ -503,6 +504,14 @@
> * and writing the xattrs as this hook is merely a filter.
> * @d_instantiate:
> * Fill in @inode security information for a @dentry if allowed.
> + * @getselfattr:
> + * Read attribute @attr for the current process and store it into @ctx.
> + * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
> + * or another negative value otherwise.
> + * @setselfattr:
> + * Set attribute @attr for the current process.
> + * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
> + * or another negative value otherwise.
> * @getprocattr:
> * Read attribute @name for process @p and store it into @value if allowed.
> * Return the length of @value on success, a negative value otherwise.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8faed81fc3b4..f7292890b6a2 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -60,6 +60,7 @@ struct fs_parameter;
> enum fs_value_type;
> struct watch;
> struct watch_notification;
> +struct lsm_ctx;
>
> /* Default (no) options for the capable function */
> #define CAP_OPT_NONE 0x0
> @@ -473,6 +474,10 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
> int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter);
> void security_d_instantiate(struct dentry *dentry, struct inode *inode);
> +int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,

Scalar values aren't marked with "__user": this is for address space
markings (i.e. only on pointers).

> + size_t __user *size, u32 __user flags);
> +int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> + size_t __user size, u32 __user flags);
> int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
> char **value);
> int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
> @@ -1343,6 +1348,20 @@ static inline void security_d_instantiate(struct dentry *dentry,
> struct inode *inode)
> { }
>
> +static inline int security_getselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx,
> + size_t __user *size, u32 __user flags)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int security_setselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx,
> + size_t __user size, u32 __user flags)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline int security_getprocattr(struct task_struct *p, int lsmid,
> const char *name, char **value)
> {
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 33a0ee3bcb2e..9a94c31bf6b6 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -71,6 +71,7 @@ struct clone_args;
> struct open_how;
> struct mount_attr;
> struct landlock_ruleset_attr;
> +struct lsm_ctx;
> enum landlock_rule_type;
>
> #include <linux/types.h>
> @@ -1058,6 +1059,10 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
> asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
> unsigned long home_node,
> unsigned long flags);
> +asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> + size_t *size, __u32 flags);
> +asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
> + size_t size, __u32 flags);

It'd be nice if these syscalls could stick to the "verionable" syscall
conventions (like openat2) as much as possible. Is "flags" needed here
if we this is only ever going to be 1 LSM at a time?

>
> /*
> * Architecture-specific system calls
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index f27c9a9cc376..eeda59a77c02 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -9,6 +9,36 @@
> #ifndef _UAPI_LINUX_LSM_H
> #define _UAPI_LINUX_LSM_H
>
> +#include <linux/types.h>
> +#include <linux/unistd.h>
> +
> +/**
> + * struct lsm_ctx - LSM context information
> + * @id: the LSM id number, see LSM_ID_XXX
> + * @flags: LSM specific flags
> + * @len: length of the lsm_ctx struct, @ctx and any other data or padding
> + * @ctx_len: the size of @ctx
> + * @ctx: the LSM context value
> + *
> + * The @len field MUST be equal to the size of the lsm_ctx struct
> + * plus any additional padding and/or data placed after @ctx.
> + *
> + * In all cases @ctx_len MUST be equal to the length of @ctx.
> + * If @ctx is a string value it should be nul terminated with
> + * @ctx_len equal to `strlen(@ctx) + 1`. Binary values are
> + * supported.
> + *
> + * The @flags and @ctx fields SHOULD only be interpreted by the
> + * LSM specified by @id; they MUST be set to zero/0 when not used.
> + */
> +struct lsm_ctx {
> + __u64 id;
> + __u64 flags;
> + __u64 len;
> + __u64 ctx_len;
> + __u8 ctx[];
> +};
> +
> /*
> * ID tokens to identify Linux Security Modules (LSMs)
> *
> @@ -51,4 +81,10 @@
> #define LSM_ATTR_PREV 104
> #define LSM_ATTR_SOCKCREATE 105
>
> +/*
> + * LSM_FLAG_XXX definitions identify special handling instructions
> + * for the API.
> + */
> +#define LSM_FLAG_SINGLE 0x0001
> +
> #endif /* _UAPI_LINUX_LSM_H */
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 860b2dcf3ac4..d03c78ef1562 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -262,6 +262,10 @@ COND_SYSCALL_COMPAT(recvmsg);
> /* mm/nommu.c, also with MMU */
> COND_SYSCALL(mremap);
>
> +/* security/lsm_syscalls.c */
> +COND_SYSCALL(lsm_get_self_attr);
> +COND_SYSCALL(lsm_set_self_attr);
> +
> /* security/keys/keyctl.c */
> COND_SYSCALL(add_key);
> COND_SYSCALL(request_key);
> diff --git a/security/Makefile b/security/Makefile
> index 18121f8f85cd..59f238490665 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
>
> # always enable default capabilities
> obj-y += commoncap.o
> +obj-$(CONFIG_SECURITY) += lsm_syscalls.o
> obj-$(CONFIG_MMU) += min_addr.o
>
> # Object file lists
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> new file mode 100644
> index 000000000000..feee31600219
> --- /dev/null
> +++ b/security/lsm_syscalls.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * System calls implementing the Linux Security Module API.
> + *
> + * Copyright (C) 2022 Casey Schaufler <[email protected]>
> + * Copyright (C) 2022 Intel Corporation
> + */
> +
> +#include <asm/current.h>
> +#include <linux/compiler_types.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/security.h>
> +#include <linux/stddef.h>
> +#include <linux/syscalls.h>
> +#include <linux/types.h>
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +
> +/**
> + * sys_lsm_set_self_attr - Set current task's security module attribute
> + * @attr: which attribute to set
> + * @ctx: the LSM contexts
> + * @size: size of @ctx
> + * @flags: reserved for future use
> + *
> + * Sets the calling task's LSM context. On success this function
> + * returns 0. If the attribute specified cannot be set a negative
> + * value indicating the reason for the error is returned.
> + */
> +SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> + ctx, size_t __user, size, u32, flags)
> +{
> + return security_setselfattr(attr, ctx, size, flags);
> +}
> +
> +/**
> + * sys_lsm_get_self_attr - Return current task's security module attributes
> + * @attr: which attribute to set
> + * @ctx: the LSM contexts
> + * @size: size of @ctx, updated on return
> + * @flags: reserved for future use
> + *
> + * Returns the calling task's LSM contexts. On success this
> + * function returns the number of @ctx array elements. This value
> + * may be zero if there are no LSM contexts assigned. If @size is
> + * insufficient to contain the return data -E2BIG is returned and
> + * @size is set to the minimum required size. In all other cases
> + * a negative value indicating the error is returned.
> + */
> +SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> + ctx, size_t __user *, size, u32, flags)
> +{
> + return security_getselfattr(attr, ctx, size, flags);
> +}
> diff --git a/security/security.c b/security/security.c
> index 38ca0e646cac..bc3f166b4bff 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2167,6 +2167,116 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
> }
> EXPORT_SYMBOL(security_d_instantiate);
>
> +/**
> + * security_getselfattr - Read an LSM attribute of the current process.
> + * @attr: which attribute to return
> + * @ctx: the user-space destination for the information, or NULL
> + * @size: the size of space available to receive the data
> + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only
> + * attributes associated with the LSM identified in the passed @ctx be
> + * reported
> + *
> + * Returns the number of attributes found on success, negative value
> + * on error. @size is reset to the total size of the data.
> + * If @size is insufficient to contain the data -E2BIG is returned.
> + */
> +int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> + size_t __user *size, u32 __user flags)
> +{
> + struct security_hook_list *hp;
> + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, };
> + u8 __user *base = (u8 __user *)ctx;
> + size_t total = 0;
> + size_t entrysize;
> + size_t left;
> + bool toobig = false;
> + int count = 0;
> + int rc;
> +
> + if (attr == 0)
> + return -EINVAL;
> + if (size == NULL)
> + return -EINVAL;
> + if (get_user(left, size))
> + return -EFAULT;
> +
> + if ((flags & LSM_FLAG_SINGLE) == LSM_FLAG_SINGLE) {
> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
> + return -EFAULT;
> + if (lctx.id == LSM_ID_UNDEF)
> + return -EINVAL;
> + } else if (flags) {
> + return -EINVAL;
> + }

Can this use copy_struct_from_user() instead? It would be nice to reuse
that here.

> +
> + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) {
> + if (lctx.id != LSM_ID_UNDEF && lctx.id != hp->lsmid->id)
> + continue;
> + entrysize = left;
> + if (base)
> + ctx = (struct lsm_ctx __user *)(base + total);
> + rc = hp->hook.getselfattr(attr, ctx, &entrysize, flags);
> + if (rc == -EOPNOTSUPP) {
> + rc = 0;
> + continue;
> + }
> + if (rc == -E2BIG) {
> + toobig = true;
> + left = 0;
> + continue;
> + }
> + if (rc < 0)
> + return rc;
> +
> + left -= entrysize;
> + total += entrysize;
> + count += rc;
> + }
> + if (put_user(total, size))
> + return -EFAULT;
> + if (toobig)
> + return -E2BIG;
> + if (count == 0)
> + return LSM_RET_DEFAULT(getselfattr);
> + return count;
> +}
> +
> +/**
> + * security_setselfattr - Set an LSM attribute on the current process.
> + * @attr: which attribute to set
> + * @ctx: the user-space source for the information
> + * @size: the size of the data
> + * @flags: reserved for future use, must be 0
> + *
> + * Set an LSM attribute for the current process. The LSM, attribute
> + * and new value are included in @ctx.
> + *
> + * Returns 0 on success, -EINVAL if the input is inconsistent, -EFAULT
> + * if the user buffer is inaccessible or an LSM specific failure.
> + */
> +int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> + size_t __user size, u32 __user flags)
> +{
> + struct security_hook_list *hp;
> + struct lsm_ctx lctx;
> +
> + if (flags)
> + return -EINVAL;
> + if (size < sizeof(*ctx))
> + return -EINVAL;
> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
> + return -EFAULT;
> + if (size < lctx.len || size < lctx.ctx_len + sizeof(ctx) ||
> + lctx.len < lctx.ctx_len + sizeof(ctx))
> + return -EINVAL;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list)
> + if ((hp->lsmid->id) == lctx.id)
> + return hp->hook.setselfattr(attr, ctx, size, flags);
> +
> + return LSM_RET_DEFAULT(setselfattr);
> +}
> +
> int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
> char **value)
> {
> --
> 2.39.2
>

--
Kees Cook

2023-04-21 19:49:16

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 07/11] LSM: Helpers for attribute names and filling an lsm_ctx

On Fri, Apr 21, 2023 at 10:42:55AM -0700, Casey Schaufler wrote:
> Add lsm_name_to_attr(), which translates a text string to a
> LSM_ATTR value if one is available.
>
> Add lsm_fill_user_ctx(), which fills a struct lsm_ctx, including
> the trailing attribute value. The .len value is padded to a multiple
> of 64 bits
>
> All are used in module specific components of LSM system calls.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> ---
> include/linux/security.h | 13 ++++++++++++
> security/lsm_syscalls.c | 24 ++++++++++++++++++++++
> security/security.c | 43 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 80 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index f7292890b6a2..c96fb56159e3 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -263,6 +263,7 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb);
> /* prototypes */
> extern int security_init(void);
> extern int early_security_init(void);
> +extern u64 lsm_name_to_attr(const char *name);
>
> /* Security operations */
> int security_binder_set_context_mgr(const struct cred *mgr);
> @@ -491,6 +492,8 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> int security_locked_down(enum lockdown_reason what);
> +int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
> + size_t context_size, u64 id, u64 flags);
> #else /* CONFIG_SECURITY */
>
> static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> @@ -508,6 +511,11 @@ static inline int unregister_blocking_lsm_notifier(struct notifier_block *nb)
> return 0;
> }
>
> +static inline u64 lsm_name_to_attr(const char *name)
> +{
> + return 0;

I think this should return LSM_ATTR_UNDEF instead of literal 0.

> +}
> +
> static inline void security_free_mnt_opts(void **mnt_opts)
> {
> }
> @@ -1420,6 +1428,11 @@ static inline int security_locked_down(enum lockdown_reason what)
> {
> return 0;
> }
> +static inline int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
> + size_t context_size, u64 id, u64 flags)
> +{
> + return 0;

Shouldn't this either error or fill in an empty context?

> +}
> #endif /* CONFIG_SECURITY */
>
> #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> index 6efbe244d304..67106f642422 100644
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -17,6 +17,30 @@
> #include <linux/lsm_hooks.h>
> #include <uapi/linux/lsm.h>
>
> +/**
> + * lsm_name_to_attr - map an LSM attribute name to its ID
> + * @name: name of the attribute
> + *
> + * Returns the LSM attribute value associated with @name, or 0 if
> + * there is no mapping.
> + */
> +u64 lsm_name_to_attr(const char *name)
> +{
> + if (!strcmp(name, "current"))
> + return LSM_ATTR_CURRENT;
> + if (!strcmp(name, "exec"))
> + return LSM_ATTR_EXEC;
> + if (!strcmp(name, "fscreate"))
> + return LSM_ATTR_FSCREATE;
> + if (!strcmp(name, "keycreate"))
> + return LSM_ATTR_KEYCREATE;
> + if (!strcmp(name, "prev"))
> + return LSM_ATTR_PREV;
> + if (!strcmp(name, "sockcreate"))
> + return LSM_ATTR_SOCKCREATE;
> + return 0;

LSM_ATTR_UNDEF again?

> +}
> +
> /**
> * sys_lsm_set_self_attr - Set current task's security module attribute
> * @attr: which attribute to set
> diff --git a/security/security.c b/security/security.c
> index bc3f166b4bff..759f3d977d2e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -752,6 +752,49 @@ static int lsm_superblock_alloc(struct super_block *sb)
> return 0;
> }
>
> +/**
> + * lsm_fill_user_ctx - Fill a user space lsm_ctx structure
> + * @ctx: an LSM context to be filled
> + * @context: the new context value
> + * @context_size: the size of the new context value
> + * @id: LSM id
> + * @flags: LSM defined flags
> + *
> + * Fill all of the fields in a user space lsm_ctx structure.
> + * Caller is assumed to have verified that @ctx has enough space
> + * for @context.
> + *
> + * The total length is padded to a multiple of 64 bits.

Why?

> + *
> + * Returns 0 on success, -EFAULT on a copyout error, -ENOMEM
> + * if memory can't be allocated.
> + */
> +int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
> + size_t context_size, u64 id, u64 flags)
> +{
> + struct lsm_ctx *lctx;
> + size_t locallen = ALIGN(context_size + sizeof(*lctx), 8);

Instead of the open-coded addition, since you're dealing with a flexible
array structure:

size_t locallen = ALIGN(struct_size(lctx, ctx, context_size), 8);

> + int rc = 0;
> +
> + lctx = kzalloc(locallen, GFP_KERNEL);
> + if (lctx == NULL)
> + return -ENOMEM;
> +
> + lctx->id = id;
> + lctx->flags = flags;
> + lctx->ctx_len = context_size;
> + lctx->len = locallen;
> +
> + memcpy(&lctx[1], context, context_size);

Eek, no: there is no "second struct lsm_ctx" -- this may trip memcpy
overflow checks under certain conditions. You're wanting to copy to
the flexible array at the end, so this should be explicitly performed so
the compiler knows what's going on:

memcpy(lctx->ctx, context, context_size);

> +
> + if (copy_to_user(ctx, lctx, locallen))
> + rc = -EFAULT;
> +
> + kfree(lctx);
> +
> + return rc;
> +}
> +
> /*
> * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
> * can be accessed with:
> --
> 2.39.2
>

--
Kees Cook

2023-04-21 19:53:35

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 06/11] LSM: wireup Linux Security Module syscalls

On Fri, Apr 21, 2023 at 10:42:54AM -0700, Casey Schaufler wrote:
> Wireup lsm_get_self_attr, lsm_set_self_attr and lsm_list_modules
> system calls.
>
> Signed-off-by: Casey Schaufler <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-04-21 19:58:43

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 08/11] Smack: implement setselfattr and getselfattr hooks

On Fri, Apr 21, 2023 at 10:42:56AM -0700, Casey Schaufler wrote:
> Implement Smack support for security_[gs]etselfattr.
> Refactor the setprocattr hook to avoid code duplication.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> ---
> security/smack/smack_lsm.c | 105 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 100 insertions(+), 5 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 3cf862fcbe08..902b39c187bf 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3552,6 +3552,41 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
> return;
> }
>
> +/**
> + * smack_getselfattr - Smack current process attribute
> + * @attr: which attribute to fetch
> + * @ctx: buffer to receive the result
> + * @size: available size in, actual size out
> + * @flags: unused
> + *
> + * Fill the passed user space @ctx with the details of the requested
> + * attribute.
> + *
> + * Returns 1, the number of attributes, on success, an error code otherwise.
> + */
> +static int smack_getselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t *size,
> + u32 __user flags)
> +{
> + struct smack_known *skp = smk_of_current();
> + int total;
> + int slen;
> + int rc = 1;
> +
> + if (attr != LSM_ATTR_CURRENT)
> + return -EOPNOTSUPP;
> +
> + slen = strlen(skp->smk_known) + 1;
> + total = ALIGN(slen + sizeof(*ctx), 8);
> + if (total > *size)
> + rc = -E2BIG;
> + else
> + lsm_fill_user_ctx(ctx, skp->smk_known, slen, LSM_ID_SMACK, 0);
> +
> + *size = total;
> + return rc;
> +}
> +
> /**
> * smack_getprocattr - Smack process attribute access
> * @p: the object task
> @@ -3581,8 +3616,8 @@ static int smack_getprocattr(struct task_struct *p, const char *name, char **val
> }
>
> /**
> - * smack_setprocattr - Smack process attribute setting
> - * @name: the name of the attribute in /proc/.../attr
> + * do_setattr - Smack process attribute setting
> + * @attr: the ID of the attribute
> * @value: the value to set
> * @size: the size of the value
> *
> @@ -3591,7 +3626,7 @@ static int smack_getprocattr(struct task_struct *p, const char *name, char **val
> *
> * Returns the length of the smack label or an error code
> */
> -static int smack_setprocattr(const char *name, void *value, size_t size)
> +static int do_setattr(u64 attr, void *value, size_t size)
> {
> struct task_smack *tsp = smack_cred(current_cred());
> struct cred *new;
> @@ -3605,8 +3640,8 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
> if (value == NULL || size == 0 || size >= SMK_LONGLABEL)
> return -EINVAL;
>
> - if (strcmp(name, "current") != 0)
> - return -EINVAL;
> + if (attr != LSM_ATTR_CURRENT)
> + return -EOPNOTSUPP;
>
> skp = smk_import_entry(value, size);
> if (IS_ERR(skp))
> @@ -3645,6 +3680,64 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
> return size;
> }
>
> +/**
> + * smack_setselfattr - Set a Smack process attribute
> + * @attr: which attribute to set
> + * @ctx: buffer containing the data
> + * @size: size of @ctx
> + * @flags: unused
> + *
> + * Fill the passed user space @ctx with the details of the requested
> + * attribute.
> + *
> + * Returns 0 on success, an error code otherwise.
> + */
> +static int smack_setselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t __user size,

"size_t __user" isn't valid: it's not a pointer.

> + u32 __user flags)
> +{
> + struct lsm_ctx *lctx;
> + void *context;

Why is "context" used here at all? Can't this entirely be lctx?

> + int rc;
> +
> + context = kmalloc(size, GFP_KERNEL);
> + if (context == NULL)
> + return -ENOMEM;
> +
> + lctx = (struct lsm_ctx *)context;
> + if (copy_from_user(context, ctx, size))
> + rc = -EFAULT;
> + else if (lctx->ctx_len > size)
> + rc = -EINVAL;
> + else
> + rc = do_setattr(attr, lctx + 1, lctx->ctx_len);

There is no "second lctx", please pass lctx->ctx instead.

> +
> + kfree(context);
> + if (rc > 0)
> + return 0;
> + return rc;
> +}
> +
> +/**
> + * smack_setprocattr - Smack process attribute setting
> + * @name: the name of the attribute in /proc/.../attr
> + * @value: the value to set
> + * @size: the size of the value
> + *
> + * Sets the Smack value of the task. Only setting self
> + * is permitted and only with privilege
> + *
> + * Returns the length of the smack label or an error code
> + */
> +static int smack_setprocattr(const char *name, void *value, size_t size)
> +{
> + int attr = lsm_name_to_attr(name);
> +
> + if (attr)
> + return do_setattr(attr, value, size);
> + return -EINVAL;
> +}
> +
> /**
> * smack_unix_stream_connect - Smack access on UDS
> * @sock: one sock
> @@ -4955,6 +5048,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
>
> LSM_HOOK_INIT(d_instantiate, smack_d_instantiate),
>
> + LSM_HOOK_INIT(getselfattr, smack_getselfattr),
> + LSM_HOOK_INIT(setselfattr, smack_setselfattr),
> LSM_HOOK_INIT(getprocattr, smack_getprocattr),
> LSM_HOOK_INIT(setprocattr, smack_setprocattr),
>
> --
> 2.39.2
>

--
Kees Cook

2023-04-21 19:58:47

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

On Fri, Apr 21, 2023 at 10:42:57AM -0700, Casey Schaufler wrote:
> Add hooks for setselfattr and getselfattr. These hooks are not very
> different from their setprocattr and getprocattr equivalents, and
> much of the code is shared.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> Cc: John Johansen <[email protected]>
> ---
> security/apparmor/include/procattr.h | 2 +-
> security/apparmor/lsm.c | 96 ++++++++++++++++++++++++++--
> security/apparmor/procattr.c | 11 +++-
> 3 files changed, 99 insertions(+), 10 deletions(-)
>
> diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
> index 31689437e0e1..03dbfdb2f2c0 100644
> --- a/security/apparmor/include/procattr.h
> +++ b/security/apparmor/include/procattr.h
> @@ -11,7 +11,7 @@
> #ifndef __AA_PROCATTR_H
> #define __AA_PROCATTR_H
>
> -int aa_getprocattr(struct aa_label *label, char **string);
> +int aa_getprocattr(struct aa_label *label, char **string, bool newline);
> int aa_setprocattr_changehat(char *args, size_t size, int flags);
>
> #endif /* __AA_PROCATTR_H */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index ce6ccb7e06ec..bdaa8bac0404 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -630,6 +630,45 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
> return error;
> }
>
> +static int apparmor_getselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *lx, size_t *size,
> + u32 __user flags)
> +{
> + int error = -ENOENT;
> + struct aa_task_ctx *ctx = task_ctx(current);
> + struct aa_label *label = NULL;
> + size_t total_len;
> + char *value;
> +
> + if (attr == LSM_ATTR_CURRENT)
> + label = aa_get_newest_label(cred_label(current_cred()));
> + else if (attr == LSM_ATTR_PREV && ctx->previous)
> + label = aa_get_newest_label(ctx->previous);
> + else if (attr == LSM_ATTR_EXEC && ctx->onexec)
> + label = aa_get_newest_label(ctx->onexec);
> + else
> + error = -EOPNOTSUPP;

Is "-EOPNOTSUPP" correct for LSM_ATTR_PREV when !ctx->previous? That
seems like it should be -EINVAL? I think this would be better served as
a switch statement.

> +
> + if (label) {
> + error = aa_getprocattr(label, &value, false);
> + if (error > 0) {
> + total_len = ALIGN(error + sizeof(*ctx), 8);
> + if (total_len > *size)
> + error = -E2BIG;
> + else
> + lsm_fill_user_ctx(lx, value, error,
> + LSM_ID_APPARMOR, 0);
> + }
> + }
> +
> + aa_put_label(label);
> +
> + *size = total_len;
> + if (error > 0)
> + return 1;
> + return error;
> +}
> +
> static int apparmor_getprocattr(struct task_struct *task, const char *name,
> char **value)
> {
> @@ -649,7 +688,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
> error = -EINVAL;
>
> if (label)
> - error = aa_getprocattr(label, value);
> + error = aa_getprocattr(label, value, true);
>
> aa_put_label(label);
> put_cred(cred);
> @@ -657,8 +696,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
> return error;
> }
>
> -static int apparmor_setprocattr(const char *name, void *value,
> - size_t size)
> +static int do_setattr(u64 attr, void *value, size_t size)
> {
> char *command, *largs = NULL, *args = value;
> size_t arg_size;
> @@ -689,7 +727,7 @@ static int apparmor_setprocattr(const char *name, void *value,
> goto out;
>
> arg_size = size - (args - (largs ? largs : (char *) value));
> - if (strcmp(name, "current") == 0) {
> + if (attr == LSM_ATTR_CURRENT) {
> if (strcmp(command, "changehat") == 0) {
> error = aa_setprocattr_changehat(args, arg_size,
> AA_CHANGE_NOFLAGS);
> @@ -704,7 +742,7 @@ static int apparmor_setprocattr(const char *name, void *value,
> error = aa_change_profile(args, AA_CHANGE_STACK);
> } else
> goto fail;
> - } else if (strcmp(name, "exec") == 0) {
> + } else if (attr == LSM_ATTR_EXEC) {
> if (strcmp(command, "exec") == 0)
> error = aa_change_profile(args, AA_CHANGE_ONEXEC);
> else if (strcmp(command, "stack") == 0)
> @@ -724,13 +762,57 @@ static int apparmor_setprocattr(const char *name, void *value,
>
> fail:
> aad(&sa)->label = begin_current_label_crit_section();
> - aad(&sa)->info = name;
> + if (attr == LSM_ATTR_CURRENT)
> + aad(&sa)->info = "current";
> + else if (attr == LSM_ATTR_EXEC)
> + aad(&sa)->info = "exec";
> + else
> + aad(&sa)->info = "invalid";
> aad(&sa)->error = error = -EINVAL;
> aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
> end_current_label_crit_section(aad(&sa)->label);
> goto out;
> }
>
> +static int apparmor_setselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t __user size,
> + u32 __user flags)
> +{
> + struct lsm_ctx *lctx;
> + void *context;
> + int rc;
> +
> + if (attr != LSM_ATTR_CURRENT && attr != LSM_ATTR_EXEC)
> + return -EOPNOTSUPP;
> +
> + context = kmalloc(size, GFP_KERNEL);
> + if (context == NULL)
> + return -ENOMEM;
> +
> + lctx = (struct lsm_ctx *)context;
> + if (copy_from_user(context, ctx, size))
> + rc = -EFAULT;
> + else if (lctx->ctx_len > size)
> + rc = -EINVAL;
> + else
> + rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
> +
> + kfree(context);
> + if (rc > 0)
> + return 0;
> + return rc;
> +}
> +
> +static int apparmor_setprocattr(const char *name, void *value,
> + size_t size)
> +{
> + int attr = lsm_name_to_attr(name);
> +
> + if (attr)
> + return do_setattr(attr, value, size);
> + return -EINVAL;
> +}
> +
> /**
> * apparmor_bprm_committing_creds - do task cleanup on committing new creds
> * @bprm: binprm for the exec (NOT NULL)
> @@ -1253,6 +1335,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(file_lock, apparmor_file_lock),
> LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),
>
> + LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
> + LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
> LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
> LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
>
> diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
> index 197d41f9c32b..196f319aa3b2 100644
> --- a/security/apparmor/procattr.c
> +++ b/security/apparmor/procattr.c
> @@ -20,6 +20,7 @@
> * aa_getprocattr - Return the label information for @label
> * @label: the label to print label info about (NOT NULL)
> * @string: Returns - string containing the label info (NOT NULL)
> + * @newline: indicates that a newline should be added
> *
> * Requires: label != NULL && string != NULL
> *
> @@ -27,7 +28,7 @@
> *
> * Returns: size of string placed in @string else error code on failure
> */
> -int aa_getprocattr(struct aa_label *label, char **string)
> +int aa_getprocattr(struct aa_label *label, char **string, bool newline)
> {
> struct aa_ns *ns = labels_ns(label);
> struct aa_ns *current_ns = aa_get_current_ns();
> @@ -57,10 +58,14 @@ int aa_getprocattr(struct aa_label *label, char **string)
> return len;
> }
>
> - (*string)[len] = '\n';
> - (*string)[len + 1] = 0;
> + if (newline)
> + (*string)[len++] = '\n';
> + (*string)[len] = 0;
>
> aa_put_ns(current_ns);
> +
> + if (newline)
> + return len;
> return len + 1;
> }

This is returning the count including trailing %NUL, yes? Why is this
not always just "return len"? i.e.:

if (newline)
(*string)[len++] = '\n';
(*string)[len++] = 0;

aa_put_ns(current_ns);
return len;


>
> --
> 2.39.2
>

--
Kees Cook

2023-04-21 19:58:52

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 10/11] SELinux: Add selfattr hooks

On Fri, Apr 21, 2023 at 10:42:58AM -0700, Casey Schaufler wrote:
> Add hooks for setselfattr and getselfattr. These hooks are not very
> different from their setprocattr and getprocattr equivalents, and
> much of the code is shared.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> Cc: [email protected]
> Cc: Paul Moore <[email protected]>
> ---
> security/selinux/hooks.c | 153 +++++++++++++++++++++++++++++++--------
> 1 file changed, 123 insertions(+), 30 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9403aee75981..9bc6206fb1ef 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6348,8 +6348,8 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
> inode_doinit_with_dentry(inode, dentry);
> }
>
> -static int selinux_getprocattr(struct task_struct *p,
> - const char *name, char **value)
> +static int selinux_lsm_getattr(unsigned int attr, struct task_struct *p,
> + char **value)
> {
> const struct task_security_struct *__tsec;
> u32 sid;
> @@ -6367,20 +6367,27 @@ static int selinux_getprocattr(struct task_struct *p,
> goto bad;
> }
>
> - if (!strcmp(name, "current"))
> + switch (attr) {
> + case LSM_ATTR_CURRENT:
> sid = __tsec->sid;
> - else if (!strcmp(name, "prev"))
> + break;
> + case LSM_ATTR_PREV:
> sid = __tsec->osid;
> - else if (!strcmp(name, "exec"))
> + break;
> + case LSM_ATTR_EXEC:
> sid = __tsec->exec_sid;
> - else if (!strcmp(name, "fscreate"))
> + break;
> + case LSM_ATTR_FSCREATE:
> sid = __tsec->create_sid;
> - else if (!strcmp(name, "keycreate"))
> + break;
> + case LSM_ATTR_KEYCREATE:
> sid = __tsec->keycreate_sid;
> - else if (!strcmp(name, "sockcreate"))
> + break;
> + case LSM_ATTR_SOCKCREATE:
> sid = __tsec->sockcreate_sid;
> - else {
> - error = -EINVAL;
> + break;
> + default:
> + error = -EOPNOTSUPP;
> goto bad;
> }
> rcu_read_unlock();
> @@ -6398,7 +6405,7 @@ static int selinux_getprocattr(struct task_struct *p,
> return error;
> }
>
> -static int selinux_setprocattr(const char *name, void *value, size_t size)
> +static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> {
> struct task_security_struct *tsec;
> struct cred *new;
> @@ -6409,28 +6416,36 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> /*
> * Basic control over ability to set these attributes at all.
> */
> - if (!strcmp(name, "exec"))
> + switch (attr) {
> + case LSM_ATTR_CURRENT:
> + error = avc_has_perm(&selinux_state,
> + mysid, mysid, SECCLASS_PROCESS,
> + PROCESS__SETCURRENT, NULL);
> + break;
> + case LSM_ATTR_EXEC:
> error = avc_has_perm(&selinux_state,
> mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETEXEC, NULL);
> - else if (!strcmp(name, "fscreate"))
> + break;
> + case LSM_ATTR_FSCREATE:
> error = avc_has_perm(&selinux_state,
> mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETFSCREATE, NULL);
> - else if (!strcmp(name, "keycreate"))
> + break;
> + case LSM_ATTR_KEYCREATE:
> error = avc_has_perm(&selinux_state,
> mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETKEYCREATE, NULL);
> - else if (!strcmp(name, "sockcreate"))
> + break;
> + case LSM_ATTR_SOCKCREATE:
> error = avc_has_perm(&selinux_state,
> mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETSOCKCREATE, NULL);
> - else if (!strcmp(name, "current"))
> - error = avc_has_perm(&selinux_state,
> - mysid, mysid, SECCLASS_PROCESS,
> - PROCESS__SETCURRENT, NULL);
> - else
> - error = -EINVAL;
> + break;
> + default:
> + error = -EOPNOTSUPP;
> + break;
> + }
> if (error)
> return error;
>
> @@ -6442,13 +6457,14 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> }
> error = security_context_to_sid(&selinux_state, value, size,
> &sid, GFP_KERNEL);
> - if (error == -EINVAL && !strcmp(name, "fscreate")) {
> + if (error == -EINVAL && attr == LSM_ATTR_FSCREATE) {
> if (!has_cap_mac_admin(true)) {
> struct audit_buffer *ab;
> size_t audit_size;
>
> - /* We strip a nul only if it is at the end, otherwise the
> - * context contains a nul and we should audit that */
> + /* We strip a nul only if it is at the end,
> + * otherwise the context contains a nul and
> + * we should audit that */
> if (str[size - 1] == '\0')
> audit_size = size - 1;
> else
> @@ -6459,7 +6475,8 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> if (!ab)
> return error;
> audit_log_format(ab, "op=fscreate invalid_context=");
> - audit_log_n_untrustedstring(ab, value, audit_size);
> + audit_log_n_untrustedstring(ab, value,
> + audit_size);
> audit_log_end(ab);
>
> return error;
> @@ -6483,11 +6500,11 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> checks and may_create for the file creation checks. The
> operation will then fail if the context is not permitted. */
> tsec = selinux_cred(new);
> - if (!strcmp(name, "exec")) {
> + if (attr == LSM_ATTR_EXEC) {
> tsec->exec_sid = sid;
> - } else if (!strcmp(name, "fscreate")) {
> + } else if (attr == LSM_ATTR_FSCREATE) {
> tsec->create_sid = sid;
> - } else if (!strcmp(name, "keycreate")) {
> + } else if (attr == LSM_ATTR_KEYCREATE) {
> if (sid) {
> error = avc_has_perm(&selinux_state, mysid, sid,
> SECCLASS_KEY, KEY__CREATE, NULL);
> @@ -6495,9 +6512,9 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> goto abort_change;
> }
> tsec->keycreate_sid = sid;
> - } else if (!strcmp(name, "sockcreate")) {
> + } else if (attr == LSM_ATTR_SOCKCREATE) {
> tsec->sockcreate_sid = sid;
> - } else if (!strcmp(name, "current")) {
> + } else if (attr == LSM_ATTR_CURRENT) {
> error = -EINVAL;
> if (sid == 0)
> goto abort_change;
> @@ -6542,6 +6559,80 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
> return error;
> }
>
> +static int selinux_getselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t *size,
> + u32 __user flags)
> +{
> + char *value;
> + size_t total_len;
> + int len;
> + int rc = 1;
> +
> + len = selinux_lsm_getattr(attr, current, &value);
> + if (len < 0)
> + return len;
> +
> + total_len = ALIGN(len + sizeof(*ctx), 8);

struct_size(ctx, ctx, len)

> +
> + if (total_len > *size)
> + rc = -E2BIG;
> + else
> + lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
> +
> + *size = total_len;
> + return rc;
> +}
> +
> +static int selinux_setselfattr(unsigned int __user attr,
> + struct lsm_ctx __user *ctx, size_t __user size,
> + u32 __user flags)
> +{
> + struct lsm_ctx *lctx;
> + void *context;
> + int rc;
> +
> + context = kmalloc(size, GFP_KERNEL);
> + if (context == NULL)
> + return -ENOMEM;
> +
> + lctx = (struct lsm_ctx *)context;
> + if (copy_from_user(context, ctx, size))
> + rc = -EFAULT;
> + else if (lctx->ctx_len > size)
> + rc = -EINVAL;
> + else
> + rc = selinux_lsm_setattr(attr, lctx + 1, lctx->ctx_len);

Same nits as before:
- "context" isn't needed
- lctx + 1 doesn't exist: lctx->ctx does
- "u32 __user" isn't a sane type

> +
> + kfree(context);
> + if (rc > 0)
> + return 0;
> + return rc;
> +}
> +
> +static int selinux_getprocattr(struct task_struct *p,
> + const char *name, char **value)
> +{
> + unsigned int attr = lsm_name_to_attr(name);
> + int rc;
> +
> + if (attr) {
> + rc = selinux_lsm_getattr(attr, p, value);
> + if (rc != -EOPNOTSUPP)
> + return rc;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int selinux_setprocattr(const char *name, void *value, size_t size)
> +{
> + int attr = lsm_name_to_attr(name);
> +
> + if (attr)
> + return selinux_lsm_setattr(attr, value, size);
> + return -EINVAL;
> +}
> +
> static int selinux_ismaclabel(const char *name)
> {
> return (strcmp(name, XATTR_SELINUX_SUFFIX) == 0);
> @@ -7183,6 +7274,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>
> LSM_HOOK_INIT(d_instantiate, selinux_d_instantiate),
>
> + LSM_HOOK_INIT(getselfattr, selinux_getselfattr),
> + LSM_HOOK_INIT(setselfattr, selinux_setselfattr),
> LSM_HOOK_INIT(getprocattr, selinux_getprocattr),
> LSM_HOOK_INIT(setprocattr, selinux_setprocattr),
>
> --
> 2.39.2
>

--
Kees Cook

2023-04-21 20:11:04

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v9 11/11] LSM: selftests for Linux Security Module syscalls

On Fri, Apr 21, 2023 at 10:42:59AM -0700, Casey Schaufler wrote:
> Add selftests for the three system calls supporting the LSM
> infrastructure.

Yay tests!

With nits below fixed:

Reviewed-by: Kees Cook <[email protected]>

>
> Signed-off-by: Casey Schaufler <[email protected]>
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/lsm/Makefile | 12 +
> tools/testing/selftests/lsm/config | 2 +
> .../selftests/lsm/lsm_get_self_attr_test.c | 267 ++++++++++++++++++
> .../selftests/lsm/lsm_list_modules_test.c | 149 ++++++++++
> .../selftests/lsm/lsm_set_self_attr_test.c | 70 +++++
> 6 files changed, 501 insertions(+)
> create mode 100644 tools/testing/selftests/lsm/Makefile
> create mode 100644 tools/testing/selftests/lsm/config
> create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
> create mode 100644 tools/testing/selftests/lsm/lsm_list_modules_test.c
> create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 13a6837a0c6b..b18d133a1141 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -38,6 +38,7 @@ TARGETS += landlock
> TARGETS += lib
> TARGETS += livepatch
> TARGETS += lkdtm
> +TARGETS += lsm
> TARGETS += membarrier
> TARGETS += memfd
> TARGETS += memory-hotplug
> diff --git a/tools/testing/selftests/lsm/Makefile b/tools/testing/selftests/lsm/Makefile
> new file mode 100644
> index 000000000000..f39a75212b78
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/Makefile
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# First run: make -C ../../../.. headers_install
> +
> +CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
> +
> +TEST_GEN_PROGS := lsm_get_self_attr_test lsm_list_modules_test \
> + lsm_set_self_attr_test
> +
> +include ../lib.mk
> +
> +$(TEST_GEN_PROGS):
> diff --git a/tools/testing/selftests/lsm/config b/tools/testing/selftests/lsm/config
> new file mode 100644
> index 000000000000..afb887715f64
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/config
> @@ -0,0 +1,2 @@
> +CONFIG_SYSFS=y
> +CONFIG_SECURITY=y
> diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
> new file mode 100644
> index 000000000000..71c2b1a8a44e
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
> @@ -0,0 +1,267 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux Security Module infrastructure tests
> + * Tests for the lsm_get_self_attr system call
> + *
> + * Copyright ? 2022 Casey Schaufler <[email protected]>
> + * Copyright ? 2022 Intel Corporation
> + */
> +
> +#define _GNU_SOURCE
> +#include <linux/lsm.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include "../kselftest_harness.h"
> +
> +#define PROCATTR "/proc/self/attr/"
> +
> +static int read_proc_attr(const char *attr, char *value, __kernel_size_t size)
> +{
> + int fd;
> + int len;
> + char *path;
> +
> + len = strlen(PROCATTR) + strlen(attr) + 1;
> + path = calloc(len, 1);
> + if (path == NULL)
> + return -1;
> + sprintf(path, "%s%s", PROCATTR, attr);
> +
> + fd = open(path, O_RDONLY);
> + free(path);
> +
> + if (fd < 0)
> + return -1;
> + len = read(fd, value, size);
> + if (len <= 0)
> + return -1;
> +fprintf(stderr, "len=%d\n", len);

Accidentally leftover debugging?

> + close(fd);
> +
> + path = strchr(value, '\n');
> + if (path)
> + *path = '\0';
> +
> + return 0;
> +}
> +
> +static struct lsm_ctx *next_ctx(struct lsm_ctx *ctxp)
> +{
> + void *vp;
> +
> + vp = (void *)ctxp + sizeof(*ctxp) + ctxp->ctx_len;
> + return (struct lsm_ctx *)vp;
> +}
> +
> +TEST(size_null_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + NULL, 0));
> + ASSERT_EQ(EINVAL, errno);
> +
> + free(ctx);
> +}
> +
> +TEST(ctx_null_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, NULL,
> + &size, 0));
> + ASSERT_NE(1, size);
> +}
> +
> +TEST(size_too_small_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> + __kernel_size_t size = 1;
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + &size, 0));
> + ASSERT_EQ(E2BIG, errno);
> + ASSERT_NE(1, size);
> +
> + free(ctx);
> +}
> +
> +TEST(flags_zero_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(NULL, ctx);

I would explicitly set errno to 0 before syscalls, just to sure.

> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + &size, 1));
> + ASSERT_EQ(EINVAL, errno);
> + ASSERT_EQ(page_size, size);
> +
> + free(ctx);
> +}
> +
> +TEST(flags_overset_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(NULL, ctx);

e.g.:
errno = 0;

but repeated for all syscalls below...

> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr,
> + LSM_ATTR_CURRENT | LSM_ATTR_PREV, ctx, &size, 0));
> + ASSERT_EQ(EOPNOTSUPP, errno);
> +
> + free(ctx);
> +}
> +
> +TEST(basic_lsm_get_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + __kernel_size_t size = page_size;
> + struct lsm_ctx *ctx = calloc(page_size, 1);
> + struct lsm_ctx *tctx = NULL;
> + __u64 *syscall_lsms = calloc(page_size, 1);
> + char *attr = calloc(page_size, 1);
> + int cnt_current = 0;
> + int cnt_exec = 0;
> + int cnt_fscreate = 0;
> + int cnt_keycreate = 0;
> + int cnt_prev = 0;
> + int cnt_sockcreate = 0;
> + int lsmcount;
> + int count;
> + int i;
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_NE(NULL, syscall_lsms);
> +
> + lsmcount = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
> + ASSERT_LE(1, lsmcount);
> +
> + for (i = 0; i < lsmcount; i++) {
> + switch (syscall_lsms[i]) {
> + case LSM_ID_SELINUX:
> + cnt_current++;
> + cnt_exec++;
> + cnt_fscreate++;
> + cnt_keycreate++;
> + cnt_prev++;
> + cnt_sockcreate++;
> + break;
> + case LSM_ID_SMACK:
> + cnt_current++;
> + break;
> + case LSM_ID_APPARMOR:
> + cnt_current++;
> + cnt_exec++;
> + cnt_prev++;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (cnt_current) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + &size, 0);
> + ASSERT_EQ(cnt_current, count);
> + tctx = ctx;
> + ASSERT_EQ(0, read_proc_attr("current", attr, page_size));
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> + if (cnt_exec) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_EXEC, ctx,
> + &size, 0);
> + ASSERT_GE(cnt_exec, count);
> + if (count > 0) {
> + tctx = ctx;
> + if (read_proc_attr("exec", attr, page_size) == 0)
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + }
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> + if (cnt_fscreate) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_FSCREATE, ctx,
> + &size, 0);
> + ASSERT_GE(cnt_fscreate, count);
> + if (count > 0) {
> + tctx = ctx;
> + if (read_proc_attr("fscreate", attr, page_size) == 0)
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + }
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> + if (cnt_keycreate) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_KEYCREATE, ctx,
> + &size, 0);
> + ASSERT_GE(cnt_keycreate, count);
> + if (count > 0) {
> + tctx = ctx;
> + if (read_proc_attr("keycreate", attr, page_size) == 0)
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + }
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> + if (cnt_prev) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_PREV, ctx,
> + &size, 0);
> + ASSERT_GE(cnt_prev, count);
> + if (count > 0) {
> + tctx = ctx;
> + ASSERT_EQ(0, read_proc_attr("prev", attr, page_size));
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> + }
> + if (cnt_sockcreate) {
> + size = page_size;
> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_SOCKCREATE,
> + ctx, &size, 0);
> + ASSERT_GE(cnt_sockcreate, count);
> + if (count > 0) {
> + tctx = ctx;
> + if (read_proc_attr("sockcreate", attr, page_size) == 0)
> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
> + }
> + for (i = 1; i < count; i++) {
> + tctx = next_ctx(tctx);
> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
> + }
> + }
> +
> + free(ctx);
> + free(attr);
> + free(syscall_lsms);
> +}
> +
> +TEST_HARNESS_MAIN
> diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c
> new file mode 100644
> index 000000000000..3ec814002710
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux Security Module infrastructure tests
> + * Tests for the lsm_list_modules system call
> + *
> + * Copyright ? 2022 Casey Schaufler <[email protected]>
> + * Copyright ? 2022 Intel Corporation
> + */
> +
> +#define _GNU_SOURCE
> +#include <linux/lsm.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include "../kselftest_harness.h"
> +
> +static int read_sysfs_lsms(char *lsms, __kernel_size_t size)
> +{
> + FILE *fp;
> +
> + fp = fopen("/sys/kernel/security/lsm", "r");
> + if (fp == NULL)
> + return -1;
> + if (fread(lsms, 1, size, fp) <= 0)
> + return -1;
> + fclose(fp);
> + return 0;
> +}
> +
> +TEST(size_null_lsm_list_modules)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *syscall_lsms = calloc(page_size, 1);
> +
> + ASSERT_NE(NULL, syscall_lsms);
> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, NULL, 0));
> + ASSERT_EQ(EFAULT, errno);
> +
> + free(syscall_lsms);
> +}
> +
> +TEST(ids_null_lsm_list_modules)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, NULL, &size, 0));
> + ASSERT_EQ(EFAULT, errno);
> + ASSERT_NE(1, size);
> +}
> +
> +TEST(size_too_small_lsm_list_modules)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *syscall_lsms = calloc(page_size, 1);
> + __kernel_size_t size = 1;
> +
> + ASSERT_NE(NULL, syscall_lsms);
> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0));
> + ASSERT_EQ(E2BIG, errno);
> + ASSERT_NE(1, size);
> +
> + free(syscall_lsms);
> +}
> +
> +TEST(flags_set_lsm_list_modules)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *syscall_lsms = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(NULL, syscall_lsms);
> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 7));
> + ASSERT_EQ(EINVAL, errno);
> + ASSERT_EQ(page_size, size);
> +
> + free(syscall_lsms);
> +}
> +
> +TEST(correct_lsm_list_modules)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + __kernel_size_t size = page_size;
> + __u64 *syscall_lsms = calloc(page_size, 1);
> + char *sysfs_lsms = calloc(page_size, 1);
> + char *name;
> + char *cp;
> + int count;
> + int i;
> +
> + ASSERT_NE(NULL, sysfs_lsms);
> + ASSERT_NE(NULL, syscall_lsms);
> + ASSERT_EQ(0, read_sysfs_lsms(sysfs_lsms, page_size));
> +
> + count = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
> + ASSERT_LE(1, count);
> + cp = sysfs_lsms;
> + for (i = 0; i < count; i++) {
> + switch (syscall_lsms[i]) {
> + case LSM_ID_CAPABILITY:
> + name = "capability";
> + break;
> + case LSM_ID_SELINUX:
> + name = "selinux";
> + break;
> + case LSM_ID_SMACK:
> + name = "smack";
> + break;
> + case LSM_ID_TOMOYO:
> + name = "tomoyo";
> + break;
> + case LSM_ID_IMA:
> + name = "ima";
> + break;
> + case LSM_ID_APPARMOR:
> + name = "apparmor";
> + break;
> + case LSM_ID_YAMA:
> + name = "yama";
> + break;
> + case LSM_ID_LOADPIN:
> + name = "loadpin";
> + break;
> + case LSM_ID_SAFESETID:
> + name = "safesetid";
> + break;
> + case LSM_ID_LOCKDOWN:
> + name = "lockdown";
> + break;
> + case LSM_ID_BPF:
> + name = "bpf";
> + break;
> + case LSM_ID_LANDLOCK:
> + name = "landlock";
> + break;
> + default:
> + name = "INVALID";
> + break;
> + }
> + ASSERT_EQ(0, strncmp(cp, name, strlen(name)));
> + cp += strlen(name) + 1;
> + }
> +
> + free(sysfs_lsms);
> + free(syscall_lsms);
> +}
> +
> +TEST_HARNESS_MAIN
> diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> new file mode 100644
> index 000000000000..ca538a703168
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux Security Module infrastructure tests
> + * Tests for the lsm_set_self_attr system call
> + *
> + * Copyright ? 2022 Casey Schaufler <[email protected]>
> + * Copyright ? 2022 Intel Corporation
> + */
> +
> +#define _GNU_SOURCE
> +#include <linux/lsm.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include "../kselftest_harness.h"
> +
> +TEST(ctx_null_lsm_set_self_attr)
> +{
> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, NULL,
> + sizeof(struct lsm_ctx), 0));
> +}
> +
> +TEST(size_too_small_lsm_set_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + struct lsm_ctx *ctx = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + &size, 0));
> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx, 1,
> + 0));
> +
> + free(ctx);
> +}
> +
> +TEST(flags_zero_lsm_set_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
> + &size, 0));
> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx,
> + size, 1));
> +
> + free(ctx);
> +}
> +
> +TEST(flags_overset_lsm_set_self_attr)
> +{
> + const long page_size = sysconf(_SC_PAGESIZE);
> + char *ctx = calloc(page_size, 1);
> + __kernel_size_t size = page_size;
> + struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
> +
> + ASSERT_NE(NULL, ctx);
> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, tctx,
> + &size, 0));
> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr,
> + LSM_ATTR_CURRENT | LSM_ATTR_PREV, tctx, size, 0));
> +
> + free(ctx);
> +}
> +
> +TEST_HARNESS_MAIN
> --
> 2.39.2
>

--
Kees Cook

2023-04-22 01:32:31

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base: tip/perf/core
patch link: https://lore.kernel.org/r/20230421174259.2458-10-casey%40schaufler-ca.com
patch subject: [PATCH v9 09/11] AppArmor: Add selfattr hooks
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230422/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
git checkout 2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash security/apparmor/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> security/apparmor/lsm.c:654:7: warning: variable 'total_len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (error > 0) {
^~~~~~~~~
security/apparmor/lsm.c:666:10: note: uninitialized use occurs here
*size = total_len;
^~~~~~~~~
security/apparmor/lsm.c:654:3: note: remove the 'if' if its condition is always true
if (error > 0) {
^~~~~~~~~~~~~~~
security/apparmor/lsm.c:652:6: warning: variable 'total_len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (label) {
^~~~~
security/apparmor/lsm.c:666:10: note: uninitialized use occurs here
*size = total_len;
^~~~~~~~~
security/apparmor/lsm.c:652:2: note: remove the 'if' if its condition is always true
if (label) {
^~~~~~~~~~~
security/apparmor/lsm.c:640:18: note: initialize the variable 'total_len' to silence this warning
size_t total_len;
^
= 0
2 warnings generated.


vim +654 security/apparmor/lsm.c

632
633 static int apparmor_getselfattr(unsigned int __user attr,
634 struct lsm_ctx __user *lx, size_t *size,
635 u32 __user flags)
636 {
637 int error = -ENOENT;
638 struct aa_task_ctx *ctx = task_ctx(current);
639 struct aa_label *label = NULL;
640 size_t total_len;
641 char *value;
642
643 if (attr == LSM_ATTR_CURRENT)
644 label = aa_get_newest_label(cred_label(current_cred()));
645 else if (attr == LSM_ATTR_PREV && ctx->previous)
646 label = aa_get_newest_label(ctx->previous);
647 else if (attr == LSM_ATTR_EXEC && ctx->onexec)
648 label = aa_get_newest_label(ctx->onexec);
649 else
650 error = -EOPNOTSUPP;
651
652 if (label) {
653 error = aa_getprocattr(label, &value, false);
> 654 if (error > 0) {
655 total_len = ALIGN(error + sizeof(*ctx), 8);
656 if (total_len > *size)
657 error = -E2BIG;
658 else
659 lsm_fill_user_ctx(lx, value, error,
660 LSM_ID_APPARMOR, 0);
661 }
662 }
663
664 aa_put_label(label);
665
666 *size = total_len;
667 if (error > 0)
668 return 1;
669 return error;
670 }
671

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

2023-04-22 13:40:38

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v9 04/11] LSM: syscalls for current process attributes

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base: tip/perf/core
patch link: https://lore.kernel.org/r/20230421174259.2458-5-casey%40schaufler-ca.com
patch subject: [PATCH v9 04/11] LSM: syscalls for current process attributes
config: mips-randconfig-s051-20230421 (https://download.01.org/0day-ci/archive/20230422/[email protected]/config)
compiler: mips64el-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/78ea54cc31d7bd9e5a5c7fe8cf34fba9516fde95
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
git checkout 78ea54cc31d7bd9e5a5c7fe8cf34fba9516fde95
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)
>> security/lsm_syscalls.c:34:48: sparse: sparse: dereference of noderef expression
--
security/security.c:406:25: sparse: sparse: cast removes address space '__rcu' of expression
>> security/security.c:2196:13: sparse: sparse: dereference of noderef expression
security/security.c:2203:14: sparse: sparse: dereference of noderef expression
security/security.c:2208:20: sparse: sparse: dereference of noderef expression
security/security.c:2218:43: sparse: sparse: dereference of noderef expression
security/security.c:2218:66: sparse: sparse: dereference of noderef expression
security/security.c:2263:13: sparse: sparse: dereference of noderef expression
security/security.c:2265:13: sparse: sparse: dereference of noderef expression
security/security.c:2269:13: sparse: sparse: dereference of noderef expression
security/security.c:2269:32: sparse: sparse: dereference of noderef expression
security/security.c:2275:53: sparse: sparse: dereference of noderef expression
security/security.c:2275:64: sparse: sparse: dereference of noderef expression
security/security.c:2275:70: sparse: sparse: dereference of noderef expression

vim +34 security/lsm_syscalls.c

19
20 /**
21 * sys_lsm_set_self_attr - Set current task's security module attribute
22 * @attr: which attribute to set
23 * @ctx: the LSM contexts
24 * @size: size of @ctx
25 * @flags: reserved for future use
26 *
27 * Sets the calling task's LSM context. On success this function
28 * returns 0. If the attribute specified cannot be set a negative
29 * value indicating the reason for the error is returned.
30 */
31 SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *,
32 ctx, size_t __user, size, u32, flags)
33 {
> 34 return security_setselfattr(attr, ctx, size, flags);
35 }
36

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

2023-04-22 15:13:25

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base: tip/perf/core
patch link: https://lore.kernel.org/r/20230421174259.2458-10-casey%40schaufler-ca.com
patch subject: [PATCH v9 09/11] AppArmor: Add selfattr hooks
config: mips-randconfig-s051-20230421 (https://download.01.org/0day-ci/archive/20230422/[email protected]/config)
compiler: mips64el-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
git checkout 2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips SHELL=/bin/bash security/apparmor/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)
>> security/apparmor/lsm.c:1339:9: sparse: sparse: incorrect type in initializer (incompatible argument 3 (different address spaces)) @@ expected int ( *setselfattr )( ... ) @@ got int ( * )( ... ) @@
security/apparmor/lsm.c:1339:9: sparse: expected int ( *setselfattr )( ... )
security/apparmor/lsm.c:1339:9: sparse: got int ( * )( ... )
>> security/apparmor/lsm.c:643:13: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:645:18: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:647:18: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:785:13: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:785:41: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:788:27: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:788:27: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:793:42: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:793:42: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:795:34: sparse: sparse: dereference of noderef expression
security/apparmor/lsm.c:798:33: sparse: sparse: dereference of noderef expression

vim +1339 security/apparmor/lsm.c

1305
1306 static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
1307 LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
1308 LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
1309 LSM_HOOK_INIT(capget, apparmor_capget),
1310 LSM_HOOK_INIT(capable, apparmor_capable),
1311
1312 LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
1313 LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
1314 LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
1315
1316 LSM_HOOK_INIT(path_link, apparmor_path_link),
1317 LSM_HOOK_INIT(path_unlink, apparmor_path_unlink),
1318 LSM_HOOK_INIT(path_symlink, apparmor_path_symlink),
1319 LSM_HOOK_INIT(path_mkdir, apparmor_path_mkdir),
1320 LSM_HOOK_INIT(path_rmdir, apparmor_path_rmdir),
1321 LSM_HOOK_INIT(path_mknod, apparmor_path_mknod),
1322 LSM_HOOK_INIT(path_rename, apparmor_path_rename),
1323 LSM_HOOK_INIT(path_chmod, apparmor_path_chmod),
1324 LSM_HOOK_INIT(path_chown, apparmor_path_chown),
1325 LSM_HOOK_INIT(path_truncate, apparmor_path_truncate),
1326 LSM_HOOK_INIT(inode_getattr, apparmor_inode_getattr),
1327
1328 LSM_HOOK_INIT(file_open, apparmor_file_open),
1329 LSM_HOOK_INIT(file_receive, apparmor_file_receive),
1330 LSM_HOOK_INIT(file_permission, apparmor_file_permission),
1331 LSM_HOOK_INIT(file_alloc_security, apparmor_file_alloc_security),
1332 LSM_HOOK_INIT(file_free_security, apparmor_file_free_security),
1333 LSM_HOOK_INIT(mmap_file, apparmor_mmap_file),
1334 LSM_HOOK_INIT(file_mprotect, apparmor_file_mprotect),
1335 LSM_HOOK_INIT(file_lock, apparmor_file_lock),
1336 LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),
1337
1338 LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
> 1339 LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
1340 LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
1341 LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
1342
1343 LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
1344 LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
1345 LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
1346
1347 LSM_HOOK_INIT(socket_create, apparmor_socket_create),
1348 LSM_HOOK_INIT(socket_post_create, apparmor_socket_post_create),
1349 LSM_HOOK_INIT(socket_bind, apparmor_socket_bind),
1350 LSM_HOOK_INIT(socket_connect, apparmor_socket_connect),
1351 LSM_HOOK_INIT(socket_listen, apparmor_socket_listen),
1352 LSM_HOOK_INIT(socket_accept, apparmor_socket_accept),
1353 LSM_HOOK_INIT(socket_sendmsg, apparmor_socket_sendmsg),
1354 LSM_HOOK_INIT(socket_recvmsg, apparmor_socket_recvmsg),
1355 LSM_HOOK_INIT(socket_getsockname, apparmor_socket_getsockname),
1356 LSM_HOOK_INIT(socket_getpeername, apparmor_socket_getpeername),
1357 LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt),
1358 LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt),
1359 LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown),
1360 #ifdef CONFIG_NETWORK_SECMARK
1361 LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb),
1362 #endif
1363 LSM_HOOK_INIT(socket_getpeersec_stream,
1364 apparmor_socket_getpeersec_stream),
1365 LSM_HOOK_INIT(socket_getpeersec_dgram,
1366 apparmor_socket_getpeersec_dgram),
1367 LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
1368 #ifdef CONFIG_NETWORK_SECMARK
1369 LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
1370 #endif
1371
1372 LSM_HOOK_INIT(cred_alloc_blank, apparmor_cred_alloc_blank),
1373 LSM_HOOK_INIT(cred_free, apparmor_cred_free),
1374 LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare),
1375 LSM_HOOK_INIT(cred_transfer, apparmor_cred_transfer),
1376
1377 LSM_HOOK_INIT(bprm_creds_for_exec, apparmor_bprm_creds_for_exec),
1378 LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds),
1379 LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds),
1380
1381 LSM_HOOK_INIT(task_free, apparmor_task_free),
1382 LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
1383 LSM_HOOK_INIT(current_getsecid_subj, apparmor_current_getsecid_subj),
1384 LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid_obj),
1385 LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
1386 LSM_HOOK_INIT(task_kill, apparmor_task_kill),
1387

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

2023-04-22 17:00:41

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v9 04/11] LSM: syscalls for current process attributes

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base: tip/perf/core
patch link: https://lore.kernel.org/r/20230421174259.2458-5-casey%40schaufler-ca.com
patch subject: [PATCH v9 04/11] LSM: syscalls for current process attributes
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/78ea54cc31d7bd9e5a5c7fe8cf34fba9516fde95
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
git checkout 78ea54cc31d7bd9e5a5c7fe8cf34fba9516fde95
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> Documentation/userspace-api/lsm.rst:61: WARNING: Title underline too short.

vim +61 Documentation/userspace-api/lsm.rst

56
57 .. kernel-doc:: security/lsm_syscalls.c
58 :identifiers: sys_lsm_set_self_attr
59
60 Get the specified security attributes of the current process
> 61 --------------------------------------------------
62

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

2023-04-23 08:10:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v9 02/11] LSM: Maintain a table of LSM attribute data


Hello,

kernel test robot noticed "WARNING:at_security/security.c:#append_ordered_lsm" on:

commit: 9898687125b05a8db163332bfa897796e6044da7 ("[PATCH v9 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/20230422-024331
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 15def34e2635ab7e0e96f1bc32e1b69609f14942
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v9 02/11] LSM: Maintain a table of LSM attribute data

in testcase: phoronix-test-suite
version:
with following parameters:

need_x: true
test: jxrendermark-1.2.4
option_a: Transformed Blit Linear
option_b: 32x32
cpufreq_governor: performance

test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added.
test-url: http://www.phoronix-test-suite.com/


compiler: gcc-11
test machine: 12 threads 1 sockets Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-lkp/[email protected]


[ 2.554563][ T0] ------------[ cut here ]------------
[ 2.554563][ T0] builtin: out of LSM slots!?
[ 2.554563][ T0] WARNING: CPU: 0 PID: 0 at security/security.c:173 append_ordered_lsm (security/security.c:173 (discriminator 1))
[ 2.554563][ T0] Modules linked in:
[ 2.554563][ T0] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.3.0-rc3-00006-g9898687125b0 #1
[ 2.554563][ T0] Hardware name: Dell Inc. OptiPlex 7060/0C96W1, BIOS 1.4.2 06/11/2019
[ 2.554563][ T0] RIP: 0010:append_ordered_lsm (security/security.c:173 (discriminator 1))
[ 2.554563][ T0] Code: 82 48 0f 45 c8 48 8b 17 48 c7 c7 70 a4 67 82 e9 34 03 e4 fd 48 c7 47 18 88 0c 49 83 eb ae 48 c7 c7 58 a4 67 82 e8 de 97 da fd <0f> 0b e9 cb 34 bd fe 48 c7 c1 8c 7e 65 82 eb cb 66 66 2e 0f 1f 84
All code
========
0: 82 (bad)
1: 48 0f 45 c8 cmovne %rax,%rcx
5: 48 8b 17 mov (%rdi),%rdx
8: 48 c7 c7 70 a4 67 82 mov $0xffffffff8267a470,%rdi
f: e9 34 03 e4 fd jmpq 0xfffffffffde40348
14: 48 c7 47 18 88 0c 49 movq $0xffffffff83490c88,0x18(%rdi)
1b: 83
1c: eb ae jmp 0xffffffffffffffcc
1e: 48 c7 c7 58 a4 67 82 mov $0xffffffff8267a458,%rdi
25: e8 de 97 da fd callq 0xfffffffffdda9808
2a:* 0f 0b ud2 <-- trapping instruction
2c: e9 cb 34 bd fe jmpq 0xfffffffffebd34fc
31: 48 c7 c1 8c 7e 65 82 mov $0xffffffff82657e8c,%rcx
38: eb cb jmp 0x5
3a: 66 data16
3b: 66 data16
3c: 2e cs
3d: 0f .byte 0xf
3e: 1f (bad)
3f: 84 .byte 0x84

Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: e9 cb 34 bd fe jmpq 0xfffffffffebd34d2
7: 48 c7 c1 8c 7e 65 82 mov $0xffffffff82657e8c,%rcx
e: eb cb jmp 0xffffffffffffffdb
10: 66 data16
11: 66 data16
12: 2e cs
13: 0f .byte 0xf
14: 1f (bad)
15: 84 .byte 0x84
[ 2.554563][ T0] RSP: 0000:ffffffff82803eb0 EFLAGS: 00010286
[ 2.554563][ T0] RAX: 0000000000000000 RBX: ffffffff8353be48 RCX: c0000000ffff7fff
[ 2.554563][ T0] RDX: 0000000000000000 RSI: 0000000000027ffb RDI: 0000000000000001
[ 2.554563][ T0] RBP: ffffffff8265acb1 R08: 0000000000000000 R09: 00000000ffff7fff
[ 2.554563][ T0] R10: ffffffff82803d50 R11: ffffffff82bd8d88 R12: ffff888100192300
[ 2.554563][ T0] R13: ffffffff8353bde8 R14: ffff888100192333 R15: 0000000000000001
[ 2.554563][ T0] FS: 0000000000000000(0000) GS:ffff888853e00000(0000) knlGS:0000000000000000
[ 2.554563][ T0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.554563][ T0] CR2: ffff888000000413 CR3: 000000087b418001 CR4: 00000000000606b0
[ 2.554563][ T0] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2.554563][ T0] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 2.554563][ T0] Call Trace:
[ 2.554563][ T0] <TASK>
[ 2.554563][ T0] ordered_lsm_parse (security/security.c:303)
[ 2.554563][ T0] ordered_lsm_init (security/security.c:379)
[ 2.554563][ T0] security_init (security/security.c:459)
[ 2.554563][ T0] start_kernel (init/main.c:1129)
[ 2.554563][ T0] secondary_startup_64_no_verify (arch/x86/kernel/head_64.S:358)
[ 2.554563][ T0] </TASK>
[ 2.554563][ T0] ---[ end trace 0000000000000000 ]---
[ 2.554563][ T0] LSM: initializing lsm=capability,yama,integrity
[ 2.554563][ T0] Yama: becoming mindful.
[ 2.554563][ T0] Mount-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
[ 2.554563][ T0] Mountpoint-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
[ 2.554563][ T0] CPU0: Thermal monitoring enabled (TM1)
[ 2.554563][ T0] process: using mwait in idle threads
[ 2.554563][ T0] Last level iTLB entries: 4KB 64, 2MB 8, 4MB 8
[ 2.554563][ T0] Last level dTLB entries: 4KB 64, 2MB 0, 4MB 0, 1GB 4


To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



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



Attachments:
(No filename) (6.13 kB)
config-6.3.0-rc3-00006-g9898687125b0 (159.65 kB)
job-script (7.73 kB)
dmesg.xz (37.68 kB)
phoronix-test-suite (7.03 kB)
job.yaml (5.06 kB)
reproduce (314.00 B)
Download all attachments

2023-04-26 01:13:31

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH v9 04/11] LSM: syscalls for current process attributes

On 4/21/2023 12:36 PM, Kees Cook wrote:
> On Fri, Apr 21, 2023 at 10:42:52AM -0700, Casey Schaufler wrote:
>> Create a system call lsm_get_self_attr() to provide the security
>> module maintained attributes of the current process.
>> Create a system call lsm_set_self_attr() to set a security
>> module maintained attribute of the current process.
>> Historically these attributes have been exposed to user space via
>> entries in procfs under /proc/self/attr.
>>
>> The attribute value is provided in a lsm_ctx structure. The structure
>> identifies the size of the attribute, and the attribute value. The format
>> of the attribute value is defined by the security module. A flags field
>> is included for LSM specific information. It is currently unused and must
>> be 0. The total size of the data, including the lsm_ctx structure and any
>> padding, is maintained as well.
>>
>> struct lsm_ctx {
>> __u64 id;
>> __u64 flags;
>> __u64 len;
>> __u64 ctx_len;
>> __u8 ctx[];
>> };
>>
>> Two new LSM hooks are used to interface with the LSMs.
>> security_getselfattr() collects the lsm_ctx values from the
>> LSMs that support the hook, accounting for space requirements.
>> security_setselfattr() identifies which LSM the attribute is
>> intended for and passes it along.
>>
>> Signed-off-by: Casey Schaufler <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
>
> Nits/questions below...
>
>> ---
>> Documentation/userspace-api/lsm.rst | 15 ++++
>> include/linux/lsm_hook_defs.h | 4 +
>> include/linux/lsm_hooks.h | 9 +++
>> include/linux/security.h | 19 +++++
>> include/linux/syscalls.h | 5 ++
>> include/uapi/linux/lsm.h | 36 +++++++++
>> kernel/sys_ni.c | 4 +
>> security/Makefile | 1 +
>> security/lsm_syscalls.c | 55 ++++++++++++++
>> security/security.c | 110 ++++++++++++++++++++++++++++
>> 10 files changed, 258 insertions(+)
>> create mode 100644 security/lsm_syscalls.c
>>
>> diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst
>> index 6ddf5506110b..b45e402302b3 100644
>> --- a/Documentation/userspace-api/lsm.rst
>> +++ b/Documentation/userspace-api/lsm.rst
>> @@ -48,6 +48,21 @@ creating socket objects.
>> The proc filesystem provides this value in ``/proc/self/attr/sockcreate``.
>> This is supported by the SELinux security module.
>>
>> +Kernel interface
>> +================
>> +
>> +Set a security attribute of the current process
>> +--------------------------------------------------
>> +
>> +.. kernel-doc:: security/lsm_syscalls.c
>> + :identifiers: sys_lsm_set_self_attr
>> +
>> +Get the specified security attributes of the current process
>> +--------------------------------------------------
>> +
>> +.. kernel-doc:: security/lsm_syscalls.c
>> + :identifiers: sys_lsm_get_self_attr
>> +
>> Additional documentation
>> ========================
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 094b76dc7164..7177d9554f4a 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -261,6 +261,10 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops,
>> LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb)
>> LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry,
>> struct inode *inode)
>> +LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int __user attr,
>> + struct lsm_ctx __user *ctx, size_t *size, u32 __user flags)
>> +LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int __user attr,
>> + struct lsm_ctx __user *ctx, size_t size, u32 __user flags)
>> LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
>> char **value)
>> LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 8e6ba0a9896e..ed38ad5eb444 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -25,6 +25,7 @@
>> #ifndef __LINUX_LSM_HOOKS_H
>> #define __LINUX_LSM_HOOKS_H
>>
>> +#include <uapi/linux/lsm.h>
>> #include <linux/security.h>
>> #include <linux/init.h>
>> #include <linux/rculist.h>
>> @@ -503,6 +504,14 @@
>> * and writing the xattrs as this hook is merely a filter.
>> * @d_instantiate:
>> * Fill in @inode security information for a @dentry if allowed.
>> + * @getselfattr:
>> + * Read attribute @attr for the current process and store it into @ctx.
>> + * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
>> + * or another negative value otherwise.
>> + * @setselfattr:
>> + * Set attribute @attr for the current process.
>> + * Return 0 on success, -EOPNOTSUPP if the attribute is not supported,
>> + * or another negative value otherwise.
>> * @getprocattr:
>> * Read attribute @name for process @p and store it into @value if allowed.
>> * Return the length of @value on success, a negative value otherwise.
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 8faed81fc3b4..f7292890b6a2 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -60,6 +60,7 @@ struct fs_parameter;
>> enum fs_value_type;
>> struct watch;
>> struct watch_notification;
>> +struct lsm_ctx;
>>
>> /* Default (no) options for the capable function */
>> #define CAP_OPT_NONE 0x0
>> @@ -473,6 +474,10 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
>> int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
>> unsigned nsops, int alter);
>> void security_d_instantiate(struct dentry *dentry, struct inode *inode);
>> +int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
> Scalar values aren't marked with "__user": this is for address space
> markings (i.e. only on pointers).
>
>> + size_t __user *size, u32 __user flags);
>> +int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
>> + size_t __user size, u32 __user flags);
>> int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
>> char **value);
>> int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
>> @@ -1343,6 +1348,20 @@ static inline void security_d_instantiate(struct dentry *dentry,
>> struct inode *inode)
>> { }
>>
>> +static inline int security_getselfattr(unsigned int __user attr,
>> + struct lsm_ctx __user *ctx,
>> + size_t __user *size, u32 __user flags)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static inline int security_setselfattr(unsigned int __user attr,
>> + struct lsm_ctx __user *ctx,
>> + size_t __user size, u32 __user flags)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> static inline int security_getprocattr(struct task_struct *p, int lsmid,
>> const char *name, char **value)
>> {
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 33a0ee3bcb2e..9a94c31bf6b6 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -71,6 +71,7 @@ struct clone_args;
>> struct open_how;
>> struct mount_attr;
>> struct landlock_ruleset_attr;
>> +struct lsm_ctx;
>> enum landlock_rule_type;
>>
>> #include <linux/types.h>
>> @@ -1058,6 +1059,10 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
>> asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
>> unsigned long home_node,
>> unsigned long flags);
>> +asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx,
>> + size_t *size, __u32 flags);
>> +asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx,
>> + size_t size, __u32 flags);
> It'd be nice if these syscalls could stick to the "verionable" syscall
> conventions (like openat2) as much as possible. Is "flags" needed here
> if we this is only ever going to be 1 LSM at a time?

Consider a future LSM that allows you to edit an attribute. Viable flags
might be LSM_FLAG_ADD_TO_ATTRIBUTE, LSM_FLAG_DELETE_FROM_ATTRIBUTE,
LSM_FLAG_REPLACE_ATTRIBUTE, LSM_FLAG_SET_ONLY_IF_EMPTY, and so forth.
This could be useful in an Bell & LaPadula implementation, adding or
removing categories on the fly. OK, no B&L would really allow that, but
The point should be clear.

>>
>> /*
>> * Architecture-specific system calls
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index f27c9a9cc376..eeda59a77c02 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -9,6 +9,36 @@
>> #ifndef _UAPI_LINUX_LSM_H
>> #define _UAPI_LINUX_LSM_H
>>
>> +#include <linux/types.h>
>> +#include <linux/unistd.h>
>> +
>> +/**
>> + * struct lsm_ctx - LSM context information
>> + * @id: the LSM id number, see LSM_ID_XXX
>> + * @flags: LSM specific flags
>> + * @len: length of the lsm_ctx struct, @ctx and any other data or padding
>> + * @ctx_len: the size of @ctx
>> + * @ctx: the LSM context value
>> + *
>> + * The @len field MUST be equal to the size of the lsm_ctx struct
>> + * plus any additional padding and/or data placed after @ctx.
>> + *
>> + * In all cases @ctx_len MUST be equal to the length of @ctx.
>> + * If @ctx is a string value it should be nul terminated with
>> + * @ctx_len equal to `strlen(@ctx) + 1`. Binary values are
>> + * supported.
>> + *
>> + * The @flags and @ctx fields SHOULD only be interpreted by the
>> + * LSM specified by @id; they MUST be set to zero/0 when not used.
>> + */
>> +struct lsm_ctx {
>> + __u64 id;
>> + __u64 flags;
>> + __u64 len;
>> + __u64 ctx_len;
>> + __u8 ctx[];
>> +};
>> +
>> /*
>> * ID tokens to identify Linux Security Modules (LSMs)
>> *
>> @@ -51,4 +81,10 @@
>> #define LSM_ATTR_PREV 104
>> #define LSM_ATTR_SOCKCREATE 105
>>
>> +/*
>> + * LSM_FLAG_XXX definitions identify special handling instructions
>> + * for the API.
>> + */
>> +#define LSM_FLAG_SINGLE 0x0001
>> +
>> #endif /* _UAPI_LINUX_LSM_H */
>> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
>> index 860b2dcf3ac4..d03c78ef1562 100644
>> --- a/kernel/sys_ni.c
>> +++ b/kernel/sys_ni.c
>> @@ -262,6 +262,10 @@ COND_SYSCALL_COMPAT(recvmsg);
>> /* mm/nommu.c, also with MMU */
>> COND_SYSCALL(mremap);
>>
>> +/* security/lsm_syscalls.c */
>> +COND_SYSCALL(lsm_get_self_attr);
>> +COND_SYSCALL(lsm_set_self_attr);
>> +
>> /* security/keys/keyctl.c */
>> COND_SYSCALL(add_key);
>> COND_SYSCALL(request_key);
>> diff --git a/security/Makefile b/security/Makefile
>> index 18121f8f85cd..59f238490665 100644
>> --- a/security/Makefile
>> +++ b/security/Makefile
>> @@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
>>
>> # always enable default capabilities
>> obj-y += commoncap.o
>> +obj-$(CONFIG_SECURITY) += lsm_syscalls.o
>> obj-$(CONFIG_MMU) += min_addr.o
>>
>> # Object file lists
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> new file mode 100644
>> index 000000000000..feee31600219
>> --- /dev/null
>> +++ b/security/lsm_syscalls.c
>> @@ -0,0 +1,55 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * System calls implementing the Linux Security Module API.
>> + *
>> + * Copyright (C) 2022 Casey Schaufler <[email protected]>
>> + * Copyright (C) 2022 Intel Corporation
>> + */
>> +
>> +#include <asm/current.h>
>> +#include <linux/compiler_types.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/security.h>
>> +#include <linux/stddef.h>
>> +#include <linux/syscalls.h>
>> +#include <linux/types.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <uapi/linux/lsm.h>
>> +
>> +/**
>> + * sys_lsm_set_self_attr - Set current task's security module attribute
>> + * @attr: which attribute to set
>> + * @ctx: the LSM contexts
>> + * @size: size of @ctx
>> + * @flags: reserved for future use
>> + *
>> + * Sets the calling task's LSM context. On success this function
>> + * returns 0. If the attribute specified cannot be set a negative
>> + * value indicating the reason for the error is returned.
>> + */
>> +SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *,
>> + ctx, size_t __user, size, u32, flags)
>> +{
>> + return security_setselfattr(attr, ctx, size, flags);
>> +}
>> +
>> +/**
>> + * sys_lsm_get_self_attr - Return current task's security module attributes
>> + * @attr: which attribute to set
>> + * @ctx: the LSM contexts
>> + * @size: size of @ctx, updated on return
>> + * @flags: reserved for future use
>> + *
>> + * Returns the calling task's LSM contexts. On success this
>> + * function returns the number of @ctx array elements. This value
>> + * may be zero if there are no LSM contexts assigned. If @size is
>> + * insufficient to contain the return data -E2BIG is returned and
>> + * @size is set to the minimum required size. In all other cases
>> + * a negative value indicating the error is returned.
>> + */
>> +SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
>> + ctx, size_t __user *, size, u32, flags)
>> +{
>> + return security_getselfattr(attr, ctx, size, flags);
>> +}
>> diff --git a/security/security.c b/security/security.c
>> index 38ca0e646cac..bc3f166b4bff 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -2167,6 +2167,116 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
>> }
>> EXPORT_SYMBOL(security_d_instantiate);
>>
>> +/**
>> + * security_getselfattr - Read an LSM attribute of the current process.
>> + * @attr: which attribute to return
>> + * @ctx: the user-space destination for the information, or NULL
>> + * @size: the size of space available to receive the data
>> + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only
>> + * attributes associated with the LSM identified in the passed @ctx be
>> + * reported
>> + *
>> + * Returns the number of attributes found on success, negative value
>> + * on error. @size is reset to the total size of the data.
>> + * If @size is insufficient to contain the data -E2BIG is returned.
>> + */
>> +int security_getselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
>> + size_t __user *size, u32 __user flags)
>> +{
>> + struct security_hook_list *hp;
>> + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, };
>> + u8 __user *base = (u8 __user *)ctx;
>> + size_t total = 0;
>> + size_t entrysize;
>> + size_t left;
>> + bool toobig = false;
>> + int count = 0;
>> + int rc;
>> +
>> + if (attr == 0)
>> + return -EINVAL;
>> + if (size == NULL)
>> + return -EINVAL;
>> + if (get_user(left, size))
>> + return -EFAULT;
>> +
>> + if ((flags & LSM_FLAG_SINGLE) == LSM_FLAG_SINGLE) {
>> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
>> + return -EFAULT;
>> + if (lctx.id == LSM_ID_UNDEF)
>> + return -EINVAL;
>> + } else if (flags) {
>> + return -EINVAL;
>> + }
> Can this use copy_struct_from_user() instead? It would be nice to reuse
> that here.

Yes. Missed that one.

>
>> +
>> + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) {
>> + if (lctx.id != LSM_ID_UNDEF && lctx.id != hp->lsmid->id)
>> + continue;
>> + entrysize = left;
>> + if (base)
>> + ctx = (struct lsm_ctx __user *)(base + total);
>> + rc = hp->hook.getselfattr(attr, ctx, &entrysize, flags);
>> + if (rc == -EOPNOTSUPP) {
>> + rc = 0;
>> + continue;
>> + }
>> + if (rc == -E2BIG) {
>> + toobig = true;
>> + left = 0;
>> + continue;
>> + }
>> + if (rc < 0)
>> + return rc;
>> +
>> + left -= entrysize;
>> + total += entrysize;
>> + count += rc;
>> + }
>> + if (put_user(total, size))
>> + return -EFAULT;
>> + if (toobig)
>> + return -E2BIG;
>> + if (count == 0)
>> + return LSM_RET_DEFAULT(getselfattr);
>> + return count;
>> +}
>> +
>> +/**
>> + * security_setselfattr - Set an LSM attribute on the current process.
>> + * @attr: which attribute to set
>> + * @ctx: the user-space source for the information
>> + * @size: the size of the data
>> + * @flags: reserved for future use, must be 0
>> + *
>> + * Set an LSM attribute for the current process. The LSM, attribute
>> + * and new value are included in @ctx.
>> + *
>> + * Returns 0 on success, -EINVAL if the input is inconsistent, -EFAULT
>> + * if the user buffer is inaccessible or an LSM specific failure.
>> + */
>> +int security_setselfattr(unsigned int __user attr, struct lsm_ctx __user *ctx,
>> + size_t __user size, u32 __user flags)
>> +{
>> + struct security_hook_list *hp;
>> + struct lsm_ctx lctx;
>> +
>> + if (flags)
>> + return -EINVAL;
>> + if (size < sizeof(*ctx))
>> + return -EINVAL;
>> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
>> + return -EFAULT;
>> + if (size < lctx.len || size < lctx.ctx_len + sizeof(ctx) ||
>> + lctx.len < lctx.ctx_len + sizeof(ctx))
>> + return -EINVAL;
>> +
>> + hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list)
>> + if ((hp->lsmid->id) == lctx.id)
>> + return hp->hook.setselfattr(attr, ctx, size, flags);
>> +
>> + return LSM_RET_DEFAULT(setselfattr);
>> +}
>> +
>> int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
>> char **value)
>> {
>> --
>> 2.39.2
>>

2023-04-27 15:36:22

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH v9 02/11] LSM: Maintain a table of LSM attribute data

On 4/21/2023 12:20 PM, Kees Cook wrote:
> On Fri, Apr 21, 2023 at 10:42:50AM -0700, Casey Schaufler 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 <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
>
> Nit below...
>
>> [...]
>> @@ -513,6 +531,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> {
>> int i;
>>
>> + if (lsm_active_cnt >= LSM_COUNT)
>> + panic("%s Too many LSMs registered.\n", __func__);
>> + /*
>> + * 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;
>> +
> I find this logic hard to parse. I think this might be better, since
> lsm_idlist will be entirely initialized to LSM_UNDEF, yes?
>
> /*
> * A security module may call security_add_hooks() more
> * than once during initialization, and LSM initialization
> * is serialized. Landlock is one such case.
> */
> if (lsm_idlist[lsm_active_cnt] != lsmid)
> lsm_idlist[lsm_active_cnt++] = lsmid;

This code won't do the job. lsm_active_count indexes the first unset
entry, not the last set entry.


2023-04-27 16:06:03

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH v9 11/11] LSM: selftests for Linux Security Module syscalls

On 4/21/2023 1:01 PM, Kees Cook wrote:
> On Fri, Apr 21, 2023 at 10:42:59AM -0700, Casey Schaufler wrote:
>> Add selftests for the three system calls supporting the LSM
>> infrastructure.
> Yay tests!
>
> With nits below fixed:
>
> Reviewed-by: Kees Cook <[email protected]>
>
>> Signed-off-by: Casey Schaufler <[email protected]>
>> ---
>> tools/testing/selftests/Makefile | 1 +
>> tools/testing/selftests/lsm/Makefile | 12 +
>> tools/testing/selftests/lsm/config | 2 +
>> .../selftests/lsm/lsm_get_self_attr_test.c | 267 ++++++++++++++++++
>> .../selftests/lsm/lsm_list_modules_test.c | 149 ++++++++++
>> .../selftests/lsm/lsm_set_self_attr_test.c | 70 +++++
>> 6 files changed, 501 insertions(+)
>> create mode 100644 tools/testing/selftests/lsm/Makefile
>> create mode 100644 tools/testing/selftests/lsm/config
>> create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c
>> create mode 100644 tools/testing/selftests/lsm/lsm_list_modules_test.c
>> create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 13a6837a0c6b..b18d133a1141 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -38,6 +38,7 @@ TARGETS += landlock
>> TARGETS += lib
>> TARGETS += livepatch
>> TARGETS += lkdtm
>> +TARGETS += lsm
>> TARGETS += membarrier
>> TARGETS += memfd
>> TARGETS += memory-hotplug
>> diff --git a/tools/testing/selftests/lsm/Makefile b/tools/testing/selftests/lsm/Makefile
>> new file mode 100644
>> index 000000000000..f39a75212b78
>> --- /dev/null
>> +++ b/tools/testing/selftests/lsm/Makefile
>> @@ -0,0 +1,12 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# First run: make -C ../../../.. headers_install
>> +
>> +CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
>> +
>> +TEST_GEN_PROGS := lsm_get_self_attr_test lsm_list_modules_test \
>> + lsm_set_self_attr_test
>> +
>> +include ../lib.mk
>> +
>> +$(TEST_GEN_PROGS):
>> diff --git a/tools/testing/selftests/lsm/config b/tools/testing/selftests/lsm/config
>> new file mode 100644
>> index 000000000000..afb887715f64
>> --- /dev/null
>> +++ b/tools/testing/selftests/lsm/config
>> @@ -0,0 +1,2 @@
>> +CONFIG_SYSFS=y
>> +CONFIG_SECURITY=y
>> diff --git a/tools/testing/selftests/lsm/lsm_get_self_attr_test.c b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
>> new file mode 100644
>> index 000000000000..71c2b1a8a44e
>> --- /dev/null
>> +++ b/tools/testing/selftests/lsm/lsm_get_self_attr_test.c
>> @@ -0,0 +1,267 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Linux Security Module infrastructure tests
>> + * Tests for the lsm_get_self_attr system call
>> + *
>> + * Copyright © 2022 Casey Schaufler <[email protected]>
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <linux/lsm.h>
>> +#include <fcntl.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <sys/types.h>
>> +#include "../kselftest_harness.h"
>> +
>> +#define PROCATTR "/proc/self/attr/"
>> +
>> +static int read_proc_attr(const char *attr, char *value, __kernel_size_t size)
>> +{
>> + int fd;
>> + int len;
>> + char *path;
>> +
>> + len = strlen(PROCATTR) + strlen(attr) + 1;
>> + path = calloc(len, 1);
>> + if (path == NULL)
>> + return -1;
>> + sprintf(path, "%s%s", PROCATTR, attr);
>> +
>> + fd = open(path, O_RDONLY);
>> + free(path);
>> +
>> + if (fd < 0)
>> + return -1;
>> + len = read(fd, value, size);
>> + if (len <= 0)
>> + return -1;
>> +fprintf(stderr, "len=%d\n", len);
> Accidentally leftover debugging?

Yup.


>> + close(fd);
>> +
>> + path = strchr(value, '\n');
>> + if (path)
>> + *path = '\0';
>> +
>> + return 0;
>> +}
>> +
>> +static struct lsm_ctx *next_ctx(struct lsm_ctx *ctxp)
>> +{
>> + void *vp;
>> +
>> + vp = (void *)ctxp + sizeof(*ctxp) + ctxp->ctx_len;
>> + return (struct lsm_ctx *)vp;
>> +}
>> +
>> +TEST(size_null_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + NULL, 0));
>> + ASSERT_EQ(EINVAL, errno);
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(ctx_null_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, NULL,
>> + &size, 0));
>> + ASSERT_NE(1, size);
>> +}
>> +
>> +TEST(size_too_small_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = 1;
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + &size, 0));
>> + ASSERT_EQ(E2BIG, errno);
>> + ASSERT_NE(1, size);
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(flags_zero_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(NULL, ctx);
> I would explicitly set errno to 0 before syscalls, just to sure.

Good idea for the cases where errno gets checked.


>> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + &size, 1));
>> + ASSERT_EQ(EINVAL, errno);
>> + ASSERT_EQ(page_size, size);
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(flags_overset_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(NULL, ctx);
> e.g.:
> errno = 0;
>
> but repeated for all syscalls below...
>
>> + ASSERT_EQ(-1, syscall(__NR_lsm_get_self_attr,
>> + LSM_ATTR_CURRENT | LSM_ATTR_PREV, ctx, &size, 0));
>> + ASSERT_EQ(EOPNOTSUPP, errno);
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(basic_lsm_get_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + __kernel_size_t size = page_size;
>> + struct lsm_ctx *ctx = calloc(page_size, 1);
>> + struct lsm_ctx *tctx = NULL;
>> + __u64 *syscall_lsms = calloc(page_size, 1);
>> + char *attr = calloc(page_size, 1);
>> + int cnt_current = 0;
>> + int cnt_exec = 0;
>> + int cnt_fscreate = 0;
>> + int cnt_keycreate = 0;
>> + int cnt_prev = 0;
>> + int cnt_sockcreate = 0;
>> + int lsmcount;
>> + int count;
>> + int i;
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_NE(NULL, syscall_lsms);
>> +
>> + lsmcount = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
>> + ASSERT_LE(1, lsmcount);
>> +
>> + for (i = 0; i < lsmcount; i++) {
>> + switch (syscall_lsms[i]) {
>> + case LSM_ID_SELINUX:
>> + cnt_current++;
>> + cnt_exec++;
>> + cnt_fscreate++;
>> + cnt_keycreate++;
>> + cnt_prev++;
>> + cnt_sockcreate++;
>> + break;
>> + case LSM_ID_SMACK:
>> + cnt_current++;
>> + break;
>> + case LSM_ID_APPARMOR:
>> + cnt_current++;
>> + cnt_exec++;
>> + cnt_prev++;
>> + break;
>> + default:
>> + break;
>> + }
>> + }
>> +
>> + if (cnt_current) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + &size, 0);
>> + ASSERT_EQ(cnt_current, count);
>> + tctx = ctx;
>> + ASSERT_EQ(0, read_proc_attr("current", attr, page_size));
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> + if (cnt_exec) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_EXEC, ctx,
>> + &size, 0);
>> + ASSERT_GE(cnt_exec, count);
>> + if (count > 0) {
>> + tctx = ctx;
>> + if (read_proc_attr("exec", attr, page_size) == 0)
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> + if (cnt_fscreate) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_FSCREATE, ctx,
>> + &size, 0);
>> + ASSERT_GE(cnt_fscreate, count);
>> + if (count > 0) {
>> + tctx = ctx;
>> + if (read_proc_attr("fscreate", attr, page_size) == 0)
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> + if (cnt_keycreate) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_KEYCREATE, ctx,
>> + &size, 0);
>> + ASSERT_GE(cnt_keycreate, count);
>> + if (count > 0) {
>> + tctx = ctx;
>> + if (read_proc_attr("keycreate", attr, page_size) == 0)
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> + if (cnt_prev) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_PREV, ctx,
>> + &size, 0);
>> + ASSERT_GE(cnt_prev, count);
>> + if (count > 0) {
>> + tctx = ctx;
>> + ASSERT_EQ(0, read_proc_attr("prev", attr, page_size));
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> + }
>> + if (cnt_sockcreate) {
>> + size = page_size;
>> + count = syscall(__NR_lsm_get_self_attr, LSM_ATTR_SOCKCREATE,
>> + ctx, &size, 0);
>> + ASSERT_GE(cnt_sockcreate, count);
>> + if (count > 0) {
>> + tctx = ctx;
>> + if (read_proc_attr("sockcreate", attr, page_size) == 0)
>> + ASSERT_EQ(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + for (i = 1; i < count; i++) {
>> + tctx = next_ctx(tctx);
>> + ASSERT_NE(0, strcmp((char *)tctx->ctx, attr));
>> + }
>> + }
>> +
>> + free(ctx);
>> + free(attr);
>> + free(syscall_lsms);
>> +}
>> +
>> +TEST_HARNESS_MAIN
>> diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c
>> new file mode 100644
>> index 000000000000..3ec814002710
>> --- /dev/null
>> +++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c
>> @@ -0,0 +1,149 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Linux Security Module infrastructure tests
>> + * Tests for the lsm_list_modules system call
>> + *
>> + * Copyright © 2022 Casey Schaufler <[email protected]>
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <linux/lsm.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <sys/types.h>
>> +#include "../kselftest_harness.h"
>> +
>> +static int read_sysfs_lsms(char *lsms, __kernel_size_t size)
>> +{
>> + FILE *fp;
>> +
>> + fp = fopen("/sys/kernel/security/lsm", "r");
>> + if (fp == NULL)
>> + return -1;
>> + if (fread(lsms, 1, size, fp) <= 0)
>> + return -1;
>> + fclose(fp);
>> + return 0;
>> +}
>> +
>> +TEST(size_null_lsm_list_modules)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *syscall_lsms = calloc(page_size, 1);
>> +
>> + ASSERT_NE(NULL, syscall_lsms);
>> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, NULL, 0));
>> + ASSERT_EQ(EFAULT, errno);
>> +
>> + free(syscall_lsms);
>> +}
>> +
>> +TEST(ids_null_lsm_list_modules)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, NULL, &size, 0));
>> + ASSERT_EQ(EFAULT, errno);
>> + ASSERT_NE(1, size);
>> +}
>> +
>> +TEST(size_too_small_lsm_list_modules)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *syscall_lsms = calloc(page_size, 1);
>> + __kernel_size_t size = 1;
>> +
>> + ASSERT_NE(NULL, syscall_lsms);
>> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0));
>> + ASSERT_EQ(E2BIG, errno);
>> + ASSERT_NE(1, size);
>> +
>> + free(syscall_lsms);
>> +}
>> +
>> +TEST(flags_set_lsm_list_modules)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *syscall_lsms = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(NULL, syscall_lsms);
>> + ASSERT_EQ(-1, syscall(__NR_lsm_list_modules, syscall_lsms, &size, 7));
>> + ASSERT_EQ(EINVAL, errno);
>> + ASSERT_EQ(page_size, size);
>> +
>> + free(syscall_lsms);
>> +}
>> +
>> +TEST(correct_lsm_list_modules)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + __kernel_size_t size = page_size;
>> + __u64 *syscall_lsms = calloc(page_size, 1);
>> + char *sysfs_lsms = calloc(page_size, 1);
>> + char *name;
>> + char *cp;
>> + int count;
>> + int i;
>> +
>> + ASSERT_NE(NULL, sysfs_lsms);
>> + ASSERT_NE(NULL, syscall_lsms);
>> + ASSERT_EQ(0, read_sysfs_lsms(sysfs_lsms, page_size));
>> +
>> + count = syscall(__NR_lsm_list_modules, syscall_lsms, &size, 0);
>> + ASSERT_LE(1, count);
>> + cp = sysfs_lsms;
>> + for (i = 0; i < count; i++) {
>> + switch (syscall_lsms[i]) {
>> + case LSM_ID_CAPABILITY:
>> + name = "capability";
>> + break;
>> + case LSM_ID_SELINUX:
>> + name = "selinux";
>> + break;
>> + case LSM_ID_SMACK:
>> + name = "smack";
>> + break;
>> + case LSM_ID_TOMOYO:
>> + name = "tomoyo";
>> + break;
>> + case LSM_ID_IMA:
>> + name = "ima";
>> + break;
>> + case LSM_ID_APPARMOR:
>> + name = "apparmor";
>> + break;
>> + case LSM_ID_YAMA:
>> + name = "yama";
>> + break;
>> + case LSM_ID_LOADPIN:
>> + name = "loadpin";
>> + break;
>> + case LSM_ID_SAFESETID:
>> + name = "safesetid";
>> + break;
>> + case LSM_ID_LOCKDOWN:
>> + name = "lockdown";
>> + break;
>> + case LSM_ID_BPF:
>> + name = "bpf";
>> + break;
>> + case LSM_ID_LANDLOCK:
>> + name = "landlock";
>> + break;
>> + default:
>> + name = "INVALID";
>> + break;
>> + }
>> + ASSERT_EQ(0, strncmp(cp, name, strlen(name)));
>> + cp += strlen(name) + 1;
>> + }
>> +
>> + free(sysfs_lsms);
>> + free(syscall_lsms);
>> +}
>> +
>> +TEST_HARNESS_MAIN
>> diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
>> new file mode 100644
>> index 000000000000..ca538a703168
>> --- /dev/null
>> +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
>> @@ -0,0 +1,70 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Linux Security Module infrastructure tests
>> + * Tests for the lsm_set_self_attr system call
>> + *
>> + * Copyright © 2022 Casey Schaufler <[email protected]>
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <linux/lsm.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <sys/types.h>
>> +#include "../kselftest_harness.h"
>> +
>> +TEST(ctx_null_lsm_set_self_attr)
>> +{
>> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, NULL,
>> + sizeof(struct lsm_ctx), 0));
>> +}
>> +
>> +TEST(size_too_small_lsm_set_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + struct lsm_ctx *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + &size, 0));
>> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx, 1,
>> + 0));
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(flags_zero_lsm_set_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, ctx,
>> + &size, 0));
>> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr, LSM_ATTR_CURRENT, ctx,
>> + size, 1));
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST(flags_overset_lsm_set_self_attr)
>> +{
>> + const long page_size = sysconf(_SC_PAGESIZE);
>> + char *ctx = calloc(page_size, 1);
>> + __kernel_size_t size = page_size;
>> + struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
>> +
>> + ASSERT_NE(NULL, ctx);
>> + ASSERT_GE(1, syscall(__NR_lsm_get_self_attr, LSM_ATTR_CURRENT, tctx,
>> + &size, 0));
>> + ASSERT_EQ(-1, syscall(__NR_lsm_set_self_attr,
>> + LSM_ATTR_CURRENT | LSM_ATTR_PREV, tctx, size, 0));
>> +
>> + free(ctx);
>> +}
>> +
>> +TEST_HARNESS_MAIN
>> --
>> 2.39.2
>>