Re: [PATCH -next 3/9] lockdown: bound the status string with scnprintf

Re: [PATCH -next 3/9] lockdown: bound the status string with scnprintf

[sashiko-bot]

> Use scnprintf() instead of sprintf() when composing the lockdown status
> string in lockdown_read().  The current set of levels fits well within the
> 80 byte buffer, but sprintf() provides no protection if more or longer
> labels are added later.  scnprintf() makes the buffer bounds explicit.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=3

Re: [PATCH -next 1/9] lockdown: Add break in lockdown_write

[sashiko-bot]

> After the label is matched successful, any other levels judgements
> are meaningless. Therefore, add break to return early
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=1

Re: [PATCH -next 2/9] lockdown: remove useless decrement operation

[sashiko-bot]

> From: Kalevi Kolttonen <kalevi@kolttonen.fi>
> 
> Signed-off-by: Kalevi Kolttonen <kalevi@kolttonen.fi>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=2

Re: [PATCH -next 4/9] lockdown: select SECURITYFS

[sashiko-bot]

> The lockdown LSM exposes /sys/kernel/security/lockdown, but it does not
> select SECURITYFS.  If SECURITYFS is disabled, securityfs_create_file()
> returns -ENODEV, lockdown_secfs_init() fails and the LSM framework logs:
> 
>   failed LSM lockdown core initcall with errno -19
> 
> while the sysfs interface advertised by the Kconfig help text is missing.
> 
> Select SECURITYFS like the other LSMs that create securityfs entries
> (AppArmor, SafeSetID, TOMOYO and IMA) already do.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=4

Re: [PATCH -next 6/9] lockdown: drop an unused include and simplify secfs init

[sashiko-bot]

> linux/export.h is not needed since the file has no EXPORT_SYMBOL() usage.
> Also return the result of securityfs_create_file() directly instead of
> storing it in a temporary dentry.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=6

Re: [PATCH -next 5/9] lsm: remove unused lockdown audit data

[sashiko-bot]

> Commit f5d0e5e9d72d ("selinux: remove the SELinux lockdown
> implementation") removed the only producer of LSM_AUDIT_DATA_LOCKDOWN.
> The type, the u.reason union member and the audit formatting case are now
> dead, so drop them.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=5

Re: [PATCH -next 7/9] lockdown: move lockdown_reasons next to its only user

[sashiko-bot]

> lockdown_reasons[] is only used by the lockdown LSM after the audit
> data was removed from lsm_audit. Move it into
> security/lockdown/lockdown.c, make it static, drop the now stale extern
> declaration, and point the enum comment at the new location.
> 
> No functional change.
> 
> Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=7

Re: [PATCH -next 8/9] Documentation/ABI: document the lockdown securityfs file

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Documentation/ABI: securityfs-lockdown omits CONFIG_LSM activation path
--

commit a74113d4e117aa2187671cc1af3e2f7a75da9ede
Author: Cai Xinchen <caixinchen1@huawei.com>

Documentation/ABI: document the lockdown securityfs file

This patch adds ABI documentation for /sys/kernel/security/lockdown and
updates the LOCKDOWN SECURITY MODULE entry in MAINTAINERS.

> diff --git a/Documentation/ABI/testing/securityfs-lockdown b/Documentation/ABI/testing/securityfs-lockdown
> new file mode 100644
> index 0000000000000..490dc607492ce
> --- /dev/null
> +++ b/Documentation/ABI/testing/securityfs-lockdown
> @@ -0,0 +1,41 @@
> +What:		/sys/kernel/security/lockdown
> +Date:		August 2019
> +KernelVersion:	5.4
> +Contact:	linux-security-module@vger.kernel.org
> +Description:
> +		The lockdown securityfs file reports and controls the
> +		lockdown state of the running kernel.  It is provided by
> +		the lockdown LSM and is located at
> +		/sys/kernel/security/lockdown.
> +
> +		The file is present only when CONFIG_SECURITY_LOCKDOWN_LSM is
> +		enabled and the lockdown LSM is active: either
> +		CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is set, or the LSM is
> +		listed in the "lsm=" kernel parameter.  Otherwise the node
> +		is absent, and userspace must handle its absence.

[Severity: Low]
Can the lockdown LSM also be activated at build time via CONFIG_LSM when
neither CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is set nor the lsm= kernel
parameter is passed?

In security/lsm_init.c, security_init() falls back to lsm_order_builtin
(initialized from CONFIG_LSM) when lsm= is not specified on the command
line:

security/lsm_init.c:security_init() {
	...
	if (lsm_order_cmdline) {
		lsm_order_legacy = NULL;
		lsm_order_parse(lsm_order_cmdline, "cmdline");
	} else
		lsm_order_parse(lsm_order_builtin, "builtin");
	...
}

And security/Kconfig includes lockdown in the default CONFIG_LSM list,
which causes security_initcall_core() -> lockdown_secfs_init() to create
/sys/kernel/security/lockdown.

Should this paragraph also mention CONFIG_LSM as one of the ways the
lockdown LSM can be enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=8

Re: [PATCH -next 9/9] selftests/lsm: add lockdown securityfs test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/lsm: duplicated setup/teardown and double open in lockdown_test.c
--

commit 5cebacd6e5feb5064c2370aa4d99aad209688038
Author: Cai Xinchen <caixinchen1@huawei.com>

selftests/lsm: add lockdown securityfs test

This commit adds a kselftest for the lockdown securityfs interface to
check the read format, reject unknown levels, and verify that an already
locked down kernel rejects the current or a lower level. It also moves
the file reading logic in common.c into a shared read_file() helper.

> diff --git a/tools/testing/selftests/lsm/lockdown_test.c b/tools/testing/selftests/lsm/lockdown_test.c
> new file mode 100644
> index 0000000000000..35109833da373
> --- /dev/null
> +++ b/tools/testing/selftests/lsm/lockdown_test.c
> @@ -0,0 +1,176 @@

[ ... ]

> +static int write_status(const char *value)
> +{
> +	ssize_t len;
> +	int fd, err;
> +
> +	fd = open(LOCKDOWN_PATH, O_WRONLY);
> +	if (fd < 0)
> +		return -1;
> +
> +	len = write(fd, value, strlen(value));
> +	err = errno;
> +	close(fd);
> +	if (len < 0) {
> +		errno = err;
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int status_writable(void)
> +{
> +	int fd;
> +
> +	fd = open(LOCKDOWN_PATH, O_WRONLY);
> +	if (fd < 0)
> +		return -1;
> +
> +	close(fd);
> +	return 0;
> +}

[ ... ]

> +TEST(lockdown_invalid_write)
> +{
> +	char before[128], after[128];
> +
> +	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
> +		if (errno == ENOENT)
> +			SKIP(return, "%s is not present", LOCKDOWN_PATH);
> +		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
> +		     strerror(errno));
> +	}
> +	if (geteuid() != 0)
> +		SKIP(return, "requires root");
> +	if (status_writable())
> +		SKIP(return, "cannot open %s for writing: %s",
> +		     LOCKDOWN_PATH, strerror(errno));
> +
> +	errno = 0;
> +	ASSERT_EQ(-1, write_status("invalid-level"));
> +	ASSERT_EQ(EINVAL, errno);
> +
> +	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
> +	ASSERT_STREQ(before, after);
> +}
> +
> +TEST(lockdown_no_downgrade)
> +{
> +	char before[128], after[128], level[32];
> +	const char *lower;
> +
> +	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
> +		if (errno == ENOENT)
> +			SKIP(return, "%s is not present", LOCKDOWN_PATH);
> +		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
> +		     strerror(errno));
> +	}
> +	if (geteuid() != 0)
> +		SKIP(return, "requires root");
> +	if (status_writable())
> +		SKIP(return, "cannot open %s for writing: %s",
> +		     LOCKDOWN_PATH, strerror(errno));
> +	ASSERT_EQ(0, get_active_level(before, level, sizeof(level)));
> +
> +	if (!strcmp(level, "confidentiality"))
> +		lower = "integrity";
> +	else if (!strcmp(level, "integrity"))
> +		lower = "none";
> +	else
> +		SKIP(return, "kernel is not locked down, refusing to escalate");
> +
> +	errno = 0;
> +	ASSERT_EQ(-1, write_status(lower));
> +	ASSERT_EQ(EPERM, errno);
> +
> +	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
> +	ASSERT_STREQ(before, after);
> +}
> +
> +TEST(lockdown_same_level_rejected)
> +{
> +	char before[128], after[128], level[32];
> +
> +	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
> +		if (errno == ENOENT)
> +			SKIP(return, "%s is not present", LOCKDOWN_PATH);
> +		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
> +		     strerror(errno));
> +	}
> +	if (geteuid() != 0)
> +		SKIP(return, "requires root");
> +	if (status_writable())
> +		SKIP(return, "cannot open %s for writing: %s",
> +		     LOCKDOWN_PATH, strerror(errno));
> +	ASSERT_EQ(0, get_active_level(before, level, sizeof(level)));
> +
> +	errno = 0;
> +	ASSERT_EQ(-1, write_status(level));
> +	ASSERT_EQ(EPERM, errno);
> +
> +	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
> +	ASSERT_STREQ(before, after);
> +}

[Severity: Low]
This isn't a bug, but lockdown_invalid_write, lockdown_no_downgrade, and
lockdown_same_level_rejected duplicate the same initial read_file() and
SKIP() checks, root check, status_writable() check, active level parsing,
and trailing read_file() plus ASSERT_STREQ(before, after) check.

Additionally, in each of those write tests, status_writable() opens
LOCKDOWN_PATH with O_WRONLY and immediately closes it, only for
write_status() to open and close LOCKDOWN_PATH with O_WRONLY a second
time.

Could this use FIXTURE(), FIXTURE_SETUP(), FIXTURE_TEARDOWN(), and
TEST_F() from kselftest_harness.h (or a shared setup/write helper) to
avoid the duplicated prerequisite/post-condition blocks and redundant
file opens?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103082421.3033195-1-caixinchen1@huawei.com?part=9

[PATCH -next 0/9] lockdown: fix the securityfs interface and add tests

[Cai Xinchen]

The lockdown LSM exposes /sys/kernel/security/lockdown, but the file is
not documented and has no selftest.  While looking at the interface we
also found a configuration bug and several accumulated cleanups.

The series is organized as follows:

  - patches 1-2 are small cleanups in lockdown_write(): stop scanning
    after the matching level and drop an update to a length variable
    that is not used afterwards.
  - patch 3 replaces sprintf() with scnprintf() so that the status
    buffer is bounded explicitly if more levels are added later.
  - patch 4 makes the LSM select SECURITYFS.  Without it, a kernel
    built with CONFIG_SECURITYFS=n logs "failed LSM lockdown core
    initcall with errno -19" and does not create the file advertised
    by the Kconfig help text.
  - patch 5 removes LSM_AUDIT_DATA_LOCKDOWN and its formatting code,
    which became dead when the SELinux lockdown implementation was
    removed by commit f5d0e5e9d72d.
  - patches 6-7 drop an unused include, simplify the securityfs
    registration, and move lockdown_reasons[] next to its only user.
  - patch 8 documents the ABI and adds the new file to the LOCKDOWN
    SECURITY MODULE entry in MAINTAINERS.
  - patch 9 adds a kselftest covering the read format, invalid levels,
    and the rejection of the current or a lower level.

The only functional change is patch 4; the rest are cleanups,
documentation, or test coverage.  The selftest never escalates
lockdown on the machine where it runs.

Cai Xinchen (8):
  lockdown: Add break in lockdown_write
  lockdown: bound the status string with scnprintf
  lockdown: select SECURITYFS
  lsm: remove unused lockdown audit data
  lockdown: drop an unused include and simplify secfs init
  lockdown: move lockdown_reasons next to its only user
  Documentation/ABI: document the lockdown securityfs file
  selftests/lsm: add lockdown securityfs test

Kalevi Kolttonen (1):
  lockdown: remove useless decrement operation

 Documentation/ABI/testing/securityfs-lockdown |  41 ++++
 MAINTAINERS                                   |   1 +
 include/linux/lsm_audit.h                     |   2 -
 include/linux/security.h                      |   2 -
 security/lockdown/Kconfig                     |   1 +
 security/lockdown/lockdown.c                  |  63 +++++--
 security/lsm_audit.c                          |   4 -
 security/security.c                           |  40 ----
 tools/testing/selftests/lsm/Makefile          |   3 +-
 tools/testing/selftests/lsm/common.c          |  35 +++-
 tools/testing/selftests/lsm/common.h          |   2 +
 tools/testing/selftests/lsm/lockdown_test.c   | 176 ++++++++++++++++++
 12 files changed, 300 insertions(+), 70 deletions(-)
 create mode 100644 Documentation/ABI/testing/securityfs-lockdown
 create mode 100644 tools/testing/selftests/lsm/lockdown_test.c

-- 
2.18.0.huawei.25

[PATCH -next 1/9] lockdown: Add break in lockdown_write

[Cai Xinchen]

After the label is matched successful, any other levels judgements
are meaningless. Therefore, add break to return early

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lockdown/lockdown.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 8d46886d2cca..263dcc80d839 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -139,8 +139,10 @@ static ssize_t lockdown_write(struct file *file, const char __user *buf,
 		enum lockdown_reason level = lockdown_levels[i];
 		const char *label = lockdown_reasons[level];
 
-		if (label && !strcmp(state, label))
+		if (label && !strcmp(state, label)) {
 			err = lock_kernel_down("securityfs", level);
+			break;
+		}
 	}
 
 	kfree(state);
-- 
2.18.0.huawei.25

[PATCH -next 2/9] lockdown: remove useless decrement operation

[Cai Xinchen]

From: Kalevi Kolttonen <kalevi@kolttonen.fi>

Signed-off-by: Kalevi Kolttonen <kalevi@kolttonen.fi>
---
 security/lockdown/lockdown.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 263dcc80d839..8648508d49b2 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -130,10 +130,8 @@ static ssize_t lockdown_write(struct file *file, const char __user *buf,
 		return PTR_ERR(state);
 
 	len = strlen(state);
-	if (len && state[len-1] == '\n') {
+	if (len && state[len-1] == '\n')
 		state[len-1] = '\0';
-		len--;
-	}
 
 	for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
 		enum lockdown_reason level = lockdown_levels[i];
-- 
2.18.0.huawei.25

[PATCH -next 3/9] lockdown: bound the status string with scnprintf

[Cai Xinchen]

Use scnprintf() instead of sprintf() when composing the lockdown status
string in lockdown_read().  The current set of levels fits well within the
80 byte buffer, but sprintf() provides no protection if more or longer
labels are added later.  scnprintf() makes the buffer bounds explicit.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lockdown/lockdown.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 8648508d49b2..b6f99b9179b9 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -106,9 +106,13 @@ static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
 			const char *label = lockdown_reasons[level];
 
 			if (kernel_locked_down == level)
-				offset += sprintf(temp+offset, "[%s] ", label);
+				offset += scnprintf(temp + offset,
+						    sizeof(temp) - offset,
+						    "[%s] ", label);
 			else
-				offset += sprintf(temp+offset, "%s ", label);
+				offset += scnprintf(temp + offset,
+						    sizeof(temp) - offset,
+						    "%s ", label);
 		}
 	}
 
-- 
2.18.0.huawei.25

[PATCH -next 4/9] lockdown: select SECURITYFS

[Cai Xinchen]

The lockdown LSM exposes /sys/kernel/security/lockdown, but it does not
select SECURITYFS.  If SECURITYFS is disabled, securityfs_create_file()
returns -ENODEV, lockdown_secfs_init() fails and the LSM framework logs:

  failed LSM lockdown core initcall with errno -19

while the sysfs interface advertised by the Kconfig help text is missing.

Select SECURITYFS like the other LSMs that create securityfs entries
(AppArmor, SafeSetID, TOMOYO and IMA) already do.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lockdown/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index e84ddf484010..82b69d8253bd 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -2,6 +2,7 @@ config SECURITY_LOCKDOWN_LSM
 	bool "Basic module for enforcing kernel lockdown"
 	depends on SECURITY
 	select MODULE_SIG if MODULES
+	select SECURITYFS
 	help
 	  Build support for an LSM that enforces a coarse kernel lockdown
 	  behaviour.
-- 
2.18.0.huawei.25

[PATCH -next 5/9] lsm: remove unused lockdown audit data

[Cai Xinchen]

Commit f5d0e5e9d72d ("selinux: remove the SELinux lockdown
implementation") removed the only producer of LSM_AUDIT_DATA_LOCKDOWN.
The type, the u.reason union member and the audit formatting case are now
dead, so drop them.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 include/linux/lsm_audit.h | 2 --
 security/lsm_audit.c      | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 5cf0b4795065..51b498c0ec74 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -74,7 +74,6 @@ struct common_audit_data {
 #define LSM_AUDIT_DATA_FILE	12
 #define LSM_AUDIT_DATA_IBPKEY	13
 #define LSM_AUDIT_DATA_IBENDPORT 14
-#define LSM_AUDIT_DATA_LOCKDOWN 15
 #define LSM_AUDIT_DATA_NOTIFICATION 16
 #define LSM_AUDIT_DATA_ANONINODE	17
 #define LSM_AUDIT_DATA_NLMSGTYPE	18
@@ -98,7 +97,6 @@ struct common_audit_data {
 		const struct file *file;
 		struct lsm_ibpkey_audit *ibpkey;
 		struct lsm_ibendport_audit *ibendport;
-		int reason;
 		const char *anonclass;
 		u16 nlmsg_type;
 		struct {
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 955b2e7b2c8e..4b653edcb77e 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -393,10 +393,6 @@ void audit_log_lsm_data(struct audit_buffer *ab,
 				 a->u.ibendport->dev_name,
 				 a->u.ibendport->port);
 		break;
-	case LSM_AUDIT_DATA_LOCKDOWN:
-		audit_log_format(ab, " lockdown_reason=\"%s\"",
-				 lockdown_reasons[a->u.reason]);
-		break;
 	case LSM_AUDIT_DATA_ANONINODE:
 		audit_log_format(ab, " anonclass=%s", a->u.anonclass);
 		break;
-- 
2.18.0.huawei.25

[PATCH -next 6/9] lockdown: drop an unused include and simplify secfs init

[Cai Xinchen]

linux/export.h is not needed since the file has no EXPORT_SYMBOL() usage.
Also return the result of securityfs_create_file() directly instead of
storing it in a temporary dentry.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 security/lockdown/lockdown.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index b6f99b9179b9..08a30cd471e0 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -11,7 +11,6 @@
  */
 
 #include <linux/security.h>
-#include <linux/export.h>
 #include <linux/lsm_hooks.h>
 #include <uapi/linux/lsm.h>
 
@@ -158,11 +157,8 @@ static const struct file_operations lockdown_ops = {
 
 static int __init lockdown_secfs_init(void)
 {
-	struct dentry *dentry;
-
-	dentry = securityfs_create_file("lockdown", 0644, NULL, NULL,
-					&lockdown_ops);
-	return PTR_ERR_OR_ZERO(dentry);
+	return PTR_ERR_OR_ZERO(securityfs_create_file("lockdown", 0644, NULL,
+						      NULL, &lockdown_ops));
 }
 
 #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
-- 
2.18.0.huawei.25

[PATCH -next 7/9] lockdown: move lockdown_reasons next to its only user

[Cai Xinchen]

lockdown_reasons[] is only used by the lockdown LSM after the audit
data was removed from lsm_audit. Move it into
security/lockdown/lockdown.c, make it static, drop the now stale extern
declaration, and point the enum comment at the new location.

No functional change.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 include/linux/security.h     |  2 --
 security/lockdown/lockdown.c | 39 +++++++++++++++++++++++++++++++++++
 security/security.c          | 40 ------------------------------------
 3 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 40831fab5470..9d3a1eaa90ff 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -170,8 +170,6 @@ struct lsm_prop {
 	struct lsm_prop_bpf bpf;
 };
 
-extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
-
 /* These functions are in security/commoncap.c */
 extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
 		       int cap, unsigned int opts);
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 08a30cd471e0..8c6344399967 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -14,6 +14,45 @@
 #include <linux/lsm_hooks.h>
 #include <uapi/linux/lsm.h>
 
+/*
+ * These are descriptions of the reasons that can be passed to the
+ * security_locked_down() LSM hook, used for the denial message and the
+ * securityfs status file.
+ */
+static const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
+	[LOCKDOWN_NONE] = "none",
+	[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
+	[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
+	[LOCKDOWN_EFI_TEST] = "/dev/efi_test access",
+	[LOCKDOWN_KEXEC] = "kexec of unsigned images",
+	[LOCKDOWN_HIBERNATION] = "hibernation",
+	[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
+	[LOCKDOWN_IOPORT] = "raw io port access",
+	[LOCKDOWN_MSR] = "raw MSR access",
+	[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
+	[LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
+	[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
+	[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
+	[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
+	[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
+	[LOCKDOWN_DEBUGFS] = "debugfs access",
+	[LOCKDOWN_XMON_WR] = "xmon write access",
+	[LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
+	[LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
+	[LOCKDOWN_RTAS_ERROR_INJECTION] = "RTAS error injection",
+	[LOCKDOWN_XEN_USER_ACTIONS] = "Xen guest user action",
+	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
+	[LOCKDOWN_KCORE] = "/proc/kcore access",
+	[LOCKDOWN_KPROBES] = "use of kprobes",
+	[LOCKDOWN_BPF_READ_KERNEL] = "use of bpf to read kernel RAM",
+	[LOCKDOWN_DBG_READ_KERNEL] = "use of kgdb/kdb to read kernel RAM",
+	[LOCKDOWN_PERF] = "unsafe use of perf",
+	[LOCKDOWN_TRACEFS] = "use of tracefs",
+	[LOCKDOWN_XMON_RW] = "xmon read and write access",
+	[LOCKDOWN_XFRM_SECRET] = "xfrm SA secret",
+	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
+};
+
 static enum lockdown_reason kernel_locked_down;
 
 static const enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
diff --git a/security/security.c b/security/security.c
index f6583d507395..71cc823881b8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -35,46 +35,6 @@
 
 #include "lsm.h"
 
-/*
- * These are descriptions of the reasons that can be passed to the
- * security_locked_down() LSM hook. Placing this array here allows
- * all security modules to use the same descriptions for auditing
- * purposes.
- */
-const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
-	[LOCKDOWN_NONE] = "none",
-	[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
-	[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
-	[LOCKDOWN_EFI_TEST] = "/dev/efi_test access",
-	[LOCKDOWN_KEXEC] = "kexec of unsigned images",
-	[LOCKDOWN_HIBERNATION] = "hibernation",
-	[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
-	[LOCKDOWN_IOPORT] = "raw io port access",
-	[LOCKDOWN_MSR] = "raw MSR access",
-	[LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
-	[LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
-	[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
-	[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
-	[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
-	[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
-	[LOCKDOWN_DEBUGFS] = "debugfs access",
-	[LOCKDOWN_XMON_WR] = "xmon write access",
-	[LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
-	[LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
-	[LOCKDOWN_RTAS_ERROR_INJECTION] = "RTAS error injection",
-	[LOCKDOWN_XEN_USER_ACTIONS] = "Xen guest user action",
-	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
-	[LOCKDOWN_KCORE] = "/proc/kcore access",
-	[LOCKDOWN_KPROBES] = "use of kprobes",
-	[LOCKDOWN_BPF_READ_KERNEL] = "use of bpf to read kernel RAM",
-	[LOCKDOWN_DBG_READ_KERNEL] = "use of kgdb/kdb to read kernel RAM",
-	[LOCKDOWN_PERF] = "unsafe use of perf",
-	[LOCKDOWN_TRACEFS] = "use of tracefs",
-	[LOCKDOWN_XMON_RW] = "xmon read and write access",
-	[LOCKDOWN_XFRM_SECRET] = "xfrm SA secret",
-	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
-};
-
 bool lsm_debug __ro_after_init;
 
 unsigned int lsm_active_cnt __ro_after_init;
-- 
2.18.0.huawei.25

[PATCH -next 8/9] Documentation/ABI: document the lockdown securityfs file

[Cai Xinchen]

Describe the /sys/kernel/security/lockdown interface: the levels it
reports, how the active level is marked, when the file is present, and
the write semantics (escalation only, EPERM for the current or a lower
level, EINVAL for an unknown level).

Also add the new ABI file to the LOCKDOWN SECURITY MODULE entry in
MAINTAINERS.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 Documentation/ABI/testing/securityfs-lockdown | 41 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 42 insertions(+)
 create mode 100644 Documentation/ABI/testing/securityfs-lockdown

diff --git a/Documentation/ABI/testing/securityfs-lockdown b/Documentation/ABI/testing/securityfs-lockdown
new file mode 100644
index 000000000000..490dc607492c
--- /dev/null
+++ b/Documentation/ABI/testing/securityfs-lockdown
@@ -0,0 +1,41 @@
+What:		/sys/kernel/security/lockdown
+Date:		August 2019
+KernelVersion:	5.4
+Contact:	linux-security-module@vger.kernel.org
+Description:
+		The lockdown securityfs file reports and controls the
+		lockdown state of the running kernel.  It is provided by
+		the lockdown LSM and is located at
+		/sys/kernel/security/lockdown.
+
+		The file is present only when CONFIG_SECURITY_LOCKDOWN_LSM is
+		enabled and the lockdown LSM is active: either
+		CONFIG_SECURITY_LOCKDOWN_LSM_EARLY is set, or the LSM is
+		listed in the "lsm=" kernel parameter.  Otherwise the node
+		is absent, and userspace must handle its absence.
+
+		Reading the file returns the supported lockdown levels,
+		with the currently active level enclosed in square
+		brackets.  For example, when the kernel is not locked
+		down::
+
+		  [none] integrity confidentiality
+
+		and when the kernel is in integrity mode::
+
+		  none [integrity] confidentiality
+
+		The file may be written with the name of a level to lock
+		the kernel down further.  Lockdown can only be escalated,
+		never relaxed, so writing the currently active level or a
+		lower level fails with EPERM.  Writing an unrecognized
+		level fails with EINVAL.  Successful writes return the
+		number of bytes written.
+
+		The kernel can also be locked down at boot time with the
+		"lockdown=" kernel parameter, or unconditionally at build
+		time via CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY or
+		CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY.
+
+Users:		fwupd and other userspace tools that query or manage
+		kernel lockdown state
diff --git a/MAINTAINERS b/MAINTAINERS
index aaf2825ac945..38ff3797f2cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15621,6 +15621,7 @@ M:	Xiu Jianfeng <xiujianfeng@huawei.com>
 L:	linux-security-module@vger.kernel.org
 S:	Maintained
 T:	git https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
+F:	Documentation/ABI/testing/securityfs-lockdown
 F:	security/lockdown/
 
 LOCKING PRIMITIVES
-- 
2.18.0.huawei.25

[PATCH -next 9/9] selftests/lsm: add lockdown securityfs test

[Cai Xinchen]

Add a kselftest for the lockdown securityfs interface.  It checks the
read format, rejects unknown levels, and verifies that an already locked
down kernel neither accepts the current level nor a lower one.  The test
skips itself when lockdown is not enabled, when not run as root, or when
the file cannot be opened for writing, and it never escalates lockdown
on the test machine.

Use the shared file read helper from common.c instead of duplicating it.

Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
 tools/testing/selftests/lsm/Makefile        |   3 +-
 tools/testing/selftests/lsm/common.c        |  35 +++-
 tools/testing/selftests/lsm/common.h        |   2 +
 tools/testing/selftests/lsm/lockdown_test.c | 176 ++++++++++++++++++++
 4 files changed, 206 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/lsm/lockdown_test.c

diff --git a/tools/testing/selftests/lsm/Makefile b/tools/testing/selftests/lsm/Makefile
index 3f80c0bc093d..737e8686bb7d 100644
--- a/tools/testing/selftests/lsm/Makefile
+++ b/tools/testing/selftests/lsm/Makefile
@@ -6,12 +6,13 @@ CFLAGS += -Wall -O2 $(KHDR_INCLUDES)
 LOCAL_HDRS += common.h
 
 TEST_GEN_PROGS := lsm_get_self_attr_test lsm_list_modules_test \
-		  lsm_set_self_attr_test
+		  lsm_set_self_attr_test lockdown_test
 
 include ../lib.mk
 
 $(OUTPUT)/lsm_get_self_attr_test: lsm_get_self_attr_test.c common.c
 $(OUTPUT)/lsm_set_self_attr_test: lsm_set_self_attr_test.c common.c
 $(OUTPUT)/lsm_list_modules_test: lsm_list_modules_test.c common.c
+$(OUTPUT)/lockdown_test: lockdown_test.c common.c
 
 EXTRA_CLEAN = $(OUTPUT)/common.o
diff --git a/tools/testing/selftests/lsm/common.c b/tools/testing/selftests/lsm/common.c
index 927dce4f04cb..e7d5cdb10137 100644
--- a/tools/testing/selftests/lsm/common.c
+++ b/tools/testing/selftests/lsm/common.c
@@ -7,6 +7,7 @@
 
 #define _GNU_SOURCE
 #include <linux/lsm.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <string.h>
 #include <stdio.h>
@@ -50,23 +51,39 @@ int read_proc_attr(const char *attr, char *value, size_t size)
 	return 0;
 }
 
-int read_sysfs_lsms(char *lsms, size_t size)
+int read_file(const char *path, char *buf, size_t size)
 {
-	FILE *fp;
-	size_t red;
+	ssize_t len;
+	int fd;
 
-	fp = fopen("/sys/kernel/security/lsm", "r");
-	if (fp == NULL)
+	if (!size) {
+		errno = EINVAL;
 		return -1;
-	red = fread(lsms, 1, size, fp);
-	fclose(fp);
+	}
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	len = read(fd, buf, size - 1);
+	if (len < 0) {
+		int err = errno;
 
-	if (red <= 0 || red == size)
+		close(fd);
+		errno = err;
 		return -1;
-	lsms[red] = '\0';
+	}
+
+	close(fd);
+	buf[len] = '\0';
 	return 0;
 }
 
+int read_sysfs_lsms(char *lsms, size_t size)
+{
+	return read_file("/sys/kernel/security/lsm", lsms, size);
+}
+
 int attr_lsm_count(void)
 {
 	char *names = calloc(sysconf(_SC_PAGESIZE), 1);
diff --git a/tools/testing/selftests/lsm/common.h b/tools/testing/selftests/lsm/common.h
index 06d12110d241..2f824de2d1ad 100644
--- a/tools/testing/selftests/lsm/common.h
+++ b/tools/testing/selftests/lsm/common.h
@@ -31,3 +31,5 @@ static inline int lsm_list_modules(__u64 *ids, __u32 *size, __u32 flags)
 extern int read_proc_attr(const char *attr, char *value, size_t size);
 extern int read_sysfs_lsms(char *lsms, size_t size);
 int attr_lsm_count(void);
+
+extern int read_file(const char *path, char *buf, size_t size);
diff --git a/tools/testing/selftests/lsm/lockdown_test.c b/tools/testing/selftests/lsm/lockdown_test.c
new file mode 100644
index 000000000000..35109833da37
--- /dev/null
+++ b/tools/testing/selftests/lsm/lockdown_test.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux Security Module infrastructure tests
+ * Tests for the lockdown securityfs interface
+ */
+
+#define _GNU_SOURCE
+#include <linux/lsm.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include "kselftest_harness.h"
+#include "common.h"
+
+#define LOCKDOWN_PATH "/sys/kernel/security/lockdown"
+
+static int write_status(const char *value)
+{
+	ssize_t len;
+	int fd, err;
+
+	fd = open(LOCKDOWN_PATH, O_WRONLY);
+	if (fd < 0)
+		return -1;
+
+	len = write(fd, value, strlen(value));
+	err = errno;
+	close(fd);
+	if (len < 0) {
+		errno = err;
+		return -1;
+	}
+
+	return 0;
+}
+
+static int status_writable(void)
+{
+	int fd;
+
+	fd = open(LOCKDOWN_PATH, O_WRONLY);
+	if (fd < 0)
+		return -1;
+
+	close(fd);
+	return 0;
+}
+
+static int get_active_level(const char *buf, char *level, size_t size)
+{
+	const char *start, *end;
+	size_t len;
+
+	start = strchr(buf, '[');
+	if (!start)
+		return -1;
+	end = strchr(start, ']');
+	if (!end)
+		return -1;
+
+	len = end - start - 1;
+	if (len == 0 || len >= size)
+		return -1;
+
+	memcpy(level, start + 1, len);
+	level[len] = '\0';
+	return 0;
+}
+
+TEST(lockdown_read_format)
+{
+	char buf[128], level[32];
+
+	if (read_file(LOCKDOWN_PATH, buf, sizeof(buf))) {
+		if (errno == ENOENT)
+			SKIP(return, "%s is not present", LOCKDOWN_PATH);
+		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
+		     strerror(errno));
+	}
+
+	ASSERT_EQ(0, get_active_level(buf, level, sizeof(level)));
+	ASSERT_TRUE(!strcmp(level, "none") ||
+		    !strcmp(level, "integrity") ||
+		    !strcmp(level, "confidentiality"));
+
+	ASSERT_NE(NULL, strstr(buf, "none"));
+	ASSERT_NE(NULL, strstr(buf, "integrity"));
+	ASSERT_NE(NULL, strstr(buf, "confidentiality"));
+}
+
+TEST(lockdown_invalid_write)
+{
+	char before[128], after[128];
+
+	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
+		if (errno == ENOENT)
+			SKIP(return, "%s is not present", LOCKDOWN_PATH);
+		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
+		     strerror(errno));
+	}
+	if (geteuid() != 0)
+		SKIP(return, "requires root");
+	if (status_writable())
+		SKIP(return, "cannot open %s for writing: %s",
+		     LOCKDOWN_PATH, strerror(errno));
+
+	errno = 0;
+	ASSERT_EQ(-1, write_status("invalid-level"));
+	ASSERT_EQ(EINVAL, errno);
+
+	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
+	ASSERT_STREQ(before, after);
+}
+
+TEST(lockdown_no_downgrade)
+{
+	char before[128], after[128], level[32];
+	const char *lower;
+
+	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
+		if (errno == ENOENT)
+			SKIP(return, "%s is not present", LOCKDOWN_PATH);
+		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
+		     strerror(errno));
+	}
+	if (geteuid() != 0)
+		SKIP(return, "requires root");
+	if (status_writable())
+		SKIP(return, "cannot open %s for writing: %s",
+		     LOCKDOWN_PATH, strerror(errno));
+	ASSERT_EQ(0, get_active_level(before, level, sizeof(level)));
+
+	if (!strcmp(level, "confidentiality"))
+		lower = "integrity";
+	else if (!strcmp(level, "integrity"))
+		lower = "none";
+	else
+		SKIP(return, "kernel is not locked down, refusing to escalate");
+
+	errno = 0;
+	ASSERT_EQ(-1, write_status(lower));
+	ASSERT_EQ(EPERM, errno);
+
+	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
+	ASSERT_STREQ(before, after);
+}
+
+TEST(lockdown_same_level_rejected)
+{
+	char before[128], after[128], level[32];
+
+	if (read_file(LOCKDOWN_PATH, before, sizeof(before))) {
+		if (errno == ENOENT)
+			SKIP(return, "%s is not present", LOCKDOWN_PATH);
+		SKIP(return, "cannot read %s: %s", LOCKDOWN_PATH,
+		     strerror(errno));
+	}
+	if (geteuid() != 0)
+		SKIP(return, "requires root");
+	if (status_writable())
+		SKIP(return, "cannot open %s for writing: %s",
+		     LOCKDOWN_PATH, strerror(errno));
+	ASSERT_EQ(0, get_active_level(before, level, sizeof(level)));
+
+	errno = 0;
+	ASSERT_EQ(-1, write_status(level));
+	ASSERT_EQ(EPERM, errno);
+
+	ASSERT_EQ(0, read_file(LOCKDOWN_PATH, after, sizeof(after)));
+	ASSERT_STREQ(before, after);
+}
+
+TEST_HARNESS_MAIN
-- 
2.18.0.huawei.25