2022-09-05 15:58:49

by Roberto Sassu

[permalink] [raw]
Subject: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

From: Roberto Sassu <[email protected]>

One of the desirable features in security is the ability to restrict import
of data to a given system based on data authenticity. If data import can be
restricted, it would be possible to enforce a system-wide policy based on
the signing keys the system owner trusts.

This feature is widely used in the kernel. For example, if the restriction
is enabled, kernel modules can be plugged in only if they are signed with a
key whose public part is in the primary or secondary keyring.

For eBPF, it can be useful as well. For example, it might be useful to
authenticate data an eBPF program makes security decisions on.

After a discussion in the eBPF mailing list, it was decided that the stated
goal should be accomplished by introducing four new kfuncs:
bpf_lookup_user_key() and bpf_lookup_system_key(), for retrieving a keyring
with keys trusted for signature verification, respectively from its serial
and from a pre-determined ID; bpf_key_put(), to release the reference
obtained with the former two kfuncs, bpf_verify_pkcs7_signature(), for
verifying PKCS#7 signatures.

Other than the key serial, bpf_lookup_user_key() also accepts key lookup
flags, that influence the behavior of the lookup. bpf_lookup_system_key()
accepts pre-determined IDs defined in include/linux/verification.h.

bpf_key_put() accepts the new bpf_key structure, introduced to tell whether
the other structure member, a key pointer, is valid or not. The reason is
that verify_pkcs7_signature() also accepts invalid pointers, set with the
pre-determined ID, to select a system-defined keyring. key_put() must be
called only for valid key pointers.

Since the two key lookup functions allocate memory and one increments a key
reference count, they must be used in conjunction with bpf_key_put(). The
latter must be called only if the lookup functions returned a non-NULL
pointer. The verifier denies the execution of eBPF programs that don't
respect this rule.

The two key lookup functions should be used in alternative, depending on
the use case. While bpf_lookup_user_key() provides great flexibility, it
seems suboptimal in terms of security guarantees, as even if the eBPF
program is assumed to be trusted, the serial used to obtain the key pointer
might come from untrusted user space not choosing one that the system
administrator approves to enforce a mandatory policy.

bpf_lookup_system_key() instead provides much stronger guarantees,
especially if the pre-determined ID is not passed by user space but is
hardcoded in the eBPF program, and that program is signed. In this case,
bpf_verify_pkcs7_signature() will always perform signature verification
with a key that the system administrator approves, i.e. the primary,
secondary or platform keyring.

Nevertheless, key permission checks need to be done accurately. Since
bpf_lookup_user_key() cannot determine how a key will be used by other
kfuncs, it has to defer the permission check to the actual kfunc using the
key. It does it by calling lookup_user_key() with KEY_DEFER_PERM_CHECK as
needed permission. Later, bpf_verify_pkcs7_signature(), if called,
completes the permission check by calling key_validate(). It does not need
to call key_task_permission() with permission KEY_NEED_SEARCH, as it is
already done elsewhere by the key subsystem. Future kfuncs using the
bpf_key structure need to implement the proper checks as well.

Finally, the last kfunc, bpf_verify_pkcs7_signature(), accepts the data and
signature to verify as eBPF dynamic pointers, to minimize the number of
kfunc parameters, and the keyring with keys for signature verification as a
bpf_key structure, returned by one of the two key lookup functions.

bpf_lookup_user_key() and bpf_verify_pkcs7_signature() can be called only
from sleepable programs, because of memory allocation and crypto
operations. For example, the lsm.s/bpf attach point is suitable,
fexit/array_map_update_elem is not.

The correctness of implementation of the new kfuncs and of their usage is
checked with the introduced tests.

The patch set includes a patch from another author (dependency) for sake of
completeness. It is organized as follows.

Patch 1 from KP Singh allows kfuncs to be used by LSM programs. Patch 2
splits is_dynptr_reg_valid_init() and introduces is_dynptr_type_expected(),
to know more precisely the cause of a negative result of a dynamic pointer
check. Patch 3 allows dynamic pointers to be used as kfunc parameters.
Patch 4 exports bpf_dynptr_get_size(), to obtain the real size of data
carried by a dynamic pointer. Patch 5 makes available for new eBPF kfuncs
and programs some key-related definitions. Patch 6 introduces the
bpf_lookup_*_key() and bpf_key_put() kfuncs. Patch 7 introduces the
bpf_verify_pkcs7_signature() kfunc. Patch 8 changes the testing kernel
configuration to compile everything as built-in. Finally, patches 9-12
introduce the tests.

Changelog

v15:
- Add kfunc_dynptr_param test to deny list for s390x

v14:
- Explain that is_dynptr_type_expected() will be useful also for BTF
(suggested by Joanne)
- Rename KEY_LOOKUP_FLAGS_ALL to KEY_LOOKUP_ALL (suggested by Jarkko)
- Swap declaration of spi and dynptr_type in is_dynptr_type_expected()
(suggested by Joanne)
- Reimplement kfunc dynptr tests with a regular eBPF program instead of
executing them with test_verifier (suggested by Joanne)
- Make key lookup flags as enum so that they are automatically exported
through BTF (suggested by Alexei)

v13:
- Split is_dynptr_reg_valid_init() and introduce is_dynptr_type_expected()
to see if the dynamic pointer type passed as argument to a kfunc is
supported (suggested by Kumar)
- Add forward declaration of struct key in include/linux/bpf.h (suggested
by Song)
- Declare mask for key lookup flags, remove key_lookup_flags_check()
(suggested by Jarkko and KP)
- Allow only certain dynamic pointer types (currently, local) to be passed
as argument to kfuncs (suggested by Kumar)
- For each dynamic pointer parameter in kfunc, additionally check if the
passed pointer is to the stack (suggested by Kumar)
- Split the validity/initialization and dynamic pointer type check also in
the verifier, and adjust the expected error message in the test (a test
for an unexpected dynptr type passed to a helper cannot be added due to
missing suitable helpers, but this case has been tested manually)
- Add verifier tests to check the dynamic pointers passed as argument to
kfuncs (suggested by Kumar)

v12:
- Put lookup_key and verify_pkcs7_sig tests in deny list for s390x (JIT
does not support calling kernel function)

v11:
- Move stringify_struct() macro to include/linux/btf.h (suggested by
Daniel)
- Change kernel configuration options in
tools/testing/selftests/bpf/config* from =m to =y

v10:
- Introduce key_lookup_flags_check() and system_keyring_id_check() inline
functions to check parameters (suggested by KP)
- Fix descriptions and comment of key-related kfuncs (suggested by KP)
- Register kfunc set only once (suggested by Alexei)
- Move needed kernel options to the architecture-independent configuration
for testing

v9:
- Drop patch to introduce KF_SLEEPABLE kfunc flag (already merged)
- Rename valid_ptr member of bpf_key to has_ref (suggested by Daniel)
- Check dynamic pointers in kfunc definition with bpf_dynptr_kern struct
definition instead of string, to detect structure renames (suggested by
Daniel)
- Explicitly say that we permit initialized dynamic pointers in kfunc
definition (suggested by Daniel)
- Remove noinline __weak from kfuncs definition (reported by Daniel)
- Simplify key lookup flags check in bpf_lookup_user_key() (suggested by
Daniel)
- Explain the reason for deferring key permission check (suggested by
Daniel)
- Allocate memory with GFP_ATOMIC in bpf_lookup_system_key(), and remove
KF_SLEEPABLE kfunc flag from kfunc declaration (suggested by Daniel)
- Define only one kfunc set and remove the loop for registration
(suggested by Alexei)

v8:
- Define the new bpf_key structure to carry the key pointer and whether
that pointer is valid or not (suggested by Daniel)
- Drop patch to mark a kfunc parameter with the __maybe_null suffix
- Improve documentation of kfuncs
- Introduce bpf_lookup_system_key() to obtain a key pointer suitable for
verify_pkcs7_signature() (suggested by Daniel)
- Use the new kfunc registration API
- Drop patch to test the __maybe_null suffix
- Add tests for bpf_lookup_system_key()

v7:
- Add support for using dynamic and NULL pointers in kfunc (suggested by
Alexei)
- Add new kfunc-related tests

v6:
- Switch back to key lookup helpers + signature verification (until v5),
and defer permission check from bpf_lookup_user_key() to
bpf_verify_pkcs7_signature()
- Add additional key lookup test to illustrate the usage of the
KEY_LOOKUP_CREATE flag and validate the flags (suggested by Daniel)
- Make description of flags of bpf_lookup_user_key() more user-friendly
(suggested by Daniel)
- Fix validation of flags parameter in bpf_lookup_user_key() (reported by
Daniel)
- Rename bpf_verify_pkcs7_signature() keyring-related parameters to
user_keyring and system_keyring to make their purpose more clear
- Accept keyring-related parameters of bpf_verify_pkcs7_signature() as
alternatives (suggested by KP)
- Replace unsigned long type with u64 in helper declaration (suggested by
Daniel)
- Extend the bpf_verify_pkcs7_signature() test by calling the helper
without data, by ensuring that the helper enforces the keyring-related
parameters as alternatives, by ensuring that the helper rejects
inaccessible and expired keyrings, and by checking all system keyrings
- Move bpf_lookup_user_key() and bpf_key_put() usage tests to
ref_tracking.c (suggested by John)
- Call bpf_lookup_user_key() and bpf_key_put() only in sleepable programs

v5:
- Move KEY_LOOKUP_ to include/linux/key.h
for validation of bpf_verify_pkcs7_signature() parameter
- Remove bpf_lookup_user_key() and bpf_key_put() helpers, and the
corresponding tests
- Replace struct key parameter of bpf_verify_pkcs7_signature() with the
keyring serial and lookup flags
- Call lookup_user_key() and key_put() in bpf_verify_pkcs7_signature()
code, to ensure that the retrieved key is used according to the
permission requested at lookup time
- Clarified keyring precedence in the description of
bpf_verify_pkcs7_signature() (suggested by John)
- Remove newline in the second argument of ASSERT_
- Fix helper prototype regular expression in bpf_doc.py

v4:
- Remove bpf_request_key_by_id(), don't return an invalid pointer that
other helpers can use
- Pass the keyring ID (without ULONG_MAX, suggested by Alexei) to
bpf_verify_pkcs7_signature()
- Introduce bpf_lookup_user_key() and bpf_key_put() helpers (suggested by
Alexei)
- Add lookup_key_norelease test, to ensure that the verifier blocks eBPF
programs which don't decrement the key reference count
- Parse raw PKCS#7 signature instead of module-style signature in the
verify_pkcs7_signature test (suggested by Alexei)
- Parse kernel module in user space and pass raw PKCS#7 signature to the
eBPF program for signature verification

v3:
- Rename bpf_verify_signature() back to bpf_verify_pkcs7_signature() to
avoid managing different parameters for each signature verification
function in one helper (suggested by Daniel)
- Use dynamic pointers and export bpf_dynptr_get_size() (suggested by
Alexei)
- Introduce bpf_request_key_by_id() to give more flexibility to the caller
of bpf_verify_pkcs7_signature() to retrieve the appropriate keyring
(suggested by Alexei)
- Fix test by reordering the gcc command line, always compile sign-file
- Improve helper support check mechanism in the test

v2:
- Rename bpf_verify_pkcs7_signature() to a more generic
bpf_verify_signature() and pass the signature type (suggested by KP)
- Move the helper and prototype declaration under #ifdef so that user
space can probe for support for the helper (suggested by Daniel)
- Describe better the keyring types (suggested by Daniel)
- Include linux/bpf.h instead of vmlinux.h to avoid implicit or
redeclaration
- Make the test selfcontained (suggested by Alexei)

v1:
- Don't define new map flag but introduce simple wrapper of
verify_pkcs7_signature() (suggested by Alexei and KP)

KP Singh (1):
bpf: Allow kfuncs to be used in LSM programs

Roberto Sassu (11):
bpf: Move dynptr type check to is_dynptr_type_expected()
btf: Allow dynamic pointer parameters in kfuncs
bpf: Export bpf_dynptr_get_size()
KEYS: Move KEY_LOOKUP_ to include/linux/key.h and define
KEY_LOOKUP_ALL
bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs
bpf: Add bpf_verify_pkcs7_signature() kfunc
selftests/bpf: Compile kernel with everything as built-in
selftests/bpf: Add verifier tests for bpf_lookup_*_key() and
bpf_key_put()
selftests/bpf: Add additional tests for bpf_lookup_*_key()
selftests/bpf: Add test for bpf_verify_pkcs7_signature() kfunc
selftests/bpf: Add tests for dynamic pointers parameters in kfuncs

include/linux/bpf.h | 9 +
include/linux/bpf_verifier.h | 5 +
include/linux/btf.h | 9 +
include/linux/key.h | 6 +
include/linux/verification.h | 8 +
kernel/bpf/btf.c | 34 ++
kernel/bpf/helpers.c | 2 +-
kernel/bpf/verifier.c | 35 +-
kernel/trace/bpf_trace.c | 180 ++++++++
security/keys/internal.h | 2 -
tools/testing/selftests/bpf/DENYLIST.s390x | 3 +
tools/testing/selftests/bpf/Makefile | 14 +-
tools/testing/selftests/bpf/config | 32 +-
tools/testing/selftests/bpf/config.x86_64 | 7 +-
.../testing/selftests/bpf/prog_tests/dynptr.c | 2 +-
.../bpf/prog_tests/kfunc_dynptr_param.c | 103 +++++
.../selftests/bpf/prog_tests/lookup_key.c | 112 +++++
.../bpf/prog_tests/verify_pkcs7_sig.c | 399 ++++++++++++++++++
.../bpf/progs/test_kfunc_dynptr_param.c | 57 +++
.../selftests/bpf/progs/test_lookup_key.c | 46 ++
.../bpf/progs/test_verify_pkcs7_sig.c | 100 +++++
tools/testing/selftests/bpf/test_verifier.c | 3 +-
.../selftests/bpf/verifier/ref_tracking.c | 139 ++++++
.../testing/selftests/bpf/verify_sig_setup.sh | 104 +++++
24 files changed, 1376 insertions(+), 35 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/lookup_key.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c
create mode 100644 tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
create mode 100644 tools/testing/selftests/bpf/progs/test_lookup_key.c
create mode 100644 tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c
create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh

--
2.25.1


2022-09-05 15:58:52

by Roberto Sassu

[permalink] [raw]
Subject: [PATCH v16 06/12] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

From: Roberto Sassu <[email protected]>

Add the bpf_lookup_user_key(), bpf_lookup_system_key() and bpf_key_put()
kfuncs, to respectively search a key with a given key handle serial number
and flags, obtain a key from a pre-determined ID defined in
include/linux/verification.h, and cleanup.

Introduce system_keyring_id_check() to validate the keyring ID parameter of
bpf_lookup_system_key().

Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/bpf.h | 8 +++
include/linux/verification.h | 8 +++
kernel/trace/bpf_trace.c | 135 +++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 9dbd7c3f8929..f3db614aece6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2595,4 +2595,12 @@ static inline void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype) {}
static inline void bpf_cgroup_atype_put(int cgroup_atype) {}
#endif /* CONFIG_BPF_LSM */

+struct key;
+
+#ifdef CONFIG_KEYS
+struct bpf_key {
+ struct key *key;
+ bool has_ref;
+};
+#endif /* CONFIG_KEYS */
#endif /* _LINUX_BPF_H */
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..f34e50ebcf60 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -17,6 +17,14 @@
#define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
#define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)

+static inline int system_keyring_id_check(u64 id)
+{
+ if (id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)
+ return -EINVAL;
+
+ return 0;
+}
+
/*
* The use to which an asymmetric key is being put.
*/
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 68e5cdd24cef..7a7023704ac2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -20,6 +20,8 @@
#include <linux/fprobe.h>
#include <linux/bsearch.h>
#include <linux/sort.h>
+#include <linux/key.h>
+#include <linux/verification.h>

#include <net/bpf_sk_storage.h>

@@ -1181,6 +1183,139 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
.arg1_type = ARG_PTR_TO_CTX,
};

+#ifdef CONFIG_KEYS
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "kfuncs which will be used in BPF programs");
+
+/**
+ * bpf_lookup_user_key - lookup a key by its serial
+ * @serial: key handle serial number
+ * @flags: lookup-specific flags
+ *
+ * Search a key with a given *serial* and the provided *flags*.
+ * If found, increment the reference count of the key by one, and
+ * return it in the bpf_key structure.
+ *
+ * The bpf_key structure must be passed to bpf_key_put() when done
+ * with it, so that the key reference count is decremented and the
+ * bpf_key structure is freed.
+ *
+ * Permission checks are deferred to the time the key is used by
+ * one of the available key-specific kfuncs.
+ *
+ * Set *flags* with KEY_LOOKUP_CREATE, to attempt creating a requested
+ * special keyring (e.g. session keyring), if it doesn't yet exist.
+ * Set *flags* with KEY_LOOKUP_PARTIAL, to lookup a key without waiting
+ * for the key construction, and to retrieve uninstantiated keys (keys
+ * without data attached to them).
+ *
+ * Return: a bpf_key pointer with a valid key pointer if the key is found, a
+ * NULL pointer otherwise.
+ */
+struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
+{
+ key_ref_t key_ref;
+ struct bpf_key *bkey;
+
+ if (flags & ~KEY_LOOKUP_ALL)
+ return NULL;
+
+ /*
+ * Permission check is deferred until the key is used, as the
+ * intent of the caller is unknown here.
+ */
+ key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
+ if (IS_ERR(key_ref))
+ return NULL;
+
+ bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
+ if (!bkey) {
+ key_put(key_ref_to_ptr(key_ref));
+ return NULL;
+ }
+
+ bkey->key = key_ref_to_ptr(key_ref);
+ bkey->has_ref = true;
+
+ return bkey;
+}
+
+/**
+ * bpf_lookup_system_key - lookup a key by a system-defined ID
+ * @id: key ID
+ *
+ * Obtain a bpf_key structure with a key pointer set to the passed key ID.
+ * The key pointer is marked as invalid, to prevent bpf_key_put() from
+ * attempting to decrement the key reference count on that pointer. The key
+ * pointer set in such way is currently understood only by
+ * verify_pkcs7_signature().
+ *
+ * Set *id* to one of the values defined in include/linux/verification.h:
+ * 0 for the primary keyring (immutable keyring of system keys);
+ * VERIFY_USE_SECONDARY_KEYRING for both the primary and secondary keyring
+ * (where keys can be added only if they are vouched for by existing keys
+ * in those keyrings); VERIFY_USE_PLATFORM_KEYRING for the platform
+ * keyring (primarily used by the integrity subsystem to verify a kexec'ed
+ * kerned image and, possibly, the initramfs signature).
+ *
+ * Return: a bpf_key pointer with an invalid key pointer set from the
+ * pre-determined ID on success, a NULL pointer otherwise
+ */
+struct bpf_key *bpf_lookup_system_key(u64 id)
+{
+ struct bpf_key *bkey;
+
+ if (system_keyring_id_check(id) < 0)
+ return NULL;
+
+ bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
+ if (!bkey)
+ return NULL;
+
+ bkey->key = (struct key *)(unsigned long)id;
+ bkey->has_ref = false;
+
+ return bkey;
+}
+
+/**
+ * bpf_key_put - decrement key reference count if key is valid and free bpf_key
+ * @bkey: bpf_key structure
+ *
+ * Decrement the reference count of the key inside *bkey*, if the pointer
+ * is valid, and free *bkey*.
+ */
+void bpf_key_put(struct bpf_key *bkey)
+{
+ if (bkey->has_ref)
+ key_put(bkey->key);
+
+ kfree(bkey);
+}
+
+__diag_pop();
+
+BTF_SET8_START(key_sig_kfunc_set)
+BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
+BTF_SET8_END(key_sig_kfunc_set)
+
+static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &key_sig_kfunc_set,
+};
+
+static int __init bpf_key_sig_kfuncs_init(void)
+{
+ return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
+ &bpf_key_sig_kfunc_set);
+}
+
+late_initcall(bpf_key_sig_kfuncs_init);
+#endif /* CONFIG_KEYS */
+
static const struct bpf_func_proto *
bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
--
2.25.1

2022-09-05 19:53:03

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

On Mon, 5 Sept 2022 at 16:34, Roberto Sassu
<[email protected]> wrote:
>
> From: Roberto Sassu <[email protected]>
>
> One of the desirable features in security is the ability to restrict import
> of data to a given system based on data authenticity. If data import can be
> restricted, it would be possible to enforce a system-wide policy based on
> the signing keys the system owner trusts.
>
> This feature is widely used in the kernel. For example, if the restriction
> is enabled, kernel modules can be plugged in only if they are signed with a
> key whose public part is in the primary or secondary keyring.
>
> For eBPF, it can be useful as well. For example, it might be useful to
> authenticate data an eBPF program makes security decisions on.
>
> [...]

CI is crashing with NULL deref for test_progs-no_alu32 with llvm-16,
but I don't think the problem is in this series. This is most likely
unrelated to BPF, as the crash happens inside
kernel/time/tick-sched.c:tick_nohz_restart_sched_tick.

This was the same case in
https://lore.kernel.org/bpf/CAP01T74steDfP6O8QOshoto3e3RnHhKtAeTbnrPBZS3YJXjvbA@mail.gmail.com.

So, https://github.com/kernel-patches/bpf/runs/8194263557?check_suite_focus=true
and https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true

look similar to me, and may not be related to BPF. They only trigger
during runs compiled using LLVM 16, so maybe some compiler
transformation is surfacing the problem?

2022-09-06 02:57:00

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH v16 06/12] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
<[email protected]> wrote:
>
> From: Roberto Sassu <[email protected]>
>
> Add the bpf_lookup_user_key(), bpf_lookup_system_key() and bpf_key_put()
> kfuncs, to respectively search a key with a given key handle serial number
> and flags, obtain a key from a pre-determined ID defined in
> include/linux/verification.h, and cleanup.
>
> Introduce system_keyring_id_check() to validate the keyring ID parameter of
> bpf_lookup_system_key().
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---

With a small nit below,
Acked-by: Kumar Kartikeya Dwivedi <[email protected]>

> include/linux/bpf.h | 8 +++
> include/linux/verification.h | 8 +++
> kernel/trace/bpf_trace.c | 135 +++++++++++++++++++++++++++++++++++
> 3 files changed, 151 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 9dbd7c3f8929..f3db614aece6 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2595,4 +2595,12 @@ static inline void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype) {}
> static inline void bpf_cgroup_atype_put(int cgroup_atype) {}
> #endif /* CONFIG_BPF_LSM */
>
> +struct key;
> +
> +#ifdef CONFIG_KEYS
> +struct bpf_key {
> + struct key *key;
> + bool has_ref;
> +};
> +#endif /* CONFIG_KEYS */
> #endif /* _LINUX_BPF_H */
> diff --git a/include/linux/verification.h b/include/linux/verification.h
> index a655923335ae..f34e50ebcf60 100644
> --- a/include/linux/verification.h
> +++ b/include/linux/verification.h
> @@ -17,6 +17,14 @@
> #define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
> #define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)
>
> +static inline int system_keyring_id_check(u64 id)
> +{
> + if (id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> /*
> * The use to which an asymmetric key is being put.
> */
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 68e5cdd24cef..7a7023704ac2 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -20,6 +20,8 @@
> #include <linux/fprobe.h>
> #include <linux/bsearch.h>
> #include <linux/sort.h>
> +#include <linux/key.h>
> +#include <linux/verification.h>
>
> #include <net/bpf_sk_storage.h>
>
> @@ -1181,6 +1183,139 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> +#ifdef CONFIG_KEYS
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "kfuncs which will be used in BPF programs");
> +
> +/**
> + * bpf_lookup_user_key - lookup a key by its serial
> + * @serial: key handle serial number
> + * @flags: lookup-specific flags
> + *
> + * Search a key with a given *serial* and the provided *flags*.
> + * If found, increment the reference count of the key by one, and
> + * return it in the bpf_key structure.
> + *
> + * The bpf_key structure must be passed to bpf_key_put() when done
> + * with it, so that the key reference count is decremented and the
> + * bpf_key structure is freed.
> + *
> + * Permission checks are deferred to the time the key is used by
> + * one of the available key-specific kfuncs.
> + *
> + * Set *flags* with KEY_LOOKUP_CREATE, to attempt creating a requested
> + * special keyring (e.g. session keyring), if it doesn't yet exist.
> + * Set *flags* with KEY_LOOKUP_PARTIAL, to lookup a key without waiting
> + * for the key construction, and to retrieve uninstantiated keys (keys
> + * without data attached to them).
> + *
> + * Return: a bpf_key pointer with a valid key pointer if the key is found, a
> + * NULL pointer otherwise.
> + */
> +struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
> +{
> + key_ref_t key_ref;
> + struct bpf_key *bkey;
> +
> + if (flags & ~KEY_LOOKUP_ALL)
> + return NULL;
> +
> + /*
> + * Permission check is deferred until the key is used, as the
> + * intent of the caller is unknown here.
> + */
> + key_ref = lookup_user_key(serial, flags, KEY_DEFER_PERM_CHECK);
> + if (IS_ERR(key_ref))
> + return NULL;
> +
> + bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);

Since this function (due to lookup_user_key) is sleepable, do we
really need GFP_ATOMIC here?

> + if (!bkey) {
> + key_put(key_ref_to_ptr(key_ref));
> + return NULL;
> + }
> +
> + bkey->key = key_ref_to_ptr(key_ref);
> + bkey->has_ref = true;
> +
> + return bkey;
> +}
> +
> +/**
> + * bpf_lookup_system_key - lookup a key by a system-defined ID
> + * @id: key ID
> + *
> + * Obtain a bpf_key structure with a key pointer set to the passed key ID.
> + * The key pointer is marked as invalid, to prevent bpf_key_put() from
> + * attempting to decrement the key reference count on that pointer. The key
> + * pointer set in such way is currently understood only by
> + * verify_pkcs7_signature().
> + *
> + * Set *id* to one of the values defined in include/linux/verification.h:
> + * 0 for the primary keyring (immutable keyring of system keys);
> + * VERIFY_USE_SECONDARY_KEYRING for both the primary and secondary keyring
> + * (where keys can be added only if they are vouched for by existing keys
> + * in those keyrings); VERIFY_USE_PLATFORM_KEYRING for the platform
> + * keyring (primarily used by the integrity subsystem to verify a kexec'ed
> + * kerned image and, possibly, the initramfs signature).
> + *
> + * Return: a bpf_key pointer with an invalid key pointer set from the
> + * pre-determined ID on success, a NULL pointer otherwise
> + */
> +struct bpf_key *bpf_lookup_system_key(u64 id)
> +{
> + struct bpf_key *bkey;
> +
> + if (system_keyring_id_check(id) < 0)
> + return NULL;
> +
> + bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
> + if (!bkey)
> + return NULL;
> +
> + bkey->key = (struct key *)(unsigned long)id;
> + bkey->has_ref = false;
> +
> + return bkey;
> +}
> +
> +/**
> + * bpf_key_put - decrement key reference count if key is valid and free bpf_key
> + * @bkey: bpf_key structure
> + *
> + * Decrement the reference count of the key inside *bkey*, if the pointer
> + * is valid, and free *bkey*.
> + */
> +void bpf_key_put(struct bpf_key *bkey)
> +{
> + if (bkey->has_ref)
> + key_put(bkey->key);
> +
> + kfree(bkey);
> +}
> +
> +__diag_pop();
> +
> +BTF_SET8_START(key_sig_kfunc_set)
> +BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
> +BTF_SET8_END(key_sig_kfunc_set)
> +
> +static const struct btf_kfunc_id_set bpf_key_sig_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &key_sig_kfunc_set,
> +};
> +
> +static int __init bpf_key_sig_kfuncs_init(void)
> +{
> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
> + &bpf_key_sig_kfunc_set);
> +}
> +
> +late_initcall(bpf_key_sig_kfuncs_init);
> +#endif /* CONFIG_KEYS */
> +
> static const struct bpf_func_proto *
> bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> --
> 2.25.1
>

2022-09-06 07:53:46

by Roberto Sassu

[permalink] [raw]
Subject: Re: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

On Mon, 2022-09-05 at 21:26 +0200, Kumar Kartikeya Dwivedi wrote:
> On Mon, 5 Sept 2022 at 16:34, Roberto Sassu
> <[email protected]> wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > One of the desirable features in security is the ability to
> > restrict import
> > of data to a given system based on data authenticity. If data
> > import can be
> > restricted, it would be possible to enforce a system-wide policy
> > based on
> > the signing keys the system owner trusts.
> >
> > This feature is widely used in the kernel. For example, if the
> > restriction
> > is enabled, kernel modules can be plugged in only if they are
> > signed with a
> > key whose public part is in the primary or secondary keyring.
> >
> > For eBPF, it can be useful as well. For example, it might be useful
> > to
> > authenticate data an eBPF program makes security decisions on.
> >
> > [...]
>
> CI is crashing with NULL deref for test_progs-no_alu32 with llvm-16,
> but I don't think the problem is in this series. This is most likely
> unrelated to BPF, as the crash happens inside
> kernel/time/tick-sched.c:tick_nohz_restart_sched_tick.
>
> This was the same case in
> https://lore.kernel.org/bpf/CAP01T74steDfP6O8QOshoto3e3RnHhKtAeTbnrPBZS3YJXjvbA@mail.gmail.com.
>
> So,
> https://github.com/kernel-patches/bpf/runs/8194263557?check_suite_focus=true
> and
> https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
>
> look similar to me, and may not be related to BPF. They only trigger
> during runs compiled using LLVM 16, so maybe some compiler
> transformation is surfacing the problem?

Yes, I saw that too. Not sure what the cause could be.

Thanks

Roberto

2022-09-06 08:29:51

by Roberto Sassu

[permalink] [raw]
Subject: Re: [PATCH v16 06/12] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

On Tue, 2022-09-06 at 04:43 +0200, Kumar Kartikeya Dwivedi wrote:
> On Mon, 5 Sept 2022 at 16:35, Roberto Sassu
> <[email protected]> wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > Add the bpf_lookup_user_key(), bpf_lookup_system_key() and
> > bpf_key_put()
> > kfuncs, to respectively search a key with a given key handle serial
> > number
> > and flags, obtain a key from a pre-determined ID defined in
> > include/linux/verification.h, and cleanup.
> >
> > Introduce system_keyring_id_check() to validate the keyring ID
> > parameter of
> > bpf_lookup_system_key().
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
>
> With a small nit below,
> Acked-by: Kumar Kartikeya Dwivedi <[email protected]>
>
> > include/linux/bpf.h | 8 +++
> > include/linux/verification.h | 8 +++
> > kernel/trace/bpf_trace.c | 135
> > +++++++++++++++++++++++++++++++++++
> > 3 files changed, 151 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 9dbd7c3f8929..f3db614aece6 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -2595,4 +2595,12 @@ static inline void bpf_cgroup_atype_get(u32
> > attach_btf_id, int cgroup_atype) {}
> > static inline void bpf_cgroup_atype_put(int cgroup_atype) {}
> > #endif /* CONFIG_BPF_LSM */
> >
> > +struct key;
> > +
> > +#ifdef CONFIG_KEYS
> > +struct bpf_key {
> > + struct key *key;
> > + bool has_ref;
> > +};
> > +#endif /* CONFIG_KEYS */
> > #endif /* _LINUX_BPF_H */
> > diff --git a/include/linux/verification.h
> > b/include/linux/verification.h
> > index a655923335ae..f34e50ebcf60 100644
> > --- a/include/linux/verification.h
> > +++ b/include/linux/verification.h
> > @@ -17,6 +17,14 @@
> > #define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
> > #define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)
> >
> > +static inline int system_keyring_id_check(u64 id)
> > +{
> > + if (id > (unsigned long)VERIFY_USE_PLATFORM_KEYRING)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * The use to which an asymmetric key is being put.
> > */
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 68e5cdd24cef..7a7023704ac2 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -20,6 +20,8 @@
> > #include <linux/fprobe.h>
> > #include <linux/bsearch.h>
> > #include <linux/sort.h>
> > +#include <linux/key.h>
> > +#include <linux/verification.h>
> >
> > #include <net/bpf_sk_storage.h>
> >
> > @@ -1181,6 +1183,139 @@ static const struct bpf_func_proto
> > bpf_get_func_arg_cnt_proto = {
> > .arg1_type = ARG_PTR_TO_CTX,
> > };
> >
> > +#ifdef CONFIG_KEYS
> > +__diag_push();
> > +__diag_ignore_all("-Wmissing-prototypes",
> > + "kfuncs which will be used in BPF programs");
> > +
> > +/**
> > + * bpf_lookup_user_key - lookup a key by its serial
> > + * @serial: key handle serial number
> > + * @flags: lookup-specific flags
> > + *
> > + * Search a key with a given *serial* and the provided *flags*.
> > + * If found, increment the reference count of the key by one, and
> > + * return it in the bpf_key structure.
> > + *
> > + * The bpf_key structure must be passed to bpf_key_put() when done
> > + * with it, so that the key reference count is decremented and the
> > + * bpf_key structure is freed.
> > + *
> > + * Permission checks are deferred to the time the key is used by
> > + * one of the available key-specific kfuncs.
> > + *
> > + * Set *flags* with KEY_LOOKUP_CREATE, to attempt creating a
> > requested
> > + * special keyring (e.g. session keyring), if it doesn't yet
> > exist.
> > + * Set *flags* with KEY_LOOKUP_PARTIAL, to lookup a key without
> > waiting
> > + * for the key construction, and to retrieve uninstantiated keys
> > (keys
> > + * without data attached to them).
> > + *
> > + * Return: a bpf_key pointer with a valid key pointer if the key
> > is found, a
> > + * NULL pointer otherwise.
> > + */
> > +struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
> > +{
> > + key_ref_t key_ref;
> > + struct bpf_key *bkey;
> > +
> > + if (flags & ~KEY_LOOKUP_ALL)
> > + return NULL;
> > +
> > + /*
> > + * Permission check is deferred until the key is used, as
> > the
> > + * intent of the caller is unknown here.
> > + */
> > + key_ref = lookup_user_key(serial, flags,
> > KEY_DEFER_PERM_CHECK);
> > + if (IS_ERR(key_ref))
> > + return NULL;
> > +
> > + bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
>
> Since this function (due to lookup_user_key) is sleepable, do we
> really need GFP_ATOMIC here?

Daniel suggested it for bpf_lookup_system_key(), so that the kfunc does
not have to be sleepable. For symmetry, I did the same to
bpf_lookup_user_key(). Will switch back to GFP_KERNEL.

Thanks

Roberto

2022-09-06 19:05:14

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH v16 06/12] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

On Tue, Sep 6, 2022 at 1:01 AM Roberto Sassu
<[email protected]> wrote:
>
> > > +struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
> > > +{
> > > + key_ref_t key_ref;
> > > + struct bpf_key *bkey;
> > > +
> > > + if (flags & ~KEY_LOOKUP_ALL)
> > > + return NULL;
> > > +
> > > + /*
> > > + * Permission check is deferred until the key is used, as
> > > the
> > > + * intent of the caller is unknown here.
> > > + */
> > > + key_ref = lookup_user_key(serial, flags,
> > > KEY_DEFER_PERM_CHECK);
> > > + if (IS_ERR(key_ref))
> > > + return NULL;
> > > +
> > > + bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
> >
> > Since this function (due to lookup_user_key) is sleepable, do we
> > really need GFP_ATOMIC here?
>
> Daniel suggested it for bpf_lookup_system_key(), so that the kfunc does
> not have to be sleepable.

Hold on. It has to be sleepable. Just take a look
at what lookup_user_key is doing inside.

> For symmetry, I did the same to
> bpf_lookup_user_key(). Will switch back to GFP_KERNEL.
>
> Thanks
>
> Roberto
>

2022-09-07 07:25:37

by Roberto Sassu

[permalink] [raw]
Subject: Re: [PATCH v16 06/12] bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs

On Tue, 2022-09-06 at 11:45 -0700, Alexei Starovoitov wrote:
> On Tue, Sep 6, 2022 at 1:01 AM Roberto Sassu
> <[email protected]> wrote:
> > > > +struct bpf_key *bpf_lookup_user_key(u32 serial, u64 flags)
> > > > +{
> > > > + key_ref_t key_ref;
> > > > + struct bpf_key *bkey;
> > > > +
> > > > + if (flags & ~KEY_LOOKUP_ALL)
> > > > + return NULL;
> > > > +
> > > > + /*
> > > > + * Permission check is deferred until the key is used,
> > > > as
> > > > the
> > > > + * intent of the caller is unknown here.
> > > > + */
> > > > + key_ref = lookup_user_key(serial, flags,
> > > > KEY_DEFER_PERM_CHECK);
> > > > + if (IS_ERR(key_ref))
> > > > + return NULL;
> > > > +
> > > > + bkey = kmalloc(sizeof(*bkey), GFP_ATOMIC);
> > >
> > > Since this function (due to lookup_user_key) is sleepable, do we
> > > really need GFP_ATOMIC here?
> >
> > Daniel suggested it for bpf_lookup_system_key(), so that the kfunc
> > does
> > not have to be sleepable.
>
> Hold on. It has to be sleepable. Just take a look
> at what lookup_user_key is doing inside.
>

https://lore.kernel.org/bpf/[email protected]/

Roberto

2022-09-07 15:24:10

by Roberto Sassu

[permalink] [raw]
Subject: Re: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

On Wed, 2022-09-07 at 16:57 +0200, Kumar Kartikeya Dwivedi wrote:
> On Wed, 7 Sept 2022 at 16:49, Roberto Sassu
> <[email protected]> wrote:
> > On Tue, 2022-09-06 at 09:35 +0200, Roberto Sassu wrote:
> > > On Mon, 2022-09-05 at 21:26 +0200, Kumar Kartikeya Dwivedi wrote:
> > > > On Mon, 5 Sept 2022 at 16:34, Roberto Sassu
> > > > <[email protected]> wrote:
> > > > > From: Roberto Sassu <[email protected]>
> > > > >
> > > > > One of the desirable features in security is the ability to
> > > > > restrict import
> > > > > of data to a given system based on data authenticity. If data
> > > > > import can be
> > > > > restricted, it would be possible to enforce a system-wide
> > > > > policy
> > > > > based on
> > > > > the signing keys the system owner trusts.
> > > > >
> > > > > This feature is widely used in the kernel. For example, if
> > > > > the
> > > > > restriction
> > > > > is enabled, kernel modules can be plugged in only if they are
> > > > > signed with a
> > > > > key whose public part is in the primary or secondary keyring.
> > > > >
> > > > > For eBPF, it can be useful as well. For example, it might be
> > > > > useful
> > > > > to
> > > > > authenticate data an eBPF program makes security decisions
> > > > > on.
> > > > >
> > > > > [...]
> > > >
> > > > CI is crashing with NULL deref for test_progs-no_alu32 with
> > > > llvm-
> > > > 16,
> > > > but I don't think the problem is in this series. This is most
> > > > likely
> > > > unrelated to BPF, as the crash happens inside
> > > > kernel/time/tick-sched.c:tick_nohz_restart_sched_tick.
> > > >
> > > > This was the same case in
> > > > https://lore.kernel.org/bpf/CAP01T74steDfP6O8QOshoto3e3RnHhKtAeTbnrPBZS3YJXjvbA@mail.gmail.com.
> > > >
> > > > So,
> > > > https://github.com/kernel-patches/bpf/runs/8194263557?check_suite_focus=true
> > > > and
> > > > https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
> > > >
> > > > look similar to me, and may not be related to BPF. They only
> > > > trigger
> > > > during runs compiled using LLVM 16, so maybe some compiler
> > > > transformation is surfacing the problem?
> > >
> > > Yes, I saw that too. Not sure what the cause could be.
> > >
> >
> > Another occurrence, this time with gcc:
> >
> > https://github.com/robertosassu/vmtest/runs/8230071814?check_suite_focus=true
> >
>
> ... and it seems like this run does not even have your patches,
> right?
>

Uhm, the kernel patches are there. The tests except the verifier ones
weren't successfuly applied, probably due to the deny list.

One thing in common with the failures seems when the panic happens,
when test_progs reaches verif_twfw. I will try to execute this and
earlier tests to reproduce the panic locally.

Roberto

2022-09-07 15:41:56

by Roberto Sassu

[permalink] [raw]
Subject: Re: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

On Tue, 2022-09-06 at 09:35 +0200, Roberto Sassu wrote:
> On Mon, 2022-09-05 at 21:26 +0200, Kumar Kartikeya Dwivedi wrote:
> > On Mon, 5 Sept 2022 at 16:34, Roberto Sassu
> > <[email protected]> wrote:
> > > From: Roberto Sassu <[email protected]>
> > >
> > > One of the desirable features in security is the ability to
> > > restrict import
> > > of data to a given system based on data authenticity. If data
> > > import can be
> > > restricted, it would be possible to enforce a system-wide policy
> > > based on
> > > the signing keys the system owner trusts.
> > >
> > > This feature is widely used in the kernel. For example, if the
> > > restriction
> > > is enabled, kernel modules can be plugged in only if they are
> > > signed with a
> > > key whose public part is in the primary or secondary keyring.
> > >
> > > For eBPF, it can be useful as well. For example, it might be
> > > useful
> > > to
> > > authenticate data an eBPF program makes security decisions on.
> > >
> > > [...]
> >
> > CI is crashing with NULL deref for test_progs-no_alu32 with llvm-
> > 16,
> > but I don't think the problem is in this series. This is most
> > likely
> > unrelated to BPF, as the crash happens inside
> > kernel/time/tick-sched.c:tick_nohz_restart_sched_tick.
> >
> > This was the same case in
> > https://lore.kernel.org/bpf/CAP01T74steDfP6O8QOshoto3e3RnHhKtAeTbnrPBZS3YJXjvbA@mail.gmail.com.
> >
> > So,
> > https://github.com/kernel-patches/bpf/runs/8194263557?check_suite_focus=true
> > and
> > https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
> >
> > look similar to me, and may not be related to BPF. They only
> > trigger
> > during runs compiled using LLVM 16, so maybe some compiler
> > transformation is surfacing the problem?
>
> Yes, I saw that too. Not sure what the cause could be.
>

Another occurrence, this time with gcc:

https://github.com/robertosassu/vmtest/runs/8230071814?check_suite_focus=true

Roberto

2022-09-07 15:44:08

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH v16 00/12] bpf: Add kfuncs for PKCS#7 signature verification

On Wed, 7 Sept 2022 at 16:49, Roberto Sassu
<[email protected]> wrote:
>
> On Tue, 2022-09-06 at 09:35 +0200, Roberto Sassu wrote:
> > On Mon, 2022-09-05 at 21:26 +0200, Kumar Kartikeya Dwivedi wrote:
> > > On Mon, 5 Sept 2022 at 16:34, Roberto Sassu
> > > <[email protected]> wrote:
> > > > From: Roberto Sassu <[email protected]>
> > > >
> > > > One of the desirable features in security is the ability to
> > > > restrict import
> > > > of data to a given system based on data authenticity. If data
> > > > import can be
> > > > restricted, it would be possible to enforce a system-wide policy
> > > > based on
> > > > the signing keys the system owner trusts.
> > > >
> > > > This feature is widely used in the kernel. For example, if the
> > > > restriction
> > > > is enabled, kernel modules can be plugged in only if they are
> > > > signed with a
> > > > key whose public part is in the primary or secondary keyring.
> > > >
> > > > For eBPF, it can be useful as well. For example, it might be
> > > > useful
> > > > to
> > > > authenticate data an eBPF program makes security decisions on.
> > > >
> > > > [...]
> > >
> > > CI is crashing with NULL deref for test_progs-no_alu32 with llvm-
> > > 16,
> > > but I don't think the problem is in this series. This is most
> > > likely
> > > unrelated to BPF, as the crash happens inside
> > > kernel/time/tick-sched.c:tick_nohz_restart_sched_tick.
> > >
> > > This was the same case in
> > > https://lore.kernel.org/bpf/CAP01T74steDfP6O8QOshoto3e3RnHhKtAeTbnrPBZS3YJXjvbA@mail.gmail.com.
> > >
> > > So,
> > > https://github.com/kernel-patches/bpf/runs/8194263557?check_suite_focus=true
> > > and
> > > https://github.com/kernel-patches/bpf/runs/7982907380?check_suite_focus=true
> > >
> > > look similar to me, and may not be related to BPF. They only
> > > trigger
> > > during runs compiled using LLVM 16, so maybe some compiler
> > > transformation is surfacing the problem?
> >
> > Yes, I saw that too. Not sure what the cause could be.
> >
>
> Another occurrence, this time with gcc:
>
> https://github.com/robertosassu/vmtest/runs/8230071814?check_suite_focus=true
>

... and it seems like this run does not even have your patches, right?

> Roberto
>