* Re: [PATCH 5.8 000/232] 5.8.3-rc1 review
From: Naresh Kamboju @ 2020-08-20 15:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: open list, Shuah Khan, patches, lkft-triage, Ben Hutchings,
linux- stable, Andrew Morton, Linus Torvalds, Guenter Roeck,
Herbert Xu, LTP List, linux-security-module, keyrings,
Eric Biggers
In-Reply-To: <20200820091612.692383444@linuxfoundation.org>
On Thu, 20 Aug 2020 at 14:55, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> This is the start of the stable review cycle for the 5.8.3 release.
> There are 232 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Sat, 22 Aug 2020 09:15:09 +0000.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.3-rc1.gz
> or in the git tree and branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.8.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h
> Herbert Xu <herbert@gondor.apana.org.au>
> crypto: af_alg - Fix regression on empty requests
Results from Linaro’s test farm.
Regressions detected.
ltp-crypto-tests:
* af_alg02
ltp-cve-tests:
* cve-2017-17805
af_alg02.c:52: BROK: Timed out while reading from request socket.
We are running the LTP 20200515 tag released test suite.
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
Summary
------------------------------------------------------------------------
kernel: 5.8.3-rc1
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-5.8.y
git commit: 201fff807310ce10485bcff294d47be95f3769eb
git describe: v5.8.2-233-g201fff807310
Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-5.8-oe/build/v5.8.2-233-g201fff807310
Regressions (compared to build v5.8.2)
------------------------------------------------------------------------
x15:
ltp-crypto-tests:
* af_alg02
ltp-cve-tests:
* cve-2017-17805
--
Linaro LKFT
https://lkft.linaro.org
^ permalink raw reply
* [RFC] security: replace indirect calls with static calls
From: Brendan Jackman @ 2020-08-20 16:47 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Paul Renauld, Alexei Starovoitov, Daniel Borkmann, James Morris,
pjt, jannh, peterz, rafael.j.wysocki, keescook, thgarnie, kpsingh,
paul.renauld.epfl, Brendan Jackman
From: Paul Renauld <renauld@google.com>
LSMs have high overhead due to indirect function calls through
retpolines. This RPC proposes to replace these with static calls [1]
instead.
This overhead is especially significant for the "bpf" LSM which supports
the implementation of LSM hooks with eBPF programs (security/bpf)[2]. In
order to facilitate this, the "bpf" LSM provides a default nop callback for
all LSM hooks. When enabled, the "bpf", LSM incurs an unnecessary /
avoidable indirect call to this nop callback.
The performance impact on a simple syscall eventfd_write (which triggers
the file_permission hook) was measured with and without "bpf" LSM
enabled. Activating the LSM resulted in an overhead of 4% [3].
This overhead prevents the adoption of bpf LSM on performance critical
systems, and also, in general, slows down all LSMs.
Currently, the LSM hook callbacks are stored in a linked list and
dispatched as indirect calls. Using static calls can remove this overhead
by replacing all indirect calls with direct calls.
During the discussion of the "bpf" LSM patch-set it was proposed to special
case BPF LSM to avoid the overhead by using static keys. This was however
not accepted and it was decided to [4]:
- Not special-case the "bpf" LSM.
- Implement a general solution benefitting the whole LSM framework.
This is based on the static call branch [5].
For each LSM hook, a table of static calls is defined (referred to as
"static slots", or "slots"). When all the LSMs are initialized and linked
lists are filled, the hook callbacks are copied to the appropriate static
slot. The callbacks are continuously added at the end of the table, and the
index of the first slot that is non empty is stored. Then, when a LSM hook
is called (macro call_[int/void]_hook), the execution jumps to this first
non-empty slot and all of the subsequent static slots are executed.
The static calls are re-initialized every time the linked list is modified,
i.e. after the early LSM init, and the LSM init.
Let's say, there are 5 static slots per LSM hook, and 3 LSMs implement some
hook with the callbacks A, B, C.
Previously, the code for this hook would have looked like this:
ret = DEFAULT_RET;
for each cb in [A, B, C]:
ret = cb(args); <--- costly indirect call here
if ret != 0:
break;
return ret;
Static calls are defined at build time and are initially empty (NOP
instructions). When the LSMs are initialized, the slots are filled as
follows:
slot idx content
|-----------|
0 | |
|-----------|
1 | |
|-----------|
2 | call A | <-- base_slot_idx = 2
|-----------|
3 | call B |
|-----------|
4 | call C |
|-----------|
The generated code will unroll the foreach loop to have a static call for
each possible LSM:
ret = DEFAULT_RET;
switch(base_slot_idx):
case 0:
NOP
if ret != 0:
break;
// fallthrough
case 1:
NOP
if ret != 0:
break;
// fallthrough
case 2:
ret = A(args); <--- direct call, no retpoline
if ret != 0:
break;
// fallthrough
case 3:
ret = B(args); <--- direct call, no retpoline
if ret != 0:
break;
// fallthrough
[...]
default:
break;
return ret;
A similar logic is applied for void hooks.
Why this trick with a switch statement? The table of static call is defined
at compile time. The number of hook callbacks that will be defined is
unknown at that time, and the table cannot be resized at runtime. Static
calls do not define a conditional execution for a non-void function, so the
executed slots must be non-empty. With this use of the table and the
switch, it is possible to jump directly to the first used slot and execute
all of the slots after. This essentially makes the entry point of the table
dynamic. Instead, it would also be possible to start from 0 and break after
the final populated slot, but that would require an additional conditional
after each slot.
This macro is used to generate the code for each static slot, (e.g. each
case statement in the previous example). This will expand into a call to
MACRO for each static slot defined. For example, if with again 5 slots:
SECURITY_FOREACH_STATIC_SLOT(MACRO, x, y) ->
MACRO(0, x, y)
MACRO(1, x, y)
MACRO(2, x, y)
MACRO(3, x, y)
MACRO(4, x, y)
This is used in conjunction with LSM_HOOK definitions in
linux/lsm_hook_defs.h to execute a macro for each static slot of each LSM
hook.
The patches for static calls [6] are not upstreamed yet.
The number of available slots for each LSM hook is currently fixed at
11 (the number of LSMs in the kernel). Ideally, it should automatically
adapt to the number of LSMs compiled into the kernel.
If there’s no practical way to implement such automatic adaptation, an
option instead would be to remove the panic call by falling-back to the old
linked-list mechanism, which is still present anyway (see below).
A few special cases of LSM don't use the macro call_[int/void]_hook but
have their own calling logic. The linked-lists are kept as a possible slow
path fallback for them.
Before:
https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
After:
https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
With this implementation, any overhead of the indirect call in the LSM
framework is completely mitigated (performance results: [7]). This
facilitates the adoption of "bpf" LSM on production machines and also
benefits all other LSMs.
[1]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/
[2]: https://lwn.net/Articles/798157/
[3] measurements: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
protocol: https://gist.github.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5#file-measurement-protocol-md
[4]: https://lwn.net/Articles/813261/
[5]: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/static_call
[6]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/#t
[7]: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morris <jmorris@namei.org>
Cc: pjt@google.com
Cc: jannh@google.com
Cc: peterz@infradead.org
Cc: rafael.j.wysocki@intel.com
Cc: keescook@chromium.org
Cc: thgarnie@chromium.org
Cc: kpsingh@google.com
Cc: paul.renauld.epfl@gmail.com
Signed-off-by: Paul Renauld <renauld@google.com>
Signed-off-by: KP Singh <kpsingh@google.com>
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
include/linux/lsm_hooks.h | 1 +
include/linux/lsm_static_call.h | 134 ++++++++++++++++++++
security/security.c | 217 ++++++++++++++++++++++++++++----
3 files changed, 331 insertions(+), 21 deletions(-)
create mode 100644 include/linux/lsm_static_call.h
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 95b7c1d32062..d11e116b588e 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1524,6 +1524,7 @@ union security_list_options {
#define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
#include "lsm_hook_defs.h"
#undef LSM_HOOK
+ void *generic_func;
};
struct security_hook_heads {
diff --git a/include/linux/lsm_static_call.h b/include/linux/lsm_static_call.h
new file mode 100644
index 000000000000..f5f5698292e0
--- /dev/null
+++ b/include/linux/lsm_static_call.h
@@ -0,0 +1,134 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2020 Google LLC.
+ */
+
+#ifndef __LINUX_LSM_STATIC_CALL_H
+#define __LINUX_LSM_STATIC_CALL_H
+
+/*
+ * Static slots are used in security/security.c to avoid costly
+ * indirect calls by replacing them with static calls.
+ * The number of static calls for each LSM hook is fixed.
+ */
+#define SECURITY_STATIC_SLOT_COUNT 11
+
+/*
+ * Identifier for the LSM static slots.
+ * HOOK is an LSM hook as defined in linux/lsm_hookdefs.h
+ * IDX is the index of the slot. 0 <= NUM < SECURITY_STATIC_SLOT_COUNT
+ */
+#define STATIC_SLOT(HOOK, IDX) security_static_slot_##HOOK##_##IDX
+
+/*
+ * Call the macro M for each LSM hook slot.
+ * M should take as first argument the index and then
+ * the same __VA_ARGS__
+ * Essentially, this will expand to:
+ * M(0, ...)
+ * M(1, ...)
+ * M(2, ...)
+ * ...
+ * Note that no trailing semicolon is placed so M should be defined
+ * accordingly.
+ * This adapts to a change to SECURITY_STATIC_SLOT_COUNT.
+ */
+#define SECURITY_FOREACH_STATIC_SLOT(M, ...) \
+ UNROLL_MACRO_LOOP(SECURITY_STATIC_SLOT_COUNT, M, __VA_ARGS__)
+
+/*
+ * Intermediate macros to expand SECURITY_STATIC_SLOT_COUNT
+ */
+#define UNROLL_MACRO_LOOP(N, MACRO, ...) \
+ _UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
+
+#define _UNROLL_MACRO_LOOP(N, MACRO, ...) \
+ __UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP(N, MACRO, ...) \
+ __UNROLL_MACRO_LOOP_##N(MACRO, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_0(MACRO, ...)
+
+#define __UNROLL_MACRO_LOOP_1(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_0(MACRO, __VA_ARGS__) \
+ MACRO(0, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_2(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_1(MACRO, __VA_ARGS__) \
+ MACRO(1, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_3(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_2(MACRO, __VA_ARGS__) \
+ MACRO(2, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_4(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_3(MACRO, __VA_ARGS__) \
+ MACRO(3, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_5(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_4(MACRO, __VA_ARGS__) \
+ MACRO(4, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_6(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_5(MACRO, __VA_ARGS__) \
+ MACRO(5, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_7(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_6(MACRO, __VA_ARGS__) \
+ MACRO(6, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_8(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_7(MACRO, __VA_ARGS__) \
+ MACRO(7, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_9(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_8(MACRO, __VA_ARGS__) \
+ MACRO(8, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_10(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_9(MACRO, __VA_ARGS__) \
+ MACRO(9, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_11(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_10(MACRO, __VA_ARGS__) \
+ MACRO(10, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_12(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_11(MACRO, __VA_ARGS__) \
+ MACRO(11, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_13(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_12(MACRO, __VA_ARGS__) \
+ MACRO(12, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_14(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_13(MACRO, __VA_ARGS__) \
+ MACRO(13, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_15(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_14(MACRO, __VA_ARGS__) \
+ MACRO(14, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_16(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_15(MACRO, __VA_ARGS__) \
+ MACRO(15, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_17(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_16(MACRO, __VA_ARGS__) \
+ MACRO(16, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_18(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_17(MACRO, __VA_ARGS__) \
+ MACRO(17, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_19(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_18(MACRO, __VA_ARGS__) \
+ MACRO(18, __VA_ARGS__)
+
+#define __UNROLL_MACRO_LOOP_20(MACRO, ...) \
+ __UNROLL_MACRO_LOOP_19(MACRO, __VA_ARGS__) \
+ MACRO(19, __VA_ARGS__)
+
+#endif /* __LINUX_LSM_STATIC_CALL_H */
diff --git a/security/security.c b/security/security.c
index 70a7ad357bc6..15026bc716f2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -28,6 +28,8 @@
#include <linux/string.h>
#include <linux/msg.h>
#include <net/flow.h>
+#include <linux/static_call.h>
+#include <linux/lsm_static_call.h>
#define MAX_LSM_EVM_XATTR 2
@@ -86,6 +88,128 @@ static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
static __initdata struct lsm_info **ordered_lsms;
static __initdata struct lsm_info *exclusive;
+/*
+ * Necessary information about a static
+ * slot to call __static_call_update
+ */
+struct static_slot {
+ /* static call key as defined by STATIC_CALL_KEY */
+ struct static_call_key *key;
+ /* static call trampoline as defined by STATIC_CALL_TRAMP */
+ void *trampoline;
+};
+
+/*
+ * Table of the static calls for each LSM hook.
+ * Once the LSMs are initialized, their callbacks will be copied to these
+ * tables such that the slots are filled backwards (from last to first).
+ * This way, we can jump directly to the first used slot, and execute
+ * all of them after. This essentially makes the entry point point
+ * dynamic to adapt the number of slot to the number of callbacks.
+ */
+struct static_slot_list {
+ #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ struct static_slot NAME[SECURITY_STATIC_SLOT_COUNT];
+ #include <linux/lsm_hook_defs.h>
+ #undef LSM_HOOK
+} __randomize_layout;
+
+/*
+ * Index of the first used static call for each LSM hook
+ * in the corresponding static_slot_list table.
+ * All slots with greater indices are used.
+ * If no slot is used, the default value is INT_MAX.
+ */
+struct base_slot_idx {
+ #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ int NAME;
+ #include <linux/lsm_hook_defs.h>
+ #undef LSM_HOOK
+} __randomize_layout;
+
+/*
+ * Create the static slots for each LSM hook, initially empty.
+ * This will expand to:
+ *
+ * [...]
+ *
+ * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_0,
+ * *((int(*)(struct file *file, int mask)))NULL);
+ * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_1, ...);
+ *
+ * [...]
+ */
+#define CREATE_STATIC_SLOT(NUM, NAME, RET, ...) \
+ DEFINE_STATIC_CALL_NULL(STATIC_SLOT(NAME, NUM), \
+ *((RET(*)(__VA_ARGS__))NULL));
+
+#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ SECURITY_FOREACH_STATIC_SLOT(CREATE_STATIC_SLOT, NAME, RET, __VA_ARGS__)
+#include <linux/lsm_hook_defs.h>
+#undef LSM_HOOK
+#undef CREATE_STATIC_SLOT
+
+/*
+ * Initialise a table of static slots for each LSM hook.
+ * When defined with DEFINE_STATIC_CALL_NULL as above, a static call is
+ * a key and a trampoline. Both are needed to use __static_call_update.
+ * This will expand to:
+ * struct static_slot_list static_slots = {
+ * [...]
+ * .file_permission = {
+ * (struct static_slot) {
+ * .key = &STATIC_CALL_KEY(
+ * security_static_slot_file_permission_0),
+ * .trampoline = &STATIC_CALL_TRAMP(
+ * security_static_slot_file_permission_0)
+ * },
+ * (struct static_slot) {
+ * .key = &STATIC_CALL_KEY(
+ * security_static_slot_file_permission_1),
+ * .trampoline = &STATIC_CALL_TRAMP(
+ * security_static_slot_file_permission_1)
+ * },
+ * [...]
+ * },
+ * .file_alloc_security = {
+ * [...]
+ * },
+ * [...]
+ * }
+ */
+static struct static_slot_list static_slots __initdata = {
+#define DEFINE_SLOT(NUM, NAME) \
+ (struct static_slot) { \
+ .key = &STATIC_CALL_KEY(STATIC_SLOT(NAME, NUM)), \
+ .trampoline = &STATIC_CALL_TRAMP(STATIC_SLOT(NAME, NUM))\
+ },
+#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ .NAME = { \
+ SECURITY_FOREACH_STATIC_SLOT(DEFINE_SLOT, NAME) \
+ },
+#include <linux/lsm_hook_defs.h>
+#undef LSM_HOOK
+#undef DEFINE_SLOT
+};
+
+/*
+ * The base slot index for each is initially INT_MAX, which means
+ * that no slot is used yet.
+ * When expanded, this results in:
+ * struct base_slot_idx base_slot_idx = {
+ * [...]
+ * .file_permission = INT_MAX,
+ * .file_alloc_security = INT_MAX,
+ * [...]
+ * }
+ */
+static struct base_slot_idx base_slot_idx __lsm_ro_after_init = {
+#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ .NAME = INT_MAX,
+#include <linux/lsm_hook_defs.h>
+#undef LSM_HOOK
+};
+
static __initdata bool debug;
#define init_debug(...) \
do { \
@@ -307,6 +431,46 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
kfree(sep);
}
+static void __init lsm_init_hook_static_slot(struct static_slot *slots,
+ struct hlist_head *head,
+ int *first_slot_idx)
+{
+ struct security_hook_list *pos;
+ struct static_slot *slot;
+ int slot_cnt;
+
+ slot_cnt = 0;
+ hlist_for_each_entry_rcu(pos, head, list)
+ slot_cnt++;
+
+ if (slot_cnt > SECURITY_STATIC_SLOT_COUNT)
+ panic("%s - No static hook slot remaining to add LSM hook.\n",
+ __func__);
+
+ if (slot_cnt == 0) {
+ *first_slot_idx = INT_MAX;
+ return;
+ }
+
+ *first_slot_idx = SECURITY_STATIC_SLOT_COUNT - slot_cnt;
+ slot = slots + *first_slot_idx;
+ hlist_for_each_entry_rcu(pos, head, list) {
+ __static_call_update(slot->key, slot->trampoline,
+ pos->hook.generic_func);
+ slot++;
+ }
+}
+
+static void __init lsm_init_static_slots(void)
+{
+#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
+ lsm_init_hook_static_slot(static_slots.NAME, \
+ &security_hook_heads.NAME, \
+ &base_slot_idx.NAME);
+#include <linux/lsm_hook_defs.h>
+#undef LSM_HOOK
+}
+
static void __init lsm_early_cred(struct cred *cred);
static void __init lsm_early_task(struct task_struct *task);
@@ -354,6 +518,7 @@ static void __init ordered_lsm_init(void)
lsm_early_task(current);
for (lsm = ordered_lsms; *lsm; lsm++)
initialize_lsm(*lsm);
+ lsm_init_static_slots();
kfree(ordered_lsms);
}
@@ -374,6 +539,7 @@ int __init early_security_init(void)
prepare_lsm(lsm);
initialize_lsm(lsm);
}
+ lsm_init_static_slots();
return 0;
}
@@ -696,27 +862,36 @@ static void __init lsm_early_task(struct task_struct *task)
* call_int_hook:
* This is a hook that returns a value.
*/
-
-#define call_void_hook(FUNC, ...) \
- do { \
- struct security_hook_list *P; \
- \
- hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
- P->hook.FUNC(__VA_ARGS__); \
- } while (0)
-
-#define call_int_hook(FUNC, IRC, ...) ({ \
- int RC = IRC; \
- do { \
- struct security_hook_list *P; \
- \
- hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
- RC = P->hook.FUNC(__VA_ARGS__); \
- if (RC != 0) \
- break; \
- } \
- } while (0); \
- RC; \
+#define __CASE_CALL_STATIC_VOID(NUM, HOOK, ...) \
+ case NUM: \
+ static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
+ fallthrough;
+
+#define call_void_hook(FUNC, ...) do { \
+ switch (base_slot_idx.FUNC) { \
+ SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_VOID, \
+ FUNC, __VA_ARGS__) \
+ default : \
+ break; \
+ } \
+} while (0)
+
+#define __CASE_CALL_STATIC_INT(NUM, R, HOOK, ...) \
+ case NUM: \
+ R = static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
+ if (R != 0) \
+ break; \
+ fallthrough;
+
+#define call_int_hook(FUNC, IRC, ...) ({ \
+ int RC = IRC; \
+ switch (base_slot_idx.FUNC) { \
+ SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_INT, \
+ RC, FUNC, __VA_ARGS__) \
+ default : \
+ break; \
+ } \
+ RC; \
})
/* Security operations */
--
2.28.0.297.g1956fa8f8d-goog
^ permalink raw reply related
* Re: [PATCH 5.8 000/232] 5.8.3-rc1 review
From: Eric Biggers @ 2020-08-20 18:25 UTC (permalink / raw)
To: Naresh Kamboju
Cc: Greg Kroah-Hartman, open list, Shuah Khan, patches, lkft-triage,
Ben Hutchings, linux- stable, Andrew Morton, Linus Torvalds,
Guenter Roeck, Herbert Xu, LTP List, linux-security-module,
keyrings
In-Reply-To: <CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com>
On Thu, Aug 20, 2020 at 08:57:57PM +0530, Naresh Kamboju wrote:
> On Thu, 20 Aug 2020 at 14:55, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > This is the start of the stable review cycle for the 5.8.3 release.
> > There are 232 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Sat, 22 Aug 2020 09:15:09 +0000.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.3-rc1.gz
> > or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.8.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
>
> > Herbert Xu <herbert@gondor.apana.org.au>
> > crypto: af_alg - Fix regression on empty requests
>
> Results from Linaro’s test farm.
> Regressions detected.
>
> ltp-crypto-tests:
> * af_alg02
> ltp-cve-tests:
> * cve-2017-17805
>
> af_alg02.c:52: BROK: Timed out while reading from request socket.
> We are running the LTP 20200515 tag released test suite.
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
>
> Summary
> ------------------------------------------------------------------------
>
> kernel: 5.8.3-rc1
> git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> git branch: linux-5.8.y
> git commit: 201fff807310ce10485bcff294d47be95f3769eb
> git describe: v5.8.2-233-g201fff807310
> Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-5.8-oe/build/v5.8.2-233-g201fff807310
>
> Regressions (compared to build v5.8.2)
> ------------------------------------------------------------------------
>
> x15:
> ltp-crypto-tests:
> * af_alg02
>
> ltp-cve-tests:
> * cve-2017-17805
>
Looks like this test is still "broken" because it assumes behavior that isn't
clearly specified, as previously discussed at
https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au.
I sent out LTP patches to fix it:
https://lkml.kernel.org/linux-crypto/20200820181918.404758-1-ebiggers@kernel.org/T/#u
- Eric
^ permalink raw reply
* Re: [PATCH RESEND] device_cgroup: Fix RCU list debugging warning
From: James Morris @ 2020-08-20 18:28 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Amol Grover, Serge E. Hallyn, Paul E. McKenney, linux-kernel,
linux-kernel-mentees, Joel Fernandes, Madhuparna Bhowmik,
linux-security-module
In-Reply-To: <20200817120731.218e1bc5@canb.auug.org.au>
On Mon, 17 Aug 2020, Stephen Rothwell wrote:
> > > mainline, so it can go up any tree. I can take it if no one else will,
> > > but it might be better going in via the security tree.
> >
> > James, do you mind pulling it in?
>
> I am still carrying this patch. Has it been superceded, or is it still
> necessary?
It appears to be necessary.
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git fixes-v5.9
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v6 0/3] SELinux support for anonymous inodes and UFFD
From: James Morris @ 2020-08-20 18:35 UTC (permalink / raw)
To: Lokesh Gidra
Cc: Alexander Viro, Stephen Smalley, Casey Schaufler, Eric Biggers,
Serge E. Hallyn, Paul Moore, Eric Paris, Daniel Colascione,
Kees Cook, Eric W. Biederman, KP Singh, David Howells,
Thomas Cedeno, Anders Roxell, Sami Tolvanen, Matthew Garrett,
Aaron Goidel, Randy Dunlap, Joel Fernandes (Google), YueHaibing,
Christian Brauner, Alexei Starovoitov, Alexey Budankov,
Adrian Reber, Aleksa Sarai, linux-fsdevel, linux-kernel,
linux-security-module, selinux, kaleshsingh, calin, surenb, nnk,
jeffv, kernel-team
In-Reply-To: <20200807224941.3440722-1-lokeshgidra@google.com>
On Fri, 7 Aug 2020, Lokesh Gidra wrote:
> Userfaultfd in unprivileged contexts could be potentially very
> useful. We'd like to harden userfaultfd to make such unprivileged use
> less risky. This patch series allows SELinux to manage userfaultfd
> file descriptors and in the future, other kinds of
> anonymous-inode-based file descriptor. SELinux policy authors can
> apply policy types to anonymous inodes by providing name-based
> transition rules keyed off the anonymous inode internal name (
> "[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
> applying policy to the new SIDs thus produced.
Can you expand more on why this would be useful, e.g. use-cases?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC] security: replace indirect calls with static calls
From: James Morris @ 2020-08-20 18:43 UTC (permalink / raw)
To: Brendan Jackman
Cc: linux-kernel, bpf, linux-security-module, Paul Renauld,
Alexei Starovoitov, Daniel Borkmann, pjt, jannh, peterz,
rafael.j.wysocki, keescook, thgarnie, kpsingh, paul.renauld.epfl,
Brendan Jackman
In-Reply-To: <20200820164753.3256899-1-jackmanb@chromium.org>
On Thu, 20 Aug 2020, Brendan Jackman wrote:
> With this implementation, any overhead of the indirect call in the LSM
> framework is completely mitigated (performance results: [7]). This
> facilitates the adoption of "bpf" LSM on production machines and also
> benefits all other LSMs.
This looks like a potentially useful improvement, although I wonder if it
would be overshadowed by an LSM hook doing real work.
Do you have any more benchmarking beyond eventfd_write() ?
>
> [1]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/
> [2]: https://lwn.net/Articles/798157/
> [3] measurements: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
> protocol: https://gist.github.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5#file-measurement-protocol-md
> [4]: https://lwn.net/Articles/813261/
> [5]: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/static_call
> [6]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/#t
> [7]: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: James Morris <jmorris@namei.org>
> Cc: pjt@google.com
> Cc: jannh@google.com
> Cc: peterz@infradead.org
> Cc: rafael.j.wysocki@intel.com
> Cc: keescook@chromium.org
> Cc: thgarnie@chromium.org
> Cc: kpsingh@google.com
> Cc: paul.renauld.epfl@gmail.com
>
> Signed-off-by: Paul Renauld <renauld@google.com>
> Signed-off-by: KP Singh <kpsingh@google.com>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> include/linux/lsm_static_call.h | 134 ++++++++++++++++++++
> security/security.c | 217 ++++++++++++++++++++++++++++----
> 3 files changed, 331 insertions(+), 21 deletions(-)
> create mode 100644 include/linux/lsm_static_call.h
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 95b7c1d32062..d11e116b588e 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1524,6 +1524,7 @@ union security_list_options {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
> #include "lsm_hook_defs.h"
> #undef LSM_HOOK
> + void *generic_func;
> };
>
> struct security_hook_heads {
> diff --git a/include/linux/lsm_static_call.h b/include/linux/lsm_static_call.h
> new file mode 100644
> index 000000000000..f5f5698292e0
> --- /dev/null
> +++ b/include/linux/lsm_static_call.h
> @@ -0,0 +1,134 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2020 Google LLC.
> + */
> +
> +#ifndef __LINUX_LSM_STATIC_CALL_H
> +#define __LINUX_LSM_STATIC_CALL_H
> +
> +/*
> + * Static slots are used in security/security.c to avoid costly
> + * indirect calls by replacing them with static calls.
> + * The number of static calls for each LSM hook is fixed.
> + */
> +#define SECURITY_STATIC_SLOT_COUNT 11
> +
> +/*
> + * Identifier for the LSM static slots.
> + * HOOK is an LSM hook as defined in linux/lsm_hookdefs.h
> + * IDX is the index of the slot. 0 <= NUM < SECURITY_STATIC_SLOT_COUNT
> + */
> +#define STATIC_SLOT(HOOK, IDX) security_static_slot_##HOOK##_##IDX
> +
> +/*
> + * Call the macro M for each LSM hook slot.
> + * M should take as first argument the index and then
> + * the same __VA_ARGS__
> + * Essentially, this will expand to:
> + * M(0, ...)
> + * M(1, ...)
> + * M(2, ...)
> + * ...
> + * Note that no trailing semicolon is placed so M should be defined
> + * accordingly.
> + * This adapts to a change to SECURITY_STATIC_SLOT_COUNT.
> + */
> +#define SECURITY_FOREACH_STATIC_SLOT(M, ...) \
> + UNROLL_MACRO_LOOP(SECURITY_STATIC_SLOT_COUNT, M, __VA_ARGS__)
> +
> +/*
> + * Intermediate macros to expand SECURITY_STATIC_SLOT_COUNT
> + */
> +#define UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + _UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define _UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + __UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + __UNROLL_MACRO_LOOP_##N(MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_0(MACRO, ...)
> +
> +#define __UNROLL_MACRO_LOOP_1(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_0(MACRO, __VA_ARGS__) \
> + MACRO(0, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_2(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_1(MACRO, __VA_ARGS__) \
> + MACRO(1, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_3(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_2(MACRO, __VA_ARGS__) \
> + MACRO(2, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_4(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_3(MACRO, __VA_ARGS__) \
> + MACRO(3, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_5(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_4(MACRO, __VA_ARGS__) \
> + MACRO(4, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_6(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_5(MACRO, __VA_ARGS__) \
> + MACRO(5, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_7(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_6(MACRO, __VA_ARGS__) \
> + MACRO(6, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_8(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_7(MACRO, __VA_ARGS__) \
> + MACRO(7, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_9(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_8(MACRO, __VA_ARGS__) \
> + MACRO(8, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_10(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_9(MACRO, __VA_ARGS__) \
> + MACRO(9, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_11(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_10(MACRO, __VA_ARGS__) \
> + MACRO(10, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_12(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_11(MACRO, __VA_ARGS__) \
> + MACRO(11, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_13(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_12(MACRO, __VA_ARGS__) \
> + MACRO(12, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_14(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_13(MACRO, __VA_ARGS__) \
> + MACRO(13, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_15(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_14(MACRO, __VA_ARGS__) \
> + MACRO(14, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_16(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_15(MACRO, __VA_ARGS__) \
> + MACRO(15, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_17(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_16(MACRO, __VA_ARGS__) \
> + MACRO(16, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_18(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_17(MACRO, __VA_ARGS__) \
> + MACRO(17, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_19(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_18(MACRO, __VA_ARGS__) \
> + MACRO(18, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_20(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_19(MACRO, __VA_ARGS__) \
> + MACRO(19, __VA_ARGS__)
> +
> +#endif /* __LINUX_LSM_STATIC_CALL_H */
> diff --git a/security/security.c b/security/security.c
> index 70a7ad357bc6..15026bc716f2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,8 @@
> #include <linux/string.h>
> #include <linux/msg.h>
> #include <net/flow.h>
> +#include <linux/static_call.h>
> +#include <linux/lsm_static_call.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -86,6 +88,128 @@ static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
> static __initdata struct lsm_info **ordered_lsms;
> static __initdata struct lsm_info *exclusive;
>
> +/*
> + * Necessary information about a static
> + * slot to call __static_call_update
> + */
> +struct static_slot {
> + /* static call key as defined by STATIC_CALL_KEY */
> + struct static_call_key *key;
> + /* static call trampoline as defined by STATIC_CALL_TRAMP */
> + void *trampoline;
> +};
> +
> +/*
> + * Table of the static calls for each LSM hook.
> + * Once the LSMs are initialized, their callbacks will be copied to these
> + * tables such that the slots are filled backwards (from last to first).
> + * This way, we can jump directly to the first used slot, and execute
> + * all of them after. This essentially makes the entry point point
> + * dynamic to adapt the number of slot to the number of callbacks.
> + */
> +struct static_slot_list {
> + #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + struct static_slot NAME[SECURITY_STATIC_SLOT_COUNT];
> + #include <linux/lsm_hook_defs.h>
> + #undef LSM_HOOK
> +} __randomize_layout;
> +
> +/*
> + * Index of the first used static call for each LSM hook
> + * in the corresponding static_slot_list table.
> + * All slots with greater indices are used.
> + * If no slot is used, the default value is INT_MAX.
> + */
> +struct base_slot_idx {
> + #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + int NAME;
> + #include <linux/lsm_hook_defs.h>
> + #undef LSM_HOOK
> +} __randomize_layout;
> +
> +/*
> + * Create the static slots for each LSM hook, initially empty.
> + * This will expand to:
> + *
> + * [...]
> + *
> + * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_0,
> + * *((int(*)(struct file *file, int mask)))NULL);
> + * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_1, ...);
> + *
> + * [...]
> + */
> +#define CREATE_STATIC_SLOT(NUM, NAME, RET, ...) \
> + DEFINE_STATIC_CALL_NULL(STATIC_SLOT(NAME, NUM), \
> + *((RET(*)(__VA_ARGS__))NULL));
> +
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + SECURITY_FOREACH_STATIC_SLOT(CREATE_STATIC_SLOT, NAME, RET, __VA_ARGS__)
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +#undef CREATE_STATIC_SLOT
> +
> +/*
> + * Initialise a table of static slots for each LSM hook.
> + * When defined with DEFINE_STATIC_CALL_NULL as above, a static call is
> + * a key and a trampoline. Both are needed to use __static_call_update.
> + * This will expand to:
> + * struct static_slot_list static_slots = {
> + * [...]
> + * .file_permission = {
> + * (struct static_slot) {
> + * .key = &STATIC_CALL_KEY(
> + * security_static_slot_file_permission_0),
> + * .trampoline = &STATIC_CALL_TRAMP(
> + * security_static_slot_file_permission_0)
> + * },
> + * (struct static_slot) {
> + * .key = &STATIC_CALL_KEY(
> + * security_static_slot_file_permission_1),
> + * .trampoline = &STATIC_CALL_TRAMP(
> + * security_static_slot_file_permission_1)
> + * },
> + * [...]
> + * },
> + * .file_alloc_security = {
> + * [...]
> + * },
> + * [...]
> + * }
> + */
> +static struct static_slot_list static_slots __initdata = {
> +#define DEFINE_SLOT(NUM, NAME) \
> + (struct static_slot) { \
> + .key = &STATIC_CALL_KEY(STATIC_SLOT(NAME, NUM)), \
> + .trampoline = &STATIC_CALL_TRAMP(STATIC_SLOT(NAME, NUM))\
> + },
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + .NAME = { \
> + SECURITY_FOREACH_STATIC_SLOT(DEFINE_SLOT, NAME) \
> + },
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +#undef DEFINE_SLOT
> +};
> +
> +/*
> + * The base slot index for each is initially INT_MAX, which means
> + * that no slot is used yet.
> + * When expanded, this results in:
> + * struct base_slot_idx base_slot_idx = {
> + * [...]
> + * .file_permission = INT_MAX,
> + * .file_alloc_security = INT_MAX,
> + * [...]
> + * }
> + */
> +static struct base_slot_idx base_slot_idx __lsm_ro_after_init = {
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + .NAME = INT_MAX,
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +};
> +
> static __initdata bool debug;
> #define init_debug(...) \
> do { \
> @@ -307,6 +431,46 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> kfree(sep);
> }
>
> +static void __init lsm_init_hook_static_slot(struct static_slot *slots,
> + struct hlist_head *head,
> + int *first_slot_idx)
> +{
> + struct security_hook_list *pos;
> + struct static_slot *slot;
> + int slot_cnt;
> +
> + slot_cnt = 0;
> + hlist_for_each_entry_rcu(pos, head, list)
> + slot_cnt++;
> +
> + if (slot_cnt > SECURITY_STATIC_SLOT_COUNT)
> + panic("%s - No static hook slot remaining to add LSM hook.\n",
> + __func__);
> +
> + if (slot_cnt == 0) {
> + *first_slot_idx = INT_MAX;
> + return;
> + }
> +
> + *first_slot_idx = SECURITY_STATIC_SLOT_COUNT - slot_cnt;
> + slot = slots + *first_slot_idx;
> + hlist_for_each_entry_rcu(pos, head, list) {
> + __static_call_update(slot->key, slot->trampoline,
> + pos->hook.generic_func);
> + slot++;
> + }
> +}
> +
> +static void __init lsm_init_static_slots(void)
> +{
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + lsm_init_hook_static_slot(static_slots.NAME, \
> + &security_hook_heads.NAME, \
> + &base_slot_idx.NAME);
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +}
> +
> static void __init lsm_early_cred(struct cred *cred);
> static void __init lsm_early_task(struct task_struct *task);
>
> @@ -354,6 +518,7 @@ static void __init ordered_lsm_init(void)
> lsm_early_task(current);
> for (lsm = ordered_lsms; *lsm; lsm++)
> initialize_lsm(*lsm);
> + lsm_init_static_slots();
>
> kfree(ordered_lsms);
> }
> @@ -374,6 +539,7 @@ int __init early_security_init(void)
> prepare_lsm(lsm);
> initialize_lsm(lsm);
> }
> + lsm_init_static_slots();
>
> return 0;
> }
> @@ -696,27 +862,36 @@ static void __init lsm_early_task(struct task_struct *task)
> * call_int_hook:
> * This is a hook that returns a value.
> */
> -
> -#define call_void_hook(FUNC, ...) \
> - do { \
> - struct security_hook_list *P; \
> - \
> - hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
> - P->hook.FUNC(__VA_ARGS__); \
> - } while (0)
> -
> -#define call_int_hook(FUNC, IRC, ...) ({ \
> - int RC = IRC; \
> - do { \
> - struct security_hook_list *P; \
> - \
> - hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
> - RC = P->hook.FUNC(__VA_ARGS__); \
> - if (RC != 0) \
> - break; \
> - } \
> - } while (0); \
> - RC; \
> +#define __CASE_CALL_STATIC_VOID(NUM, HOOK, ...) \
> + case NUM: \
> + static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
> + fallthrough;
> +
> +#define call_void_hook(FUNC, ...) do { \
> + switch (base_slot_idx.FUNC) { \
> + SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_VOID, \
> + FUNC, __VA_ARGS__) \
> + default : \
> + break; \
> + } \
> +} while (0)
> +
> +#define __CASE_CALL_STATIC_INT(NUM, R, HOOK, ...) \
> + case NUM: \
> + R = static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
> + if (R != 0) \
> + break; \
> + fallthrough;
> +
> +#define call_int_hook(FUNC, IRC, ...) ({ \
> + int RC = IRC; \
> + switch (base_slot_idx.FUNC) { \
> + SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_INT, \
> + RC, FUNC, __VA_ARGS__) \
> + default : \
> + break; \
> + } \
> + RC; \
> })
>
> /* Security operations */
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC] security: replace indirect calls with static calls
From: KP Singh @ 2020-08-20 19:04 UTC (permalink / raw)
To: James Morris
Cc: Brendan Jackman, Linux Kernel Mailing List, bpf,
linux-security-module, Paul Renauld, Alexei Starovoitov,
Daniel Borkmann, Paul Turner, Jann Horn, peterz, rafael.j.wysocki,
keescook, thgarnie, paul.renauld.epfl, Brendan Jackman
In-Reply-To: <alpine.LRH.2.21.2008210439190.29407@namei.org>
On Thu, Aug 20, 2020 at 8:43 PM James Morris <jmorris@namei.org> wrote:
>
> On Thu, 20 Aug 2020, Brendan Jackman wrote:
>
> > With this implementation, any overhead of the indirect call in the LSM
> > framework is completely mitigated (performance results: [7]). This
> > facilitates the adoption of "bpf" LSM on production machines and also
> > benefits all other LSMs.
>
> This looks like a potentially useful improvement, although I wonder if it
> would be overshadowed by an LSM hook doing real work.
>
Thanks for taking a look!
We can surely look at other examples, but the real goal is to
optimize the case where the "bpf" LSM adds callbacks to every LSM hook
which don't do any real work and cause an avoidable overhead.
This makes it not very practical for data center environments where
one would want a framework that adds a zero base case overhead and
allows the user to decide where to hook / add performance penalties.
(at boot time for other LSMs and at runtime for bpf)
I also think this would be beneficial for LSMs which use a cache for
a faster policy decision (e.g. access vector caching in SELinux).
- KP
> Do you have any more benchmarking beyond eventfd_write() ?
>
>
>
> >
> > [1]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/
[...]
> >
> > /* Security operations */
> >
>
> --
> James Morris
> <jmorris@namei.org>
>
^ permalink raw reply
* Re: [PATCH] mm/migrate: Avoid possible unnecessary ptrace_may_access() call in kernel_move_pages()
From: Kees Cook @ 2020-08-20 21:21 UTC (permalink / raw)
To: linmiaohe
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <9ce6209f41b64734b2cac748783aa441@huawei.com>
On Thu, Aug 20, 2020 at 02:18:21AM +0000, linmiaohe wrote:
> Kees Cook <keescook@chromium.org> wrote:
> >On Mon, Aug 17, 2020 at 07:59:33AM -0400, Miaohe Lin wrote:
> >> There is no need to check if this process has the right to modify the
> >> specified process when they are same.
> >>
> >> Signed-off-by: Hongxiang Lou <louhongxiang@huawei.com>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >
> >NAK, please don't do this -- the ptrace and security hooks already do these kinds of self-introspection checks, and I'd like to keep a central place to perform these kinds of checks.
> >
>
> Many thanks for your reply.
> We also avoid get_task_struct/ put_task_struct pair of atomic ops, rcu_lock, task_lock and so on this way.
>
> >Is there a specific problem you've encountered that this fixes?
> >
>
> I'am sorry but there's no specific problem. I do this mainly to skip the unnecessary ptrace and security hooks.
Cool. Let's keep this as-is so we continue to have centralized
instrumentation of these things in the LSM. :)
Thanks for your attention to performance!
--
Kees Cook
^ permalink raw reply
* Re: [RFC] security: replace indirect calls with static calls
From: Kees Cook @ 2020-08-20 21:45 UTC (permalink / raw)
To: Brendan Jackman
Cc: linux-kernel, bpf, linux-security-module, Paul Renauld,
Alexei Starovoitov, Daniel Borkmann, James Morris, pjt, jannh,
peterz, rafael.j.wysocki, thgarnie, kpsingh, paul.renauld.epfl,
Brendan Jackman
In-Reply-To: <20200820164753.3256899-1-jackmanb@chromium.org>
On Thu, Aug 20, 2020 at 06:47:53PM +0200, Brendan Jackman wrote:
> From: Paul Renauld <renauld@google.com>
>
> LSMs have high overhead due to indirect function calls through
> retpolines. This RPC proposes to replace these with static calls [1]
typo: RFC
> instead.
Yay! :)
> [...]
> This overhead prevents the adoption of bpf LSM on performance critical
> systems, and also, in general, slows down all LSMs.
I'd be curious to see other workloads too. (Your measurements are a bit
synthetic, mostly showing "worst case": one short syscall in a tight
loop. I'm curious how much performance gain can be had -- we should
still do it, it'll be a direct performance improvement, but I'm curious
about "real world" impact too.)
> [...]
> Previously, the code for this hook would have looked like this:
>
> ret = DEFAULT_RET;
>
> for each cb in [A, B, C]:
> ret = cb(args); <--- costly indirect call here
> if ret != 0:
> break;
>
> return ret;
>
> Static calls are defined at build time and are initially empty (NOP
> instructions). When the LSMs are initialized, the slots are filled as
> follows:
>
> slot idx content
> |-----------|
> 0 | |
> |-----------|
> 1 | |
> |-----------|
> 2 | call A | <-- base_slot_idx = 2
> |-----------|
> 3 | call B |
> |-----------|
> 4 | call C |
> |-----------|
>
> The generated code will unroll the foreach loop to have a static call for
> each possible LSM:
>
> ret = DEFAULT_RET;
> switch(base_slot_idx):
>
> case 0:
> NOP
> if ret != 0:
> break;
> // fallthrough
> case 1:
> NOP
> if ret != 0:
> break;
> // fallthrough
> case 2:
> ret = A(args); <--- direct call, no retpoline
> if ret != 0:
> break;
> // fallthrough
> case 3:
> ret = B(args); <--- direct call, no retpoline
> if ret != 0:
> break;
> // fallthrough
>
> [...]
>
> default:
> break;
>
> return ret;
>
> A similar logic is applied for void hooks.
>
> Why this trick with a switch statement? The table of static call is defined
> at compile time. The number of hook callbacks that will be defined is
> unknown at that time, and the table cannot be resized at runtime. Static
> calls do not define a conditional execution for a non-void function, so the
> executed slots must be non-empty. With this use of the table and the
> switch, it is possible to jump directly to the first used slot and execute
> all of the slots after. This essentially makes the entry point of the table
> dynamic. Instead, it would also be possible to start from 0 and break after
> the final populated slot, but that would require an additional conditional
> after each slot.
Instead of just "NOP", having the static branches perform a jump would
solve this pretty cleanly, yes? Something like:
ret = DEFAULT_RET;
ret = A(args); <--- direct call, no retpoline
if ret != 0:
goto out;
ret = B(args); <--- direct call, no retpoline
if ret != 0:
goto out;
goto out;
if ret != 0:
goto out;
out:
return ret;
> [...]
> The number of available slots for each LSM hook is currently fixed at
> 11 (the number of LSMs in the kernel). Ideally, it should automatically
> adapt to the number of LSMs compiled into the kernel.
Seems like a reasonable thing to do and could be a separate patch.
> If there’s no practical way to implement such automatic adaptation, an
> option instead would be to remove the panic call by falling-back to the old
> linked-list mechanism, which is still present anyway (see below).
>
> A few special cases of LSM don't use the macro call_[int/void]_hook but
> have their own calling logic. The linked-lists are kept as a possible slow
> path fallback for them.
I assume you mean the integrity subsystem? That just needs to be fixed
correctly. If we switch to this, let's ditch the linked list entirely.
Fixing integrity's stacking can be a separate patch too.
> [...]
> Signed-off-by: Paul Renauld <renauld@google.com>
> Signed-off-by: KP Singh <kpsingh@google.com>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
This implies a maintainership chain, with Paul as the sole author. If
you mean all of you worked on the patch, include Co-developed-by: as
needed[1].
-Kees
[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
--
Kees Cook
^ permalink raw reply
* Re: [PATCH RESEND] device_cgroup: Fix RCU list debugging warning
From: Stephen Rothwell @ 2020-08-20 22:38 UTC (permalink / raw)
To: James Morris
Cc: Amol Grover, Serge E. Hallyn, Paul E. McKenney, linux-kernel,
linux-kernel-mentees, Joel Fernandes, Madhuparna Bhowmik,
linux-security-module
In-Reply-To: <alpine.LRH.2.21.2008210427590.29407@namei.org>
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]
Hi James,
On Fri, 21 Aug 2020 04:28:34 +1000 (AEST) James Morris <jmorris@namei.org> wrote:
>
> On Mon, 17 Aug 2020, Stephen Rothwell wrote:
>
> > > > mainline, so it can go up any tree. I can take it if no one else will,
> > > > but it might be better going in via the security tree.
> > >
> > > James, do you mind pulling it in?
> >
> > I am still carrying this patch. Has it been superceded, or is it still
> > necessary?
>
> It appears to be necessary.
>
> Applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git fixes-v5.9
Thanks, I have dropped my copy.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC] security: replace indirect calls with static calls
From: Casey Schaufler @ 2020-08-20 22:46 UTC (permalink / raw)
To: Brendan Jackman, linux-kernel, bpf, linux-security-module
Cc: Paul Renauld, Alexei Starovoitov, Daniel Borkmann, James Morris,
pjt, jannh, peterz, rafael.j.wysocki, keescook, thgarnie, kpsingh,
paul.renauld.epfl, Brendan Jackman, Casey Schaufler
In-Reply-To: <20200820164753.3256899-1-jackmanb@chromium.org>
On 8/20/2020 9:47 AM, Brendan Jackman wrote:
> From: Paul Renauld <renauld@google.com>
>
> LSMs have high overhead due to indirect function calls through
> retpolines. This RPC proposes to replace these with static calls [1]
> instead.
>
> This overhead is especially significant for the "bpf" LSM which supports
> the implementation of LSM hooks with eBPF programs (security/bpf)[2]. In
> order to facilitate this, the "bpf" LSM provides a default nop callback for
> all LSM hooks. When enabled, the "bpf", LSM incurs an unnecessary /
> avoidable indirect call to this nop callback.
>
> The performance impact on a simple syscall eventfd_write (which triggers
> the file_permission hook) was measured with and without "bpf" LSM
> enabled. Activating the LSM resulted in an overhead of 4% [3].
>
> This overhead prevents the adoption of bpf LSM on performance critical
> systems, and also, in general, slows down all LSMs.
>
> Currently, the LSM hook callbacks are stored in a linked list and
> dispatched as indirect calls. Using static calls can remove this overhead
> by replacing all indirect calls with direct calls.
>
> During the discussion of the "bpf" LSM patch-set it was proposed to special
> case BPF LSM to avoid the overhead by using static keys. This was however
> not accepted and it was decided to [4]:
>
> - Not special-case the "bpf" LSM.
> - Implement a general solution benefitting the whole LSM framework.
>
> This is based on the static call branch [5].
>
> For each LSM hook, a table of static calls is defined (referred to as
> "static slots", or "slots"). When all the LSMs are initialized and linked
> lists are filled, the hook callbacks are copied to the appropriate static
> slot. The callbacks are continuously added at the end of the table, and the
> index of the first slot that is non empty is stored. Then, when a LSM hook
> is called (macro call_[int/void]_hook), the execution jumps to this first
> non-empty slot and all of the subsequent static slots are executed.
>
> The static calls are re-initialized every time the linked list is modified,
> i.e. after the early LSM init, and the LSM init.
>
> Let's say, there are 5 static slots per LSM hook, and 3 LSMs implement some
> hook with the callbacks A, B, C.
>
> Previously, the code for this hook would have looked like this:
>
> ret = DEFAULT_RET;
>
> for each cb in [A, B, C]:
> ret = cb(args); <--- costly indirect call here
> if ret != 0:
> break;
>
> return ret;
>
> Static calls are defined at build time and are initially empty (NOP
> instructions). When the LSMs are initialized, the slots are filled as
> follows:
>
> slot idx content
> |-----------|
> 0 | |
> |-----------|
> 1 | |
> |-----------|
> 2 | call A | <-- base_slot_idx = 2
> |-----------|
> 3 | call B |
> |-----------|
> 4 | call C |
> |-----------|
>
> The generated code will unroll the foreach loop to have a static call for
> each possible LSM:
>
> ret = DEFAULT_RET;
> switch(base_slot_idx):
>
> case 0:
> NOP
What does NOP really look like?
> if ret != 0:
I assume you'd want "ret != DEFAULT_RET" instead of "ret != 0".
> break;
> // fallthrough
> case 1:
> NOP
> if ret != 0:
> break;
> // fallthrough
> case 2:
> ret = A(args); <--- direct call, no retpoline
> if ret != 0:
> break;
> // fallthrough
> case 3:
> ret = B(args); <--- direct call, no retpoline
> if ret != 0:
> break;
> // fallthrough
>
> [...]
>
> default:
> break;
>
> return ret;
>
> A similar logic is applied for void hooks.
>
> Why this trick with a switch statement? The table of static call is defined
> at compile time. The number of hook callbacks that will be defined is
> unknown at that time, and the table cannot be resized at runtime. Static
> calls do not define a conditional execution for a non-void function, so the
> executed slots must be non-empty.
So what goes in for empty slots? What about gaps in the table?
> With this use of the table and the
> switch, it is possible to jump directly to the first used slot and execute
> all of the slots after. This essentially makes the entry point of the table
> dynamic. Instead, it would also be possible to start from 0 and break after
> the final populated slot, but that would require an additional conditional
> after each slot.
>
> This macro is used to generate the code for each static slot, (e.g. each
> case statement in the previous example). This will expand into a call to
> MACRO for each static slot defined. For example, if with again 5 slots:
>
> SECURITY_FOREACH_STATIC_SLOT(MACRO, x, y) ->
>
> MACRO(0, x, y)
> MACRO(1, x, y)
> MACRO(2, x, y)
> MACRO(3, x, y)
> MACRO(4, x, y)
>
> This is used in conjunction with LSM_HOOK definitions in
> linux/lsm_hook_defs.h to execute a macro for each static slot of each LSM
> hook.
>
> The patches for static calls [6] are not upstreamed yet.
>
> The number of available slots for each LSM hook is currently fixed at
> 11 (the number of LSMs in the kernel). Ideally, it should automatically
> adapt to the number of LSMs compiled into the kernel.
#define SECURITY_STATIC_SLOT_COUNT ( \
1 + /* Capability module is always there */ \
(IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
(IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
... \
(IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0))
> If there’s no practical way to implement such automatic adaptation, an
> option instead would be to remove the panic call by falling-back to the old
> linked-list mechanism, which is still present anyway (see below).
>
> A few special cases of LSM don't use the macro call_[int/void]_hook but
> have their own calling logic. The linked-lists are kept as a possible slow
> path fallback for them.
Unfortunately, the stacking effort is increasing the number of hooks
that will require special handling. security_secid_to_secctx() is one
example.
>
> Before:
>
> https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
>
> After:
>
> https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
>
> With this implementation, any overhead of the indirect call in the LSM
> framework is completely mitigated (performance results: [7]). This
> facilitates the adoption of "bpf" LSM on production machines and also
> benefits all other LSMs.
Your numbers for a system with BPF are encouraging. What do the numbers
look like for a system with SELinux, Smack or AppArmor?
>
> [1]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/
> [2]: https://lwn.net/Articles/798157/
> [3] measurements: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/62437b1416829ca0e8a0ed9101530bc90fd42d69/lsm-performance.png
> protocol: https://gist.github.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5#file-measurement-protocol-md
> [4]: https://lwn.net/Articles/813261/
> [5]: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/static_call
> [6]: https://lwn.net/ml/linux-kernel/20200710133831.943894387@infradead.org/#t
> [7]: https://gist.githubusercontent.com/PaulRenauld/fe3ee7b51121556e03c181432c8b3dd5/raw/00e414b73e0c38c2eae8f05d5363a745179ba285/faster-lsm-results.png
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: James Morris <jmorris@namei.org>
> Cc: pjt@google.com
> Cc: jannh@google.com
> Cc: peterz@infradead.org
> Cc: rafael.j.wysocki@intel.com
> Cc: keescook@chromium.org
> Cc: thgarnie@chromium.org
> Cc: kpsingh@google.com
> Cc: paul.renauld.epfl@gmail.com
>
> Signed-off-by: Paul Renauld <renauld@google.com>
> Signed-off-by: KP Singh <kpsingh@google.com>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> include/linux/lsm_static_call.h | 134 ++++++++++++++++++++
> security/security.c | 217 ++++++++++++++++++++++++++++----
> 3 files changed, 331 insertions(+), 21 deletions(-)
> create mode 100644 include/linux/lsm_static_call.h
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 95b7c1d32062..d11e116b588e 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1524,6 +1524,7 @@ union security_list_options {
> #define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
> #include "lsm_hook_defs.h"
> #undef LSM_HOOK
> + void *generic_func;
> };
>
> struct security_hook_heads {
> diff --git a/include/linux/lsm_static_call.h b/include/linux/lsm_static_call.h
> new file mode 100644
> index 000000000000..f5f5698292e0
> --- /dev/null
> +++ b/include/linux/lsm_static_call.h
> @@ -0,0 +1,134 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2020 Google LLC.
> + */
> +
> +#ifndef __LINUX_LSM_STATIC_CALL_H
> +#define __LINUX_LSM_STATIC_CALL_H
> +
> +/*
> + * Static slots are used in security/security.c to avoid costly
> + * indirect calls by replacing them with static calls.
> + * The number of static calls for each LSM hook is fixed.
> + */
> +#define SECURITY_STATIC_SLOT_COUNT 11
See suggested code above.
> +
> +/*
> + * Identifier for the LSM static slots.
> + * HOOK is an LSM hook as defined in linux/lsm_hookdefs.h
> + * IDX is the index of the slot. 0 <= NUM < SECURITY_STATIC_SLOT_COUNT
> + */
> +#define STATIC_SLOT(HOOK, IDX) security_static_slot_##HOOK##_##IDX
> +
> +/*
> + * Call the macro M for each LSM hook slot.
> + * M should take as first argument the index and then
> + * the same __VA_ARGS__
> + * Essentially, this will expand to:
> + * M(0, ...)
> + * M(1, ...)
> + * M(2, ...)
> + * ...
> + * Note that no trailing semicolon is placed so M should be defined
> + * accordingly.
> + * This adapts to a change to SECURITY_STATIC_SLOT_COUNT.
> + */
> +#define SECURITY_FOREACH_STATIC_SLOT(M, ...) \
> + UNROLL_MACRO_LOOP(SECURITY_STATIC_SLOT_COUNT, M, __VA_ARGS__)
> +
> +/*
> + * Intermediate macros to expand SECURITY_STATIC_SLOT_COUNT
> + */
> +#define UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + _UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define _UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + __UNROLL_MACRO_LOOP(N, MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP(N, MACRO, ...) \
> + __UNROLL_MACRO_LOOP_##N(MACRO, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_0(MACRO, ...)
> +
> +#define __UNROLL_MACRO_LOOP_1(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_0(MACRO, __VA_ARGS__) \
> + MACRO(0, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_2(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_1(MACRO, __VA_ARGS__) \
> + MACRO(1, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_3(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_2(MACRO, __VA_ARGS__) \
> + MACRO(2, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_4(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_3(MACRO, __VA_ARGS__) \
> + MACRO(3, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_5(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_4(MACRO, __VA_ARGS__) \
> + MACRO(4, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_6(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_5(MACRO, __VA_ARGS__) \
> + MACRO(5, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_7(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_6(MACRO, __VA_ARGS__) \
> + MACRO(6, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_8(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_7(MACRO, __VA_ARGS__) \
> + MACRO(7, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_9(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_8(MACRO, __VA_ARGS__) \
> + MACRO(8, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_10(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_9(MACRO, __VA_ARGS__) \
> + MACRO(9, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_11(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_10(MACRO, __VA_ARGS__) \
> + MACRO(10, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_12(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_11(MACRO, __VA_ARGS__) \
> + MACRO(11, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_13(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_12(MACRO, __VA_ARGS__) \
> + MACRO(12, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_14(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_13(MACRO, __VA_ARGS__) \
> + MACRO(13, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_15(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_14(MACRO, __VA_ARGS__) \
> + MACRO(14, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_16(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_15(MACRO, __VA_ARGS__) \
> + MACRO(15, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_17(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_16(MACRO, __VA_ARGS__) \
> + MACRO(16, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_18(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_17(MACRO, __VA_ARGS__) \
> + MACRO(17, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_19(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_18(MACRO, __VA_ARGS__) \
> + MACRO(18, __VA_ARGS__)
> +
> +#define __UNROLL_MACRO_LOOP_20(MACRO, ...) \
> + __UNROLL_MACRO_LOOP_19(MACRO, __VA_ARGS__) \
> + MACRO(19, __VA_ARGS__)
> +
Where does "20" come from? Why are you unrolling beyond 11?
> +#endif /* __LINUX_LSM_STATIC_CALL_H */
> diff --git a/security/security.c b/security/security.c
> index 70a7ad357bc6..15026bc716f2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,8 @@
> #include <linux/string.h>
> #include <linux/msg.h>
> #include <net/flow.h>
> +#include <linux/static_call.h>
> +#include <linux/lsm_static_call.h>
>
> #define MAX_LSM_EVM_XATTR 2
>
> @@ -86,6 +88,128 @@ static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
> static __initdata struct lsm_info **ordered_lsms;
> static __initdata struct lsm_info *exclusive;
>
> +/*
> + * Necessary information about a static
> + * slot to call __static_call_update
> + */
> +struct static_slot {
> + /* static call key as defined by STATIC_CALL_KEY */
> + struct static_call_key *key;
> + /* static call trampoline as defined by STATIC_CALL_TRAMP */
> + void *trampoline;
> +};
> +
> +/*
> + * Table of the static calls for each LSM hook.
> + * Once the LSMs are initialized, their callbacks will be copied to these
> + * tables such that the slots are filled backwards (from last to first).
> + * This way, we can jump directly to the first used slot, and execute
> + * all of them after. This essentially makes the entry point point
> + * dynamic to adapt the number of slot to the number of callbacks.
> + */
> +struct static_slot_list {
> + #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + struct static_slot NAME[SECURITY_STATIC_SLOT_COUNT];
> + #include <linux/lsm_hook_defs.h>
> + #undef LSM_HOOK
> +} __randomize_layout;
> +
> +/*
> + * Index of the first used static call for each LSM hook
> + * in the corresponding static_slot_list table.
> + * All slots with greater indices are used.
Again, what about gaps?
> + * If no slot is used, the default value is INT_MAX.
> + */
> +struct base_slot_idx {
> + #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + int NAME;
> + #include <linux/lsm_hook_defs.h>
> + #undef LSM_HOOK
> +} __randomize_layout;
> +
> +/*
> + * Create the static slots for each LSM hook, initially empty.
> + * This will expand to:
> + *
> + * [...]
> + *
> + * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_0,
> + * *((int(*)(struct file *file, int mask)))NULL);
> + * DEFINE_STATIC_CALL_NULL(security_static_slot_file_permission_1, ...);
> + *
> + * [...]
> + */
> +#define CREATE_STATIC_SLOT(NUM, NAME, RET, ...) \
> + DEFINE_STATIC_CALL_NULL(STATIC_SLOT(NAME, NUM), \
> + *((RET(*)(__VA_ARGS__))NULL));
> +
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + SECURITY_FOREACH_STATIC_SLOT(CREATE_STATIC_SLOT, NAME, RET, __VA_ARGS__)
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +#undef CREATE_STATIC_SLOT
> +
> +/*
> + * Initialise a table of static slots for each LSM hook.
> + * When defined with DEFINE_STATIC_CALL_NULL as above, a static call is
> + * a key and a trampoline. Both are needed to use __static_call_update.
> + * This will expand to:
> + * struct static_slot_list static_slots = {
> + * [...]
> + * .file_permission = {
> + * (struct static_slot) {
> + * .key = &STATIC_CALL_KEY(
> + * security_static_slot_file_permission_0),
> + * .trampoline = &STATIC_CALL_TRAMP(
> + * security_static_slot_file_permission_0)
> + * },
> + * (struct static_slot) {
> + * .key = &STATIC_CALL_KEY(
> + * security_static_slot_file_permission_1),
> + * .trampoline = &STATIC_CALL_TRAMP(
> + * security_static_slot_file_permission_1)
> + * },
> + * [...]
> + * },
> + * .file_alloc_security = {
> + * [...]
> + * },
> + * [...]
> + * }
> + */
> +static struct static_slot_list static_slots __initdata = {
> +#define DEFINE_SLOT(NUM, NAME) \
> + (struct static_slot) { \
> + .key = &STATIC_CALL_KEY(STATIC_SLOT(NAME, NUM)), \
> + .trampoline = &STATIC_CALL_TRAMP(STATIC_SLOT(NAME, NUM))\
> + },
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + .NAME = { \
> + SECURITY_FOREACH_STATIC_SLOT(DEFINE_SLOT, NAME) \
> + },
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +#undef DEFINE_SLOT
> +};
> +
> +/*
> + * The base slot index for each is initially INT_MAX, which means
> + * that no slot is used yet.
> + * When expanded, this results in:
> + * struct base_slot_idx base_slot_idx = {
> + * [...]
> + * .file_permission = INT_MAX,
> + * .file_alloc_security = INT_MAX,
> + * [...]
> + * }
> + */
> +static struct base_slot_idx base_slot_idx __lsm_ro_after_init = {
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + .NAME = INT_MAX,
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +};
> +
> static __initdata bool debug;
> #define init_debug(...) \
> do { \
> @@ -307,6 +431,46 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> kfree(sep);
> }
>
> +static void __init lsm_init_hook_static_slot(struct static_slot *slots,
> + struct hlist_head *head,
> + int *first_slot_idx)
> +{
> + struct security_hook_list *pos;
> + struct static_slot *slot;
> + int slot_cnt;
> +
> + slot_cnt = 0;
> + hlist_for_each_entry_rcu(pos, head, list)
> + slot_cnt++;
> +
> + if (slot_cnt > SECURITY_STATIC_SLOT_COUNT)
> + panic("%s - No static hook slot remaining to add LSM hook.\n",
> + __func__);
> +
> + if (slot_cnt == 0) {
> + *first_slot_idx = INT_MAX;
> + return;
> + }
> +
> + *first_slot_idx = SECURITY_STATIC_SLOT_COUNT - slot_cnt;
> + slot = slots + *first_slot_idx;
> + hlist_for_each_entry_rcu(pos, head, list) {
> + __static_call_update(slot->key, slot->trampoline,
> + pos->hook.generic_func);
> + slot++;
> + }
> +}
> +
> +static void __init lsm_init_static_slots(void)
> +{
> +#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> + lsm_init_hook_static_slot(static_slots.NAME, \
> + &security_hook_heads.NAME, \
> + &base_slot_idx.NAME);
> +#include <linux/lsm_hook_defs.h>
> +#undef LSM_HOOK
> +}
> +
> static void __init lsm_early_cred(struct cred *cred);
> static void __init lsm_early_task(struct task_struct *task);
>
> @@ -354,6 +518,7 @@ static void __init ordered_lsm_init(void)
> lsm_early_task(current);
> for (lsm = ordered_lsms; *lsm; lsm++)
> initialize_lsm(*lsm);
> + lsm_init_static_slots();
>
> kfree(ordered_lsms);
> }
> @@ -374,6 +539,7 @@ int __init early_security_init(void)
> prepare_lsm(lsm);
> initialize_lsm(lsm);
> }
> + lsm_init_static_slots();
>
> return 0;
> }
> @@ -696,27 +862,36 @@ static void __init lsm_early_task(struct task_struct *task)
> * call_int_hook:
> * This is a hook that returns a value.
> */
> -
> -#define call_void_hook(FUNC, ...) \
> - do { \
> - struct security_hook_list *P; \
> - \
> - hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
> - P->hook.FUNC(__VA_ARGS__); \
> - } while (0)
> -
> -#define call_int_hook(FUNC, IRC, ...) ({ \
> - int RC = IRC; \
> - do { \
> - struct security_hook_list *P; \
> - \
> - hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
> - RC = P->hook.FUNC(__VA_ARGS__); \
> - if (RC != 0) \
> - break; \
> - } \
> - } while (0); \
> - RC; \
> +#define __CASE_CALL_STATIC_VOID(NUM, HOOK, ...) \
> + case NUM: \
> + static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
> + fallthrough;
> +
> +#define call_void_hook(FUNC, ...) do { \
> + switch (base_slot_idx.FUNC) { \
> + SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_VOID, \
> + FUNC, __VA_ARGS__) \
> + default : \
> + break; \
> + } \
> +} while (0)
> +
> +#define __CASE_CALL_STATIC_INT(NUM, R, HOOK, ...) \
> + case NUM: \
> + R = static_call(STATIC_SLOT(HOOK, NUM))(__VA_ARGS__); \
> + if (R != 0) \
> + break; \
> + fallthrough;
> +
> +#define call_int_hook(FUNC, IRC, ...) ({ \
> + int RC = IRC; \
> + switch (base_slot_idx.FUNC) { \
> + SECURITY_FOREACH_STATIC_SLOT(__CASE_CALL_STATIC_INT, \
> + RC, FUNC, __VA_ARGS__) \
> + default : \
> + break; \
> + } \
> + RC; \
> })
>
> /* Security operations */
^ permalink raw reply
* [net-next PATCH] netlabel: fix problems with mapping removal
From: Paul Moore @ 2020-08-21 1:46 UTC (permalink / raw)
To: netdev; +Cc: selinux, linux-security-module, Stephen Smalley
This patch fixes two main problems seen when removing NetLabel
mappings: memory leaks and potentially extra audit noise.
The memory leaks are caused by not properly free'ing the mapping's
address selector struct when free'ing the entire entry as well as
not properly cleaning up a temporary mapping entry when adding new
address selectors to an existing entry. This patch fixes both these
problems such that kmemleak reports no NetLabel associated leaks
after running the SELinux test suite.
The potentially extra audit noise was caused by the auditing code in
netlbl_domhsh_remove_entry() being called regardless of the entry's
validity. If another thread had already marked the entry as invalid,
but not removed/free'd it from the list of mappings, then it was
possible that an additional mapping removal audit record would be
generated. This patch fixes this by returning early from the removal
function when the entry was previously marked invalid. This change
also had the side benefit of improving the code by decreasing the
indentation level of large chunk of code by one (accounting for most
of the diffstat).
Reported-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
net/netlabel/netlabel_domainhash.c | 59 ++++++++++++++++++------------------
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/net/netlabel/netlabel_domainhash.c b/net/netlabel/netlabel_domainhash.c
index d07de2c0fbc7..f73a8382c275 100644
--- a/net/netlabel/netlabel_domainhash.c
+++ b/net/netlabel/netlabel_domainhash.c
@@ -85,6 +85,7 @@ static void netlbl_domhsh_free_entry(struct rcu_head *entry)
kfree(netlbl_domhsh_addr6_entry(iter6));
}
#endif /* IPv6 */
+ kfree(ptr->def.addrsel);
}
kfree(ptr->domain);
kfree(ptr);
@@ -537,6 +538,8 @@ int netlbl_domhsh_add(struct netlbl_dom_map *entry,
goto add_return;
}
#endif /* IPv6 */
+ /* cleanup the new entry since we've moved everything over */
+ netlbl_domhsh_free_entry(&entry->rcu);
} else
ret_val = -EINVAL;
@@ -580,6 +583,12 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
{
int ret_val = 0;
struct audit_buffer *audit_buf;
+ struct netlbl_af4list *iter4;
+ struct netlbl_domaddr4_map *map4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct netlbl_af6list *iter6;
+ struct netlbl_domaddr6_map *map6;
+#endif /* IPv6 */
if (entry == NULL)
return -ENOENT;
@@ -597,6 +606,9 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
ret_val = -ENOENT;
spin_unlock(&netlbl_domhsh_lock);
+ if (ret_val)
+ return ret_val;
+
audit_buf = netlbl_audit_start_common(AUDIT_MAC_MAP_DEL, audit_info);
if (audit_buf != NULL) {
audit_log_format(audit_buf,
@@ -606,40 +618,29 @@ int netlbl_domhsh_remove_entry(struct netlbl_dom_map *entry,
audit_log_end(audit_buf);
}
- if (ret_val == 0) {
- struct netlbl_af4list *iter4;
- struct netlbl_domaddr4_map *map4;
-#if IS_ENABLED(CONFIG_IPV6)
- struct netlbl_af6list *iter6;
- struct netlbl_domaddr6_map *map6;
-#endif /* IPv6 */
-
- switch (entry->def.type) {
- case NETLBL_NLTYPE_ADDRSELECT:
- netlbl_af4list_foreach_rcu(iter4,
- &entry->def.addrsel->list4) {
- map4 = netlbl_domhsh_addr4_entry(iter4);
- cipso_v4_doi_putdef(map4->def.cipso);
- }
+ switch (entry->def.type) {
+ case NETLBL_NLTYPE_ADDRSELECT:
+ netlbl_af4list_foreach_rcu(iter4, &entry->def.addrsel->list4) {
+ map4 = netlbl_domhsh_addr4_entry(iter4);
+ cipso_v4_doi_putdef(map4->def.cipso);
+ }
#if IS_ENABLED(CONFIG_IPV6)
- netlbl_af6list_foreach_rcu(iter6,
- &entry->def.addrsel->list6) {
- map6 = netlbl_domhsh_addr6_entry(iter6);
- calipso_doi_putdef(map6->def.calipso);
- }
+ netlbl_af6list_foreach_rcu(iter6, &entry->def.addrsel->list6) {
+ map6 = netlbl_domhsh_addr6_entry(iter6);
+ calipso_doi_putdef(map6->def.calipso);
+ }
#endif /* IPv6 */
- break;
- case NETLBL_NLTYPE_CIPSOV4:
- cipso_v4_doi_putdef(entry->def.cipso);
- break;
+ break;
+ case NETLBL_NLTYPE_CIPSOV4:
+ cipso_v4_doi_putdef(entry->def.cipso);
+ break;
#if IS_ENABLED(CONFIG_IPV6)
- case NETLBL_NLTYPE_CALIPSO:
- calipso_doi_putdef(entry->def.calipso);
- break;
+ case NETLBL_NLTYPE_CALIPSO:
+ calipso_doi_putdef(entry->def.calipso);
+ break;
#endif /* IPv6 */
- }
- call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
}
+ call_rcu(&entry->rcu, netlbl_domhsh_free_entry);
return ret_val;
}
^ permalink raw reply related
* Re: [PATCH v6 0/3] SELinux support for anonymous inodes and UFFD
From: Lokesh Gidra @ 2020-08-21 3:48 UTC (permalink / raw)
To: James Morris
Cc: Alexander Viro, Stephen Smalley, Casey Schaufler, Eric Biggers,
Serge E. Hallyn, Paul Moore, Eric Paris, Daniel Colascione,
Kees Cook, Eric W. Biederman, KP Singh, David Howells,
Thomas Cedeno, Anders Roxell, Sami Tolvanen, Matthew Garrett,
Aaron Goidel, Randy Dunlap, Joel Fernandes (Google), YueHaibing,
Christian Brauner, Alexei Starovoitov, Alexey Budankov,
Adrian Reber, Aleksa Sarai, Linux FS Devel, linux-kernel,
LSM List, SElinux list, Kalesh Singh, Calin Juravle,
Suren Baghdasaryan, Nick Kralevich, Jeffrey Vander Stoep,
kernel-team
In-Reply-To: <alpine.LRH.2.21.2008210433591.29407@namei.org>
On Thu, Aug 20, 2020 at 11:36 AM James Morris <jmorris@namei.org> wrote:
>
> On Fri, 7 Aug 2020, Lokesh Gidra wrote:
>
> > Userfaultfd in unprivileged contexts could be potentially very
> > useful. We'd like to harden userfaultfd to make such unprivileged use
> > less risky. This patch series allows SELinux to manage userfaultfd
> > file descriptors and in the future, other kinds of
> > anonymous-inode-based file descriptor. SELinux policy authors can
> > apply policy types to anonymous inodes by providing name-based
> > transition rules keyed off the anonymous inode internal name (
> > "[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
> > applying policy to the new SIDs thus produced.
>
> Can you expand more on why this would be useful, e.g. use-cases?
>
With SELinux managed userfaultfd file descriptors, an administrator
can control creation and movement of them. In particular, handling of
a userfaultfd descriptor by a different process is essentially a
ptrace access into the process, without any of the
corresponding security_ptrace_access_check() checks. For privacy, the
admin may want to deny such accesses,
which is possible with SELinux support.
I'll add this use case in the cover letter too in the next version.
^ permalink raw reply
* Re: [LTP] [PATCH 5.8 000/232] 5.8.3-rc1 review
From: Petr Vorel @ 2020-08-21 6:51 UTC (permalink / raw)
To: Eric Biggers
Cc: Naresh Kamboju, Ben Hutchings, Herbert Xu, Greg Kroah-Hartman,
LTP List, open list, lkft-triage, patches, Linus Torvalds,
linux-security-module, keyrings, linux- stable, Andrew Morton,
Shuah Khan, Guenter Roeck
In-Reply-To: <20200820182516.GA49496@sol.localdomain>
Hi all,
> On Thu, Aug 20, 2020 at 08:57:57PM +0530, Naresh Kamboju wrote:
> > On Thu, 20 Aug 2020 at 14:55, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > This is the start of the stable review cycle for the 5.8.3 release.
> > > There are 232 patches in this series, all will be posted as a response
> > > to this one. If anyone has any issues with these being applied, please
> > > let me know.
> > > Responses should be made by Sat, 22 Aug 2020 09:15:09 +0000.
> > > Anything received after that time might be too late.
> > > The whole patch series can be found in one patch at:
> > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.3-rc1.gz
> > > or in the git tree and branch at:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.8.y
> > > and the diffstat can be found below.
> > > thanks,
> > > greg k-h
> > > Herbert Xu <herbert@gondor.apana.org.au>
> > > crypto: af_alg - Fix regression on empty requests
> > Results from Linaro’s test farm.
> > Regressions detected.
> > ltp-crypto-tests:
> > * af_alg02
> > ltp-cve-tests:
> > * cve-2017-17805
> > af_alg02.c:52: BROK: Timed out while reading from request socket.
> > We are running the LTP 20200515 tag released test suite.
> > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
> > Summary
> > ------------------------------------------------------------------------
> > kernel: 5.8.3-rc1
> > git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > git branch: linux-5.8.y
> > git commit: 201fff807310ce10485bcff294d47be95f3769eb
> > git describe: v5.8.2-233-g201fff807310
> > Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-5.8-oe/build/v5.8.2-233-g201fff807310
> > Regressions (compared to build v5.8.2)
> > ------------------------------------------------------------------------
> > x15:
> > ltp-crypto-tests:
> > * af_alg02
> > ltp-cve-tests:
> > * cve-2017-17805
> Looks like this test is still "broken" because it assumes behavior that isn't
> clearly specified, as previously discussed at
> https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au.
> I sent out LTP patches to fix it:
> https://lkml.kernel.org/linux-crypto/20200820181918.404758-1-ebiggers@kernel.org/T/#u
FYI fix for LTP merged.
Kind regards,
Petr
> - Eric
^ permalink raw reply
* Re: [PATCH 5.8 000/232] 5.8.3-rc1 review
From: Greg Kroah-Hartman @ 2020-08-21 11:15 UTC (permalink / raw)
To: Naresh Kamboju
Cc: open list, Shuah Khan, patches, lkft-triage, Ben Hutchings,
linux- stable, Andrew Morton, Linus Torvalds, Guenter Roeck,
Herbert Xu, LTP List, linux-security-module, keyrings,
Eric Biggers
In-Reply-To: <CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com>
On Thu, Aug 20, 2020 at 08:57:57PM +0530, Naresh Kamboju wrote:
> On Thu, 20 Aug 2020 at 14:55, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > This is the start of the stable review cycle for the 5.8.3 release.
> > There are 232 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Sat, 22 Aug 2020 09:15:09 +0000.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.3-rc1.gz
> > or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.8.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
>
> > Herbert Xu <herbert@gondor.apana.org.au>
> > crypto: af_alg - Fix regression on empty requests
>
> Results from Linaro’s test farm.
> Regressions detected.
>
> ltp-crypto-tests:
> * af_alg02
> ltp-cve-tests:
> * cve-2017-17805
>
> af_alg02.c:52: BROK: Timed out while reading from request socket.
> We are running the LTP 20200515 tag released test suite.
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
Looks like the crypto tests are now fixed :)
Anyway, thanks for testing all of these and letting me know.
greg k-h
^ permalink raw reply
* Re: file metadata via fs API
From: Miklos Szeredi @ 2020-08-21 13:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Steven Whitehouse, David Howells, linux-fsdevel, Al Viro,
Karel Zak, Jeff Layton, Miklos Szeredi, Nicolas Dichtel,
Christian Brauner, Lennart Poettering, Linux API, Ian Kent, LSM,
Linux Kernel Mailing List
In-Reply-To: <CAHk-=wiUcfgC1PdbS_4mfAj2+VTacOwD_uUu6krSxjpvh42T7A@mail.gmail.com>
On Tue, Aug 18, 2020 at 10:53 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Basically, I think a rough rule of thumb can and should be:
>
> - stuff that the VFS knows about natively and fully is clearly pretty
> mount-agnostic and generic, and can be represented in whatever
> extended "struct statfs_x" directly.
>
> - anything that is variable-format and per-fs should be expressed in
> the ASCII buffer
>
> Look at our fancy new fs_context - that's pretty much what it does
> even inside the kernel. Sure, we have "binary" fields there for core
> basic information ("struct dentry *root", but also things like flags
> with MNT_NOSUID), but the configuration stuff is ASCII that the
> filesystem can parse itself.
>
> Exactly because some things are very much specific to some
> filesystems, not generic things.
>
> So we fundamentally already have a mix of "standard FS data" and
> "filesystem-specific options", and it's already basically split that
> way: binary flag fields for the generic stuff, and ASCII text for the
> odd options.
Okay.
Something else: do we want a separate statmount(2) or is it okay to
mix per-mount and per-sb attributes in the same syscall?
/proc/mounts concatenates mount and sb options (since it copies the
/etc/mtab format)
/proc/self/mountinfo separates per-mount and per-sb data into
different fields at least, but the fields themselves are mixed
If we are introducing completely new interfaces, I think it would make
sense to separate per-mount and per-sb attributes somehow. Atomicity
arguments don't apply since they have separate locking. And we
already have separate interfaces for configuring them...
Thanks,
Miklos
^ permalink raw reply
* RE: [RFC PATCH 00/30] ima: Introduce IMA namespace
From: Krzysztof Struczynski @ 2020-08-21 15:13 UTC (permalink / raw)
To: James Bottomley, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org,
containers@lists.linux-foundation.org,
linux-security-module@vger.kernel.org
Cc: zohar@linux.ibm.com, stefanb@linux.vnet.ibm.com,
sunyuqiong1988@gmail.com, mkayaalp@cs.binghamton.edu,
dmitry.kasatkin@gmail.com, serge@hallyn.com, jmorris@namei.org,
christian@brauner.io, Silviu Vlasceanu, Roberto Sassu
In-Reply-To: <1597767571.3898.15.camel@HansenPartnership.com>
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> On Tue, 2020-08-18 at 17:20 +0200, krzysztof.struczynski@huawei.com
> wrote:
> > The measurement list remains global, with the assumption that there
> > is only one TPM in the system. Each IMA namespace has a unique ID,
> > that allows to track measurements per IMA namespace. Processes in one
> > namespace, have access only to the measurements from that namespace.
> > The exception is made for the initial IMA namespace, whose processes
> > have access to all entries.
>
> So I think this can work in the use case where the system owner is
> responsible for doing the logging and attestation and the tenants just
> trust the owner without requiring an attestation. However, in a multi-
> tenant system you need a way for the attestation to be per-container
> (because the combined list of who executed what would be a security
> leak between tenants). Since we can't virtualise the PCRs without
> introducing a vtpm this is going to require a vtpm infrastructure like
> that used for virtual machines and then we can do IMA logging per
> container.
I agree and wonder if we should decouple the attestation trust model,
which depends on the specific use case (e.g. multi/single tenant,
public/private cloud), from the IMA logic of linking the measurements to
the container. Indeed, attestation from within the container might require
anchoring to a vTPM/vPCR and the current measurement tagging mechanism can
support several ways of anchoring them to a (virtual) root of trust.
> I don't think the above has to be in your first patch set, we just have
> to have an idea of how it could be done to show that nothing in this
> patch set precludes a follow on from doing this.
Given that virtualizing trust anchors seems like a separate problem in
which industry consensus is not easy to reach for all use cases, an
anchoring mechanism should probably be a separate IMA feature.
>
> James
^ permalink raw reply
* RE: [RFC PATCH 00/30] ima: Introduce IMA namespace
From: Krzysztof Struczynski @ 2020-08-21 15:18 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
containers@lists.linux-foundation.org,
linux-security-module@vger.kernel.org, zohar@linux.ibm.com,
stefanb@linux.vnet.ibm.com, sunyuqiong1988@gmail.com,
mkayaalp@cs.binghamton.edu, dmitry.kasatkin@gmail.com,
serge@hallyn.com, jmorris@namei.org, christian@brauner.io,
Silviu Vlasceanu, Roberto Sassu
In-Reply-To: <20200818155350.oy3axodt3vj5k7ij@wittgenstein>
> From: Christian Brauner [mailto:christian.brauner@ubuntu.com]
> On Tue, Aug 18, 2020 at 05:20:07PM +0200, krzysztof.struczynski@huawei.com
> wrote:
> > From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
> >
> > IMA has not been designed to work with containers. It handles every
> > process in the same way, and it cannot distinguish if a process belongs to
> > a container or not.
> >
> > Containers use namespaces to make it appear to the processes in the
> > containers that they have their own isolated instance of the global
> > resource. For IMA as well, it is desirable to let processes in the
> > containers have IMA functionality independent from other containers:
> > separate policy rules, measurement list, additional appraisal keys to
> > verify the container image, separate audit logs.
> >
> > As previous work done in this area, this patch series introduces the IMA
> > namespace, which is a separate instance of IMA to handle a subset of
> > processes that belong to a container.
> >
> > The IMA namespace is created using clone3() or unshare() system calls. It
> > is important to configure the namespace before any process appears in it,
> > so that the new policy rules apply to the very first process in the
> > namespace. To achieve that, the intermediate namespace
> ima_ns_for_children
> > is used. It stores the configuration and becomes active on the next fork
> > or when the first process enters it using the setns() system call. The
> > similar process is used for the time namespace.
> >
> > The IMA namespace can be configured using the new securityfs directory
> > entries that allow the user to set the policy rules, x509 certificate for
> > appraisal and pass IMA configuration parameters normally included in the
> > kernel command line parameters. It is intended to extend the clone_args to
> > allow configuration from clone3() syscall.
>
> Not to be the downer right away but just as an fyi, if this patchset
> makes it, clone3() will not allow to be extended with any real
> second-level pointers. That will see a hard NAK from me and several
> other maintainers.
Ok, that's a good point. It can be done without the second-level pointers
but if that's not desirable then IMA namespace creation via a direct
clone3() call can be removed. It will make the process less flexible but
it will still work with unshare() and clone3() or unshare() and setns()
calls.
>
> Christian
^ permalink raw reply
* RE: [RFC PATCH 00/30] ima: Introduce IMA namespace
From: Krzysztof Struczynski @ 2020-08-21 15:37 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
containers@lists.linux-foundation.org,
linux-security-module@vger.kernel.org, zohar@linux.ibm.com,
stefanb@linux.vnet.ibm.com, sunyuqiong1988@gmail.com,
mkayaalp@cs.binghamton.edu, dmitry.kasatkin@gmail.com,
serge@hallyn.com, jmorris@namei.org, christian@brauner.io,
Silviu Vlasceanu, Roberto Sassu, ebiederm@xmission.com,
viro@zeniv.linux.org.uk, torvalds@linux-foundation.org,
luto@amacapital.net, jannh@google.com
In-Reply-To: <20200818164943.va3um7toztazcfud@wittgenstein>
> From: Christian Brauner [mailto:christian.brauner@ubuntu.com]
> On Tue, Aug 18, 2020 at 05:20:07PM +0200, krzysztof.struczynski@huawei.com
> wrote:
> > From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
> >
> > IMA has not been designed to work with containers. It handles every
> > process in the same way, and it cannot distinguish if a process belongs to
> > a container or not.
> >
> > Containers use namespaces to make it appear to the processes in the
> > containers that they have their own isolated instance of the global
> > resource. For IMA as well, it is desirable to let processes in the
>
> IMA is brought up on a regular basis with "we want to have this" for
> years and then non-one seems to really care enough.
>
> I'm highly skeptical of the value of ~2500 lines of code even if it
> includes a bunch of namespace boilerplate. It's yet another namespace,
> and yet another security framework.
> Why does IMA need to be a separate namespace? Keyrings are tied to user
> namespaces why can't IMA be? I believe Eric has even pointed that out
> before.
The user namespace has its well defined purpose to isolate
security-related identifiers and attributes, particularly UIDs and GIDs.
I think that IMA goals are different.
A user may want to isolate e.g. UIDs but not to create a separate IML or
define the new IMA policies. On the other hand, especially in the
single-tenant environment, the user may want to have a per container IML,
but no UID/GID mapping is required. IMA policy defines subject-based
rules (uid, euid, subj_*, ...), but also object-based rules.
IMA has to be pre-configured, e.g. all actions of the process have to be
appraised/measured/audited according to the pre-defined policy, appraisal
key has to be available before the process is created, etc. If IMA is tied
to the user namespace, when is a good moment to do it?
What's the argument against adding a new namespace?
>
> Eric, thoughts?
>
> Christian
^ permalink raw reply
* Re: [PATCH 5.8 000/232] 5.8.3-rc1 review
From: Naresh Kamboju @ 2020-08-21 16:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: open list, Shuah Khan, patches, lkft-triage, Ben Hutchings,
linux- stable, Andrew Morton, Linus Torvalds, Guenter Roeck,
Herbert Xu, LTP List, linux-security-module, keyrings,
Eric Biggers
In-Reply-To: <20200821111535.GC2222852@kroah.com>
On Fri, 21 Aug 2020 at 16:45, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 20, 2020 at 08:57:57PM +0530, Naresh Kamboju wrote:
> > On Thu, 20 Aug 2020 at 14:55, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > This is the start of the stable review cycle for the 5.8.3 release.
> > > There are 232 patches in this series, all will be posted as a response
> > > to this one. If anyone has any issues with these being applied, please
> > > let me know.
> > >
> > > Responses should be made by Sat, 22 Aug 2020 09:15:09 +0000.
> > > Anything received after that time might be too late.
> > >
> > > The whole patch series can be found in one patch at:
> > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.3-rc1.gz
> > > or in the git tree and branch at:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.8.y
> > > and the diffstat can be found below.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > > Herbert Xu <herbert@gondor.apana.org.au>
> > > crypto: af_alg - Fix regression on empty requests
> >
> > Results from Linaro’s test farm.
> > Regressions detected.
> >
> > ltp-crypto-tests:
> > * af_alg02
> > ltp-cve-tests:
> > * cve-2017-17805
> >
> > af_alg02.c:52: BROK: Timed out while reading from request socket.
> > We are running the LTP 20200515 tag released test suite.
> > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/crypto/af_alg02.c
>
> Looks like the crypto tests are now fixed :)
>
> Anyway, thanks for testing all of these and letting me know.
Apart from the reported LTP crypto test case problem all other results
look good to me.
Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.
Summary
------------------------------------------------------------------------
kernel: 5.8.3
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-5.8.y
git commit: a1101e94767e2d5da5bcbee12573d96a1c8be5bb
git describe: v5.8.3
Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-5.8-oe/build/v5.8.3
No regressions (compared to build v5.8.2)
No fixes (compared to build v5.8.2)
Ran 30256 total tests in the following environments and test suites.
Environments
--------------
- dragonboard-410c
- hi6220-hikey
- i386
- juno-r2
- juno-r2-compat
- juno-r2-kasan
- nxp-ls2088
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15
- x86
- x86-kasan
Test Suites
-----------
* build
* linux-log-parser
* ltp-commands-tests
* ltp-containers-tests
* ltp-dio-tests
* ltp-fs-tests
* ltp-io-tests
* ltp-math-tests
* ltp-tracing-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-nptl-tests
* ltp-pty-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* v4l2-compliance
* libhugetlbfs
* ltp-cap_bounds-tests
* ltp-controllers-tests
* ltp-cpuhotplug-tests
* ltp-crypto-tests
* ltp-hugetlb-tests
* ltp-ipc-tests
* ltp-mm-tests
* ltp-open-posix-tests
* ltp-sched-tests
* network-basic-tests
* igt-gpu-tools
--
Linaro LKFT
https://lkft.linaro.org
^ permalink raw reply
* [PATCH v2 0/3] IMA: Infrastructure for measurement of critical kernel data
From: Tushar Sugandhi @ 2020-08-21 18:21 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey, agk, snitzer, gmazyland
Cc: tyhicks, sashal, jmorris, nramas, linux-integrity, selinux,
linux-security-module, linux-kernel, dm-devel
There are several kernel components that contain critical data which if
accidentally or maliciously altered, can compromise the security of the
kernel. Example of such components would include LSMs like SELinux, or
AppArmor; or device-mapper targets like dm-crypt, dm-verity etc.
Many of these components do not use the capabilities provided by kernel
integrity subsystem (IMA), and thus they don't use the benefits of
extended TPM PCR quotes and ultimately the benefits of remote attestation.
This series bridges this gap, so that potential kernel components that
contain data critical to the security of the kernel could take advantage
of IMA's measuring and quoting abilities - thus ultimately enabling
remote attestation for their specific data.
System administrators may want to pick and choose which kernel
components they would want to enable for measurements, quoting, and
remote attestation. To enable that, a new IMA policy is introduced.
And lastly, the functionality is exposed through a function
ima_measure_critical_data(). The functionality is generic enough to
measure the data of any kernel component at run-time.
This series is based on the following repo/branch:
repo: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
branch: next-integrity
This series also has a dependency on the following patch series:
https://patchwork.kernel.org/patch/11709527/
Change Log v2:
- Reverted the unnecessary indentations in existing #define.
- Updated the description to replace the word 'enlightened' with
'supported'.
- Reverted the unnecessary rename of attribute size to buf_len.
- Introduced a boolean parameter measure_buf_hash as per community
feedback to support measuring hash of the buffer, instead of the
buffer itself.
Tushar Sugandhi (3):
IMA: generalize keyring specific measurement constructs
IMA: add policy to support measuring critical data from kernel
components
IMA: define IMA hook to measure critical data from kernel components
Documentation/ABI/testing/ima_policy | 6 +-
include/linux/ima.h | 11 ++
security/integrity/ima/ima.h | 12 ++-
security/integrity/ima/ima_api.c | 8 +-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_asymmetric_keys.c | 2 +-
security/integrity/ima/ima_main.c | 69 +++++++++++--
security/integrity/ima/ima_policy.c | 100 +++++++++++++++----
security/integrity/ima/ima_queue_keys.c | 3 +-
9 files changed, 167 insertions(+), 46 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2 1/3] IMA: generalize keyring specific measurement constructs
From: Tushar Sugandhi @ 2020-08-21 18:21 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey, agk, snitzer, gmazyland
Cc: tyhicks, sashal, jmorris, nramas, linux-integrity, selinux,
linux-security-module, linux-kernel, dm-devel
In-Reply-To: <20200821182107.5328-1-tusharsu@linux.microsoft.com>
IMA functions such as ima_match_keyring(), process_buffer_measurement(),
ima_match_policy() etc. handle data specific to keyrings. Currently,
these constructs are not generic to handle any func specific data.
This makes it harder to extend without code duplication.
Refactor the keyring specific measurement constructs to be generic and
reusable in other measurement scenarios.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
security/integrity/ima/ima.h | 6 ++---
security/integrity/ima/ima_api.c | 6 ++---
security/integrity/ima/ima_main.c | 6 ++---
security/integrity/ima/ima_policy.c | 42 ++++++++++++++++-------------
4 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 38043074ce5e..8875085db689 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -255,7 +255,7 @@ static inline void ima_process_queued_keys(void) {}
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
- const char *keyring);
+ const char *func_data);
int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
@@ -267,7 +267,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct ima_template_desc *template_desc);
void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
- int pcr, const char *keyring);
+ int pcr, const char *func_data);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
int ima_alloc_init_template(struct ima_event_data *event_data,
@@ -283,7 +283,7 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
enum ima_hooks func, int mask, int flags, int *pcr,
struct ima_template_desc **template_desc,
- const char *keyring);
+ const char *func_data);
void ima_init_policy(void);
void ima_update_policy(void);
void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..af218babd198 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -170,7 +170,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* @func: caller identifier
* @pcr: pointer filled in if matched measure policy sets pcr=
* @template_desc: pointer filled in if matched measure policy sets template=
- * @keyring: keyring name used to determine the action
+ * @func_data: private data specific to @func, can be NULL.
*
* The policy is defined in terms of keypairs:
* subj=, obj=, type=, func=, mask=, fsmagic=
@@ -186,14 +186,14 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
- const char *keyring)
+ const char *func_data)
{
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
flags &= ima_policy_flag;
return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
- template_desc, keyring);
+ template_desc, func_data);
}
/*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8a91711ca79b..c870fd6d2f83 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -732,13 +732,13 @@ int ima_load_data(enum kernel_load_data_id id)
* @eventname: event name to be used for the buffer entry.
* @func: IMA hook
* @pcr: pcr to extend the measurement
- * @keyring: keyring name to determine the action to be performed
+ * @func_data: private data specific to @func, can be NULL.
*
* Based on policy, the buffer is measured into the ima log.
*/
void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
- int pcr, const char *keyring)
+ int pcr, const char *func_data)
{
int ret = 0;
const char *audit_cause = "ENOMEM";
@@ -770,7 +770,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
if (func) {
security_task_getsecid(current, &secid);
action = ima_get_action(inode, current_cred(), secid, 0, func,
- &pcr, &template, keyring);
+ &pcr, &template, func_data);
if (!(action & IMA_MEASURE))
return;
}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fe1df373c113..8866e84d0062 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -451,15 +451,21 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
}
/**
- * ima_match_keyring - determine whether the keyring matches the measure rule
- * @rule: a pointer to a rule
- * @keyring: name of the keyring to match against the measure rule
+ * ima_match_rule_data - determine whether the given func_data matches
+ * the measure rule data
+ * @rule: IMA policy rule
+ * @opt_list: rule data to match func_data against
+ * @func_data: data to match against the measure rule data
+ * @allow_empty_opt_list: If true matches all func_data
* @cred: a pointer to a credentials structure for user validation
*
- * Returns true if keyring matches one in the rule, false otherwise.
+ * Returns true if func_data matches one in the rule, false otherwise.
*/
-static bool ima_match_keyring(struct ima_rule_entry *rule,
- const char *keyring, const struct cred *cred)
+static bool ima_match_rule_data(struct ima_rule_entry *rule,
+ const struct ima_rule_opt_list *opt_list,
+ const char *func_data,
+ bool allow_empty_opt_list,
+ const struct cred *cred)
{
bool matched = false;
size_t i;
@@ -467,14 +473,14 @@ static bool ima_match_keyring(struct ima_rule_entry *rule,
if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid))
return false;
- if (!rule->keyrings)
- return true;
+ if (!opt_list)
+ return allow_empty_opt_list;
- if (!keyring)
+ if (!func_data)
return false;
- for (i = 0; i < rule->keyrings->count; i++) {
- if (!strcmp(rule->keyrings->items[i], keyring)) {
+ for (i = 0; i < opt_list->count; i++) {
+ if (!strcmp(opt_list->items[i], func_data)) {
matched = true;
break;
}
@@ -491,20 +497,21 @@ static bool ima_match_keyring(struct ima_rule_entry *rule,
* @secid: the secid of the task to be validated
* @func: LIM hook identifier
* @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
- * @keyring: keyring name to check in policy for KEY_CHECK func
+ * @func_data: private data specific to @func, can be NULL.
*
* Returns true on rule match, false on failure.
*/
static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
const struct cred *cred, u32 secid,
enum ima_hooks func, int mask,
- const char *keyring)
+ const char *func_data)
{
int i;
if (func == KEY_CHECK) {
return (rule->flags & IMA_FUNC) && (rule->func == func) &&
- ima_match_keyring(rule, keyring, cred);
+ ima_match_rule_data(rule, rule->keyrings, func_data,
+ true, cred);
}
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
@@ -608,8 +615,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
* @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
* @pcr: set the pcr to extend
* @template_desc: the template that should be used for this rule
- * @keyring: the keyring name, if given, to be used to check in the policy.
- * keyring can be NULL if func is anything other than KEY_CHECK.
+ * @func_data: private data specific to @func, can be NULL.
*
* Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
* conditions.
@@ -621,7 +627,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
enum ima_hooks func, int mask, int flags, int *pcr,
struct ima_template_desc **template_desc,
- const char *keyring)
+ const char *func_data)
{
struct ima_rule_entry *entry;
int action = 0, actmask = flags | (flags << 1);
@@ -636,7 +642,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
continue;
if (!ima_match_rules(entry, inode, cred, secid, func, mask,
- keyring))
+ func_data))
continue;
action |= entry->flags & IMA_ACTION_FLAGS;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/3] IMA: add policy to support measuring critical data from kernel components
From: Tushar Sugandhi @ 2020-08-21 18:21 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey, agk, snitzer, gmazyland
Cc: tyhicks, sashal, jmorris, nramas, linux-integrity, selinux,
linux-security-module, linux-kernel, dm-devel
In-Reply-To: <20200821182107.5328-1-tusharsu@linux.microsoft.com>
There would be several candidate kernel components suitable for IMA
measurement. Not all of them would have support for IMA measurement.
Also, system administrators may not want to measure data for all of
them, even when they support IMA measurement. An IMA policy specific
to various kernel components is needed to measure their respective
critical data.
Add a new IMA policy CRITICAL_DATA+data_sources to support measuring
various critical kernel components. This policy would enable the
system administrators to limit the measurement to the components,
if the components support IMA measurement.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
Documentation/ABI/testing/ima_policy | 6 ++-
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_policy.c | 62 +++++++++++++++++++++++++---
4 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index cd572912c593..a0dd0f108555 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,7 +29,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
- [KEXEC_CMDLINE] [KEY_CHECK]
+ [KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
@@ -125,3 +125,7 @@ Description:
keys added to .builtin_trusted_keys or .ima keyring:
measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
+ Example of measure rule using CRITICAL_DATA to measure critical data
+
+ measure func=CRITICAL_DATA data_sources=selinux|apparmor|dm-crypt
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 8875085db689..0f4209a92bfb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -200,6 +200,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
hook(POLICY_CHECK, policy) \
hook(KEXEC_CMDLINE, kexec_cmdline) \
hook(KEY_CHECK, key) \
+ hook(CRITICAL_DATA, critical_data) \
hook(MAX_CHECK, none)
#define __ima_hook_enumify(ENUM, str) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index af218babd198..9917e1730cb6 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -176,7 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
- * | KEXEC_CMDLINE | KEY_CHECK
+ * | KEXEC_CMDLINE | KEY_CHECK | CRITICAL_DATA
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8866e84d0062..7b649095ac7a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -33,6 +33,7 @@
#define IMA_PCR 0x0100
#define IMA_FSNAME 0x0200
#define IMA_KEYRINGS 0x0400
+#define IMA_DATA_SOURCES 0x0800
#define UNKNOWN 0
#define MEASURE 0x0001 /* same as IMA_MEASURE */
@@ -84,6 +85,7 @@ struct ima_rule_entry {
} lsm[MAX_LSM_RULES];
char *fsname;
struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
+ struct ima_rule_opt_list *data_sources; /* Measure data from these sources */
struct ima_template_desc *template;
};
@@ -508,14 +510,23 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
- if (func == KEY_CHECK) {
- return (rule->flags & IMA_FUNC) && (rule->func == func) &&
- ima_match_rule_data(rule, rule->keyrings, func_data,
- true, cred);
- }
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
+
+ switch (func) {
+ case KEY_CHECK:
+ return ((rule->func == func) &&
+ ima_match_rule_data(rule, rule->keyrings,
+ func_data, true, cred));
+ case CRITICAL_DATA:
+ return ((rule->func == func) &&
+ ima_match_rule_data(rule, rule->data_sources,
+ func_data, false, cred));
+ default:
+ break;
+ }
+
if ((rule->flags & IMA_MASK) &&
(rule->mask != mask && func != POST_SETATTR))
return false;
@@ -911,7 +922,7 @@ enum {
Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
Opt_appraise_type, Opt_appraise_flag,
Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
- Opt_err
+ Opt_data_sources, Opt_err
};
static const match_table_t policy_tokens = {
@@ -948,6 +959,7 @@ static const match_table_t policy_tokens = {
{Opt_pcr, "pcr=%s"},
{Opt_template, "template=%s"},
{Opt_keyrings, "keyrings=%s"},
+ {Opt_data_sources, "data_sources=%s"},
{Opt_err, NULL}
};
@@ -1110,6 +1122,19 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
if (ima_rule_contains_lsm_cond(entry))
return false;
+ break;
+ case CRITICAL_DATA:
+ if (entry->action & ~(MEASURE | DONT_MEASURE))
+ return false;
+
+ if (!(entry->flags & IMA_DATA_SOURCES) ||
+ (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
+ IMA_DATA_SOURCES)))
+ return false;
+
+ if (ima_rule_contains_lsm_cond(entry))
+ return false;
+
break;
default:
return false;
@@ -1242,6 +1267,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
else if (IS_ENABLED(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) &&
strcmp(args[0].from, "KEY_CHECK") == 0)
entry->func = KEY_CHECK;
+ else if (strcmp(args[0].from, "CRITICAL_DATA") == 0)
+ entry->func = CRITICAL_DATA;
else
result = -EINVAL;
if (!result)
@@ -1312,6 +1339,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->flags |= IMA_KEYRINGS;
break;
+ case Opt_data_sources:
+ ima_log_string(ab, "data_sources", args[0].from);
+
+ if (entry->data_sources) {
+ result = -EINVAL;
+ break;
+ }
+
+ entry->data_sources = ima_alloc_rule_opt_list(args);
+ if (IS_ERR(entry->data_sources)) {
+ result = PTR_ERR(entry->data_sources);
+ entry->data_sources = NULL;
+ break;
+ }
+
+ entry->flags |= IMA_DATA_SOURCES;
+ break;
case Opt_fsuuid:
ima_log_string(ab, "fsuuid", args[0].from);
@@ -1692,6 +1736,12 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, " ");
}
+ if (entry->flags & IMA_DATA_SOURCES) {
+ seq_puts(m, "data_sources=");
+ ima_show_rule_opt_list(m, entry->data_sources);
+ seq_puts(m, " ");
+ }
+
if (entry->flags & IMA_PCR) {
snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr);
seq_printf(m, pt(Opt_pcr), tbuf);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/3] IMA: define IMA hook to measure critical data from kernel components
From: Tushar Sugandhi @ 2020-08-21 18:21 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey, agk, snitzer, gmazyland
Cc: tyhicks, sashal, jmorris, nramas, linux-integrity, selinux,
linux-security-module, linux-kernel, dm-devel
In-Reply-To: <20200821182107.5328-1-tusharsu@linux.microsoft.com>
Currently, IMA does not provide a generic function to kernel components
to measure their data. A generic function provided by IMA would
enable various parts of the kernel with easier and faster on-boarding to
use IMA infrastructure, would avoid code duplication, and consistent
usage of IMA policy CRITICAL_DATA+data_sources across the kernel.
Define a generic IMA function ima_measure_critical_data() to measure
data from various kernel components. Limit the measurement to the
components that are specified in the IMA policy -
CRITICAL_DATA+data_sources.
Update process_buffer_measurement() to return the status code of the
operation.
Introduce a boolean parameter measure_buf_hash to support measuring
hash of a buffer, instead of the buffer itself. This is useful when
the buffer being measured is too large, which may result in bloated
IMA logs.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
include/linux/ima.h | 11 ++++
security/integrity/ima/ima.h | 7 ++-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_asymmetric_keys.c | 2 +-
security/integrity/ima/ima_main.c | 65 +++++++++++++++++---
security/integrity/ima/ima_queue_keys.c | 3 +-
6 files changed, 75 insertions(+), 15 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index d15100de6cdd..136fc02580db 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,10 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
extern void ima_post_path_mknod(struct dentry *dentry);
extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size);
+extern int ima_measure_critical_data(const char *event_name,
+ const char *event_data_source,
+ const void *buf, int buf_len,
+ bool measure_buf_hash);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -104,6 +108,13 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
}
static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {}
+static inline int ima_measure_critical_data(const char *event_name,
+ const char *event_data_source,
+ const void *buf, int buf_len,
+ bool measure_buf_hash)
+{
+ return -EOPNOTSUPP;
+}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 0f4209a92bfb..00b84052c8f1 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -266,9 +266,10 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
-void process_buffer_measurement(struct inode *inode, const void *buf, int size,
- const char *eventname, enum ima_hooks func,
- int pcr, const char *func_data);
+int process_buffer_measurement(struct inode *inode, const void *buf, int size,
+ const char *eventname, enum ima_hooks func,
+ int pcr, const char *func_data,
+ bool measure_buf_hash);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
int ima_alloc_init_template(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 372d16382960..20adffe5bf58 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -336,7 +336,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
process_buffer_measurement(NULL, digest, digestsize,
"blacklisted-hash", NONE,
- pcr, NULL);
+ pcr, NULL, false);
}
return rc;
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index 1c68c500c26f..a74095793936 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -60,5 +60,5 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
*/
process_buffer_measurement(NULL, payload, payload_len,
keyring->description, KEY_CHECK, 0,
- keyring->description);
+ keyring->description, false);
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c870fd6d2f83..a889bf40cb7e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -733,17 +733,21 @@ int ima_load_data(enum kernel_load_data_id id)
* @func: IMA hook
* @pcr: pcr to extend the measurement
* @func_data: private data specific to @func, can be NULL.
+ * @measure_buf_hash: if set to true - will measure hash of the buf,
+ * instead of buf
*
* Based on policy, the buffer is measured into the ima log.
*/
-void process_buffer_measurement(struct inode *inode, const void *buf, int size,
- const char *eventname, enum ima_hooks func,
- int pcr, const char *func_data)
+int process_buffer_measurement(struct inode *inode, const void *buf, int size,
+ const char *eventname, enum ima_hooks func,
+ int pcr, const char *func_data,
+ bool measure_buf_hash)
{
int ret = 0;
const char *audit_cause = "ENOMEM";
struct ima_template_entry *entry = NULL;
struct integrity_iint_cache iint = {};
+ struct integrity_iint_cache digest_iint = {};
struct ima_event_data event_data = {.iint = &iint,
.filename = eventname,
.buf = buf,
@@ -752,13 +756,13 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
struct {
struct ima_digest_data hdr;
char digest[IMA_MAX_DIGEST_SIZE];
- } hash = {};
+ } hash = {}, digest_hash = {};
int violation = 0;
int action = 0;
u32 secid;
if (!ima_policy_flag)
- return;
+ return 0;
/*
* Both LSM hooks and auxilary based buffer measurements are
@@ -772,7 +776,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
action = ima_get_action(inode, current_cred(), secid, 0, func,
&pcr, &template, func_data);
if (!(action & IMA_MEASURE))
- return;
+ return 0;
}
if (!pcr)
@@ -787,7 +791,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
pr_err("template %s init failed, result: %d\n",
(strlen(template->name) ?
template->name : template->fmt), ret);
- return;
+ return ret;
}
}
@@ -801,6 +805,24 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
goto out;
}
+ if (measure_buf_hash) {
+ digest_iint.ima_hash = &digest_hash.hdr;
+ digest_iint.ima_hash->algo = ima_hash_algo;
+ digest_iint.ima_hash->length = hash_digest_size[ima_hash_algo];
+
+ ret = ima_calc_buffer_hash(hash.hdr.digest,
+ iint.ima_hash->length,
+ digest_iint.ima_hash);
+ if (ret < 0) {
+ audit_cause = "digest_hashing_error";
+ goto out;
+ }
+
+ event_data.iint = &digest_iint;
+ event_data.buf = hash.hdr.digest;
+ event_data.buf_len = iint.ima_hash->length;
+ }
+
ret = ima_alloc_init_template(&event_data, &entry, template);
if (ret < 0) {
audit_cause = "alloc_entry";
@@ -819,7 +841,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
func_measure_str(func),
audit_cause, ret, 0, ret);
- return;
+ return ret;
}
/**
@@ -842,10 +864,35 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
return;
process_buffer_measurement(file_inode(f.file), buf, size,
- "kexec-cmdline", KEXEC_CMDLINE, 0, NULL);
+ "kexec-cmdline", KEXEC_CMDLINE, 0, NULL,
+ false);
fdput(f);
}
+/**
+ * ima_measure_critical_data - measure critical data
+ * @event_name: name for the given data
+ * @event_data_source: name of the event data source
+ * @buf: pointer to buffer containing data to measure
+ * @buf_len: length of buffer(in bytes)
+ * @measure_buf_hash: if set to true - will measure hash of the buf,
+ * instead of buf
+ *
+ * Buffers can only be measured, not appraised.
+ */
+int ima_measure_critical_data(const char *event_name,
+ const char *event_data_source,
+ const void *buf, int buf_len,
+ bool measure_buf_hash)
+{
+ if (!event_name || !event_data_source || !buf || !buf_len)
+ return -EINVAL;
+
+ return process_buffer_measurement(NULL, buf, buf_len, event_name,
+ CRITICAL_DATA, 0, event_data_source,
+ measure_buf_hash);
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index 69a8626a35c0..c2f2ad34f9b7 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -162,7 +162,8 @@ void ima_process_queued_keys(void)
entry->payload_len,
entry->keyring_name,
KEY_CHECK, 0,
- entry->keyring_name);
+ entry->keyring_name,
+ false);
list_del(&entry->list);
ima_free_key_entry(entry);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 01/11] evm: Execute evm_inode_init_security() only when the HMAC key is loaded
From: Mimi Zohar @ 2020-08-21 18:30 UTC (permalink / raw)
To: Roberto Sassu, mjg59
Cc: linux-integrity, linux-security-module, linux-kernel, stable
In-Reply-To: <20200618160133.937-1-roberto.sassu@huawei.com>
Hi Roberto,
Sorry for the delay in reviewing these patches. Missing from this
patch set is a cover letter with an explanation for grouping these
patches into a patch set, other than for convenience. In this case, it
would be along the lines that the original use case for EVM portable
and immutable keys support was for a few critical files, not combined
with an EVM encrypted key type. This patch set more fully integrates
the initial EVM portable and immutable signature support.
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
> evm_inode_init_security() requires the HMAC key to calculate the HMAC on
> initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded()
> check, the function continues even if the HMAC key is not loaded
> (evm_key_loaded() returns true also if EVM has been initialized only with a
> public key). If the HMAC key is not loaded, evm_inode_init_security()
> returns an error later when it calls evm_init_hmac().
>
> Thus, this patch replaces the evm_key_loaded() check with a check of the
> EVM_INIT_HMAC flag in evm_initialized, so that evm_inode_init_security()
> returns 0 instead of an error.
>
> Cc: stable@vger.kernel.org # 4.5.x
> Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> security/integrity/evm/evm_main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 0d36259b690d..744c105b48d1 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -521,7 +521,8 @@ int evm_inode_init_security(struct inode *inode,
> struct evm_xattr *xattr_data;
> int rc;
>
> - if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name))
> + if (!(evm_initialized & EVM_INIT_HMAC) ||
> + !evm_protected_xattr(lsm_xattr->name))
> return 0;
>
> xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox