On Mon, 5 Sept 2022 at 16:34, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

From: KP Singh kpsingh@kernel.org

In preparation for the addition of new kfuncs, allow kfuncs defined in the tracing subsystem to be used in LSM programs by mapping the LSM program type to the TRACING hook.

Signed-off-by: KP Singh kpsingh@kernel.org Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

---

Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com

kernel/bpf/btf.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 903719b89238..e49b3b6d48ad 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7243,6 +7243,7 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type) case BPF_PROG_TYPE_STRUCT_OPS: return BTF_KFUNC_HOOK_STRUCT_OPS; case BPF_PROG_TYPE_TRACING:

•   case BPF_PROG_TYPE_LSM:
            return BTF_KFUNC_HOOK_TRACING;
    case BPF_PROG_TYPE_SYSCALL:
            return BTF_KFUNC_HOOK_SYSCALL;
  

-- 2.25.1

On Mon, 5 Sept 2022 at 16:34, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

From: Roberto Sassu roberto.sassu@huawei.com

Move dynptr type check to is_dynptr_type_expected() from is_dynptr_reg_valid_init(), so that callers can better determine the cause of a negative result (dynamic pointer not valid/initialized, dynamic pointer of the wrong type). It will be useful for example for BTF, to restrict which dynamic pointer types can be passed to kfuncs, as initially only the local type will be supported.

Also, splitting makes the code more readable, since checking the dynamic pointer type is not necessarily related to validity and initialization.

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).

Cc: Joanne Koong joannelkoong@gmail.com Cc: Kumar Kartikeya Dwivedi memxor@gmail.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

---

I'm not sure the split is _really_ needed, but I don't feel strongly about it and defer to Joanne and others. Otherwise, Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com

kernel/bpf/verifier.c | 35 ++++++++++++++----- .../testing/selftests/bpf/prog_tests/dynptr.c | 2 +- 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 068b20ed34d2..10b3c0a81d09 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -779,8 +779,8 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ return true; }

-static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,

•                                enum bpf_arg_type arg_type)
  

+static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,

•                                struct bpf_reg_state *reg)
  

{ struct bpf_func_state *state = func(env, reg); int spi = get_spi(reg->off); @@ -796,11 +796,24 @@ static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_re return false; }

•   return true;
  

+}

+static bool is_dynptr_type_expected(struct bpf_verifier_env *env,

•                               struct bpf_reg_state *reg,
  

•                               enum bpf_arg_type arg_type)
  

+{

•   struct bpf_func_state *state = func(env, reg);
  

•   enum bpf_dynptr_type dynptr_type;
  

•   int spi = get_spi(reg->off);
  

•   /* ARG_PTR_TO_DYNPTR takes any type of dynptr */
    if (arg_type == ARG_PTR_TO_DYNPTR)
            return true;
  
  

•   return state->stack[spi].spilled_ptr.dynptr.type == arg_to_dynptr_type(arg_type);
  

•   dynptr_type = arg_to_dynptr_type(arg_type);
  

•   return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type;
  

}

/* The reg state of a pointer or a bounded scalar was saved when @@ -6050,21 +6063,27 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, }

                    meta->uninit_dynptr_regno = regno;

•           } else if (!is_dynptr_reg_valid_init(env, reg, arg_type)) {
  

•           } else if (!is_dynptr_reg_valid_init(env, reg)) {
  

•                   verbose(env,
  

•                           "Expected an initialized dynptr as arg #%d\n",
  

•                           arg + 1);
  

•                   return -EINVAL;
  

•           } else if (!is_dynptr_type_expected(env, reg, arg_type)) {
                    const char *err_extra = "";
  
                    switch (arg_type & DYNPTR_TYPE_FLAG_MASK) {
                    case DYNPTR_TYPE_LOCAL:
  

•                           err_extra = "local ";
  

•                           err_extra = "local";
                            break;
                    case DYNPTR_TYPE_RINGBUF:
  

•                           err_extra = "ringbuf ";
  

•                           err_extra = "ringbuf";
                            break;
                    default:
  

•                           err_extra = "<unknown>";
                            break;
                    }
  

•                   verbose(env, "Expected an initialized %sdynptr as arg #%d\n",
  

•                   verbose(env,
  

•                           "Expected a dynptr of type %s as arg #%d\n",
                            err_extra, arg + 1);
                    return -EINVAL;
            }
  

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index bcf80b9f7c27..8fc4e6c02bfd 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -30,7 +30,7 @@ static struct { {"invalid_helper2", "Expected an initialized dynptr as arg #3"}, {"invalid_write1", "Expected an initialized dynptr as arg #1"}, {"invalid_write2", "Expected an initialized dynptr as arg #3"},

•   {"invalid_write3", "Expected an initialized ringbuf dynptr as arg #1"},
  

•   {"invalid_write3", "Expected an initialized dynptr as arg #1"},
    {"invalid_write4", "arg 1 is an unacquired reference"},
    {"invalid_read1", "invalid read from stack"},
    {"invalid_read2", "cannot pass in dynptr at an offset"},
  

-- 2.25.1

On Mon, 5 Sept 2022 at 16:34, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

From: Roberto Sassu roberto.sassu@huawei.com

Allow dynamic pointers (struct bpf_dynptr_kern *) to be specified as parameters in kfuncs. Also, ensure that dynamic pointers passed as argument are valid and initialized, are a pointer to the stack, and of the type local. More dynamic pointer types can be supported in the future.

To properly detect whether a parameter is of the desired type, introduce the stringify_struct() macro to compare the returned structure name with the desired name. In addition, protect against structure renames, by halting the build with BUILD_BUG_ON(), so that developers have to revisit the code.

To check if a dynamic pointer passed to the kfunc is valid and initialized, and if its type is local, export the existing functions is_dynptr_reg_valid_init() and is_dynptr_type_expected().

Cc: Joanne Koong joannelkoong@gmail.com Cc: Kumar Kartikeya Dwivedi memxor@gmail.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

---

Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com

include/linux/bpf_verifier.h | 5 +++++ include/linux/btf.h | 9 +++++++++ kernel/bpf/btf.c | 33 +++++++++++++++++++++++++++++++++ kernel/bpf/verifier.c | 10 +++++----- 4 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 1fdddbf3546b..dd58dfccd025 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -571,6 +571,11 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state u32 regno); int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 regno, u32 mem_size); +bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,

•                         struct bpf_reg_state *reg);
  

+bool is_dynptr_type_expected(struct bpf_verifier_env *env,

•                        struct bpf_reg_state *reg,
  

•                        enum bpf_arg_type arg_type);
  
  

/* this lives here instead of in bpf.h because it needs to dereference tgt_prog */ static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog, diff --git a/include/linux/btf.h b/include/linux/btf.h index ad93c2d9cc1c..f546d368ac5d 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -52,6 +52,15 @@ #define KF_SLEEPABLE (1 << 5) /* kfunc may sleep */ #define KF_DESTRUCTIVE (1 << 6) /* kfunc performs destructive actions */

+/*

• • Return the name of the passed struct, if exists, or halt the build if for
• • example the structure gets renamed. In this way, developers have to revisit
• • the code using that structure name, and update it accordingly.
• */

+#define stringify_struct(x) \

•   ({ BUILD_BUG_ON(sizeof(struct x) < 0);  \
  

•      __stringify(x); })
  

struct btf; struct btf_member; struct btf_type; diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index e49b3b6d48ad..4266bff5fada 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6362,15 +6362,20 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,

                    if (is_kfunc) {
                            bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);

•                           bool arg_dynptr = btf_type_is_struct(ref_t) &&
  

•                                             !strcmp(ref_tname,
  

•                                                     stringify_struct(bpf_dynptr_kern));
  
                            /* Permit pointer to mem, but only when argument
                             * type is pointer to scalar, or struct composed
                             * (recursively) of scalars.
                             * When arg_mem_size is true, the pointer can be
                             * void *.
  

•                            * Also permit initialized local dynamic pointers.
                             */
                            if (!btf_type_is_scalar(ref_t) &&
                                !__btf_type_is_scalar_struct(log, btf, ref_t, 0) &&
  

•                               !arg_dynptr &&
                                (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
                                    bpf_log(log,
                                            "arg#%d pointer type %s %s must point to %sscalar, or struct with scalar\n",
  

@@ -6378,6 +6383,34 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, return -EINVAL; }

•                           if (arg_dynptr) {
  

•                                   if (reg->type != PTR_TO_STACK) {
  

•                                           bpf_log(log, "arg#%d pointer type %s %s not to stack\n",
  

•                                                   i, btf_type_str(ref_t),
  

•                                                   ref_tname);
  

•                                           return -EINVAL;
  

•                                   }
  

•                                   if (!is_dynptr_reg_valid_init(env, reg)) {
  

•                                           bpf_log(log,
  

•                                                   "arg#%d pointer type %s %s must be valid and initialized\n",
  

•                                                   i, btf_type_str(ref_t),
  

•                                                   ref_tname);
  

•                                           return -EINVAL;
  

•                                   }
  

•                                   if (!is_dynptr_type_expected(env, reg,
  

•                                                   ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL)) {
  

•                                           bpf_log(log,
  

•                                                   "arg#%d pointer type %s %s points to unsupported dynamic pointer type\n",
  

•                                                   i, btf_type_str(ref_t),
  

•                                                   ref_tname);
  

•                                           return -EINVAL;
  

•                                   }
  

•                                   continue;
  

•                           }
  

•                           /* Check for mem, len pair */
                            if (arg_mem_size) {
                                    if (check_kfunc_mem_size_reg(env, &regs[regno + 1], regno + 1)) {
  

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 10b3c0a81d09..8f02729074c6 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -779,8 +779,8 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_ return true; }

-static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,

•                                struct bpf_reg_state *reg)
  

+bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,

•                         struct bpf_reg_state *reg)
  

{ struct bpf_func_state *state = func(env, reg); int spi = get_spi(reg->off); @@ -799,9 +799,9 @@ static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, return true; }

-static bool is_dynptr_type_expected(struct bpf_verifier_env *env,

•                               struct bpf_reg_state *reg,
  

•                               enum bpf_arg_type arg_type)
  

+bool is_dynptr_type_expected(struct bpf_verifier_env *env,

•                        struct bpf_reg_state *reg,
  

•                        enum bpf_arg_type arg_type)
  

{ struct bpf_func_state *state = func(env, reg); enum bpf_dynptr_type dynptr_type; -- 2.25.1

On Mon, 5 Sept 2022 at 16:35, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

From: Roberto Sassu roberto.sassu@huawei.com

Export bpf_dynptr_get_size(), so that kernel code dealing with eBPF dynamic pointers can obtain the real size of data carried by this data structure.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reviewed-by: Joanne Koong joannelkoong@gmail.com Acked-by: KP Singh kpsingh@kernel.org

---

Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com

include/linux/bpf.h | 1 + kernel/bpf/helpers.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 9c1674973e03..9dbd7c3f8929 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2585,6 +2585,7 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data, enum bpf_dynptr_type type, u32 offset, u32 size); void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr); int bpf_dynptr_check_size(u32 size); +u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr);

#ifdef CONFIG_BPF_LSM void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype); diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index fc08035f14ed..824864ac82d1 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1408,7 +1408,7 @@ static void bpf_dynptr_set_type(struct bpf_dynptr_kern *ptr, enum bpf_dynptr_typ ptr->size |= type << DYNPTR_TYPE_SHIFT; }

-static u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) +u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr) { return ptr->size & DYNPTR_SIZE_MASK; } -- 2.25.1

On Tue, 2022-09-06 at 00:38 +0300, Jarkko Sakkinen wrote:

On Mon, Sep 05, 2022 at 04:33:11PM +0200, Roberto Sassu wrote:

...

From: Roberto Sassu roberto.sassu@huawei.com

In preparation for the patch that introduces the bpf_lookup_user_key() eBPF kfunc, move KEY_LOOKUP_ definitions to include/linux/key.h, to be able to validate the kfunc parameters. Add them to enum key_lookup_flag, so that all the current ones and the ones defined in the future are automatically exported through BTF and available to eBPF programs.

Also, add KEY_LOOKUP_ALL to the enum, to facilitate checking whether a variable contains only defined flags.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reviewed-by: KP Singh kpsingh@kernel.org Acked-by: Jarkko Sakkinen jarkko@kernel.org

You should remove ack if there is any substantial change.

Yes, sorry. I thought you were fine with the change due to:

https://lore.kernel.org/bpf/YxF4H9MTDj+PnJ+V@kernel.org/

...

---

include/linux/key.h | 6 ++++++ security/keys/internal.h | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h index 7febc4881363..d84171f90cbd 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -88,6 +88,12 @@ enum key_need_perm { KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */ }; +enum key_lookup_flag {

• KEY_LOOKUP_CREATE = 0x01, /* Create special keyrings if they

don't exist */

• KEY_LOOKUP_PARTIAL = 0x02, /* Permit partially constructed

keys to be found */

• KEY_LOOKUP_ALL = (KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL), /*

OR of previous flags */

Drop the comments (should be reviewed separately + out of context).

The same style is used for many definitions in include/linux/key.h

No problem to remove them, please just let me know where they should be. Often, eBPF maintainers asked me to add a description to the code to explain how new definitions should be used.

Thanks

Roberto

From: Roberto Sassu roberto.sassu@huawei.com

In preparation for the patch that introduces the bpf_lookup_user_key() eBPF kfunc, move KEY_LOOKUP_ definitions to include/linux/key.h, to be able to validate the kfunc parameters. Add them to enum key_lookup_flag, so that all the current ones and the ones defined in the future are automatically exported through BTF and available to eBPF programs.

Also, add KEY_LOOKUP_ALL to the enum, with the logical OR of currently defined flags as value, to facilitate checking whether a variable contains only those flags.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/key.h | 6 ++++++ security/keys/internal.h | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h index 7febc4881363..d27477faf00d 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -88,6 +88,12 @@ enum key_need_perm { KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */ };

+enum key_lookup_flag { + KEY_LOOKUP_CREATE = 0x01, + KEY_LOOKUP_PARTIAL = 0x02, + KEY_LOOKUP_ALL = (KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL), +}; + struct seq_file; struct user_struct; struct signal_struct; diff --git a/security/keys/internal.h b/security/keys/internal.h index 9b9cf3b6fcbb..3c1e7122076b 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -165,8 +165,6 @@ extern struct key *request_key_and_link(struct key_type *type,

extern bool lookup_user_key_possessed(const struct key *key, const struct key_match_data *match_data); -#define KEY_LOOKUP_CREATE 0x01 -#define KEY_LOOKUP_PARTIAL 0x02

extern long join_session_keyring(const char *name); extern void key_change_session_keyring(struct callback_head *twork);

On Tue, 2022-09-06 at 15:26 +0300, Jarkko Sakkinen wrote:

On Tue, Sep 06, 2022 at 02:15:06PM +0200, Roberto Sassu wrote:

...

From: Roberto Sassu roberto.sassu@huawei.com

In preparation for the patch that introduces the bpf_lookup_user_key() eBPF kfunc, move KEY_LOOKUP_ definitions to include/linux/key.h, to be able to validate the kfunc parameters. Add them to enum key_lookup_flag, so that all the current ones and the ones defined in the future are automatically exported through BTF and available to eBPF programs.

Also, add KEY_LOOKUP_ALL to the enum, with the logical OR of currently defined flags as value, to facilitate checking whether a variable contains only those flags.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

include/linux/key.h | 6 ++++++ security/keys/internal.h | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h index 7febc4881363..d27477faf00d 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -88,6 +88,12 @@ enum key_need_perm { KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */ }; +enum key_lookup_flag {

• KEY_LOOKUP_CREATE = 0x01,
• KEY_LOOKUP_PARTIAL = 0x02,
• KEY_LOOKUP_ALL = (KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL),

+};

struct seq_file; struct user_struct; struct signal_struct; diff --git a/security/keys/internal.h b/security/keys/internal.h index 9b9cf3b6fcbb..3c1e7122076b 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -165,8 +165,6 @@ extern struct key *request_key_and_link(struct key_type *type, extern bool lookup_user_key_possessed(const struct key *key, const struct key_match_data *match_data); -#define KEY_LOOKUP_CREATE 0x01 -#define KEY_LOOKUP_PARTIAL 0x02 extern long join_session_keyring(const char *name); extern void key_change_session_keyring(struct callback_head

*twork);

2.25.1

Thanks,

Acked-by: Jarkko Sakkinen jarkko@kernel.org

Thanks Jarkko.

KP, ok also for you?

Thanks

Roberto

On Mon, 5 Sept 2022 at 16:35, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

From: Roberto Sassu roberto.sassu@huawei.com

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 roberto.sassu@huawei.com

With a small nit below, Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com

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

On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:

On Mon, 5 Sept 2022 at 16:35, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

...

From: Roberto Sassu roberto.sassu@huawei.com

Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules the ability to check the validity of a signature against supplied data, by using user-provided or system-provided keys as trust anchor.

The new kfunc makes it possible to enforce mandatory policies, as eBPF programs might be allowed to make security decisions only based on data sources the system administrator approves.

The caller should provide the data to be verified and the signature as eBPF dynamic pointers (to minimize the number of parameters) and a bpf_key structure containing a reference to the keyring with keys trusted for signature verification, obtained from bpf_lookup_user_key() or bpf_lookup_system_key().

For bpf_key structures obtained from the former lookup function, bpf_verify_pkcs7_signature() completes the permission check deferred by that function by calling key_validate(). key_task_permission() is already called by the PKCS#7 code.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Acked-by: KP Singh kpsingh@kernel.org

---

kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 7a7023704ac2..8e2c026b0a58 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey) kfree(bkey); }

+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION +/**

• • bpf_verify_pkcs7_signature - verify a PKCS#7 signature
• • @data_ptr: data to verify
• • @sig_ptr: signature of the data
• • @trusted_keyring: keyring with keys trusted for signature

verification

• • Verify the PKCS#7 signature *sig_ptr* against the supplied

*data_ptr*

• • with keys in a keyring referenced by *trusted_keyring*.
• • Return: 0 on success, a negative value on error.
• */

+int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,

•                          struct bpf_dynptr_kern *sig_ptr,
  

•                          struct bpf_key *trusted_keyring)
  

+{

•   int ret;
  

•   if (trusted_keyring->has_ref) {
  

•           /*
  

•            * Do the permission check deferred in
  

bpf_lookup_user_key().

•            * See bpf_lookup_user_key() for more details.
  

•            *
  

•            * A call to key_task_permission() here would be
  

redundant, as

•            * it is already done by keyring_search() called by
  

•            * find_asymmetric_key().
  

•            */
  

•           ret = key_validate(trusted_keyring->key);
  

•           if (ret < 0)
  

•                   return ret;
  

•   }
  

•   return verify_pkcs7_signature(data_ptr->data,
  

•                                 bpf_dynptr_get_size(data_ptr)
  

,

•                                 sig_ptr->data,
  

•                                 bpf_dynptr_get_size(sig_ptr),
  

MIssing check for data_ptr->data == NULL before making this call? Same for sig_ptr.

Patch 3 requires the dynptrs to be initialized. Isn't enough?

Thanks

Roberto

On Wed, 2022-09-07 at 04:28 +0200, Kumar Kartikeya Dwivedi wrote:

On Tue, 6 Sept 2022 at 10:08, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

...

On Tue, 2022-09-06 at 04:57 +0200, Kumar Kartikeya Dwivedi wrote:

...

On Mon, 5 Sept 2022 at 16:35, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

...

From: Roberto Sassu roberto.sassu@huawei.com

Add the bpf_verify_pkcs7_signature() kfunc, to give eBPF security modules the ability to check the validity of a signature against supplied data, by using user-provided or system-provided keys as trust anchor.

The new kfunc makes it possible to enforce mandatory policies, as eBPF programs might be allowed to make security decisions only based on data sources the system administrator approves.

The caller should provide the data to be verified and the signature as eBPF dynamic pointers (to minimize the number of parameters) and a bpf_key structure containing a reference to the keyring with keys trusted for signature verification, obtained from bpf_lookup_user_key() or bpf_lookup_system_key().

For bpf_key structures obtained from the former lookup function, bpf_verify_pkcs7_signature() completes the permission check deferred by that function by calling key_validate(). key_task_permission() is already called by the PKCS#7 code.

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Acked-by: KP Singh kpsingh@kernel.org

---

kernel/trace/bpf_trace.c | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 7a7023704ac2..8e2c026b0a58 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1294,12 +1294,57 @@ void bpf_key_put(struct bpf_key *bkey) kfree(bkey); }

+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION +/**

• • bpf_verify_pkcs7_signature - verify a PKCS#7 signature
• • @data_ptr: data to verify
• • @sig_ptr: signature of the data
• • @trusted_keyring: keyring with keys trusted for signature

verification

• • Verify the PKCS#7 signature *sig_ptr* against the supplied

*data_ptr*

• • with keys in a keyring referenced by *trusted_keyring*.
• • Return: 0 on success, a negative value on error.
• */

+int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,

•                          struct bpf_dynptr_kern *sig_ptr,
  

•                          struct bpf_key *trusted_keyring)
  

+{

•   int ret;
  

•   if (trusted_keyring->has_ref) {
  

•           /*
  

•            * Do the permission check deferred in
  

bpf_lookup_user_key().

•            * See bpf_lookup_user_key() for more details.
  

•            *
  

•            * A call to key_task_permission() here would
  

be redundant, as

•            * it is already done by keyring_search()
  

called by

•            * find_asymmetric_key().
  

•            */
  

•           ret = key_validate(trusted_keyring->key);
  

•           if (ret < 0)
  

•                   return ret;
  

•   }
  

•   return verify_pkcs7_signature(data_ptr->data,
  

•                                 bpf_dynptr_get_size(data_
  

ptr) ,

•                                 sig_ptr->data,
  

•                                 bpf_dynptr_get_size(sig_p
  

tr),

MIssing check for data_ptr->data == NULL before making this call? Same for sig_ptr.

Patch 3 requires the dynptrs to be initialized. Isn't enough?

No, it seems even initialized dynptr can be NULL at runtime. Look at both ringbuf_submit_dynptr and ringbuf_discard_dynptr. The verifier won't know after ringbuf_reserve_dynptr whether it set it to NULL or some valid pointer.

dynptr_init is basically that stack slot is now STACK_DYNPTR, it says nothing more about the dynptr.

As far as testing this goes, you can pass invalid parameters to ringbuf_reserve_dynptr to have it set to NULL, then make sure your helper returns an error at runtime for it.

I see, thanks.

I did a quick test. Pass 1 as flags argument to bpf_dynptr_from_mem() (not supported), and see how bpf_verify_pkcs7_signature() handles it.

Everything seems good, the ASN1 parser called by pkcs7_parse_message() correctly handles zero length.

So, I will add just this test, right?

Thanks

Roberto

On Tue, 2022-09-06 at 05:15 +0200, Kumar Kartikeya Dwivedi wrote:

On Mon, 5 Sept 2022 at 16:36, Roberto Sassu roberto.sassu@huaweicloud.com wrote:

...

From: Roberto Sassu roberto.sassu@huawei.com

Add tests to ensure that only supported dynamic pointer types are accepted, that the passed argument is actually a dynamic pointer, and that the passed argument is a pointer to the stack.

The tests are currently in the deny list for s390x (JIT does not support calling kernel function).

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com

tools/testing/selftests/bpf/DENYLIST.s390x | 1 + .../bpf/prog_tests/kfunc_dynptr_param.c | 103 ++++++++++++++++++ .../bpf/progs/test_kfunc_dynptr_param.c | 57 ++++++++++ 3 files changed, 161 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c create mode 100644 tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c

diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x index 4e305baa5277..9a6dc3671c65 100644 --- a/tools/testing/selftests/bpf/DENYLIST.s390x +++ b/tools/testing/selftests/bpf/DENYLIST.s390x @@ -71,3 +71,4 @@ cgroup_hierarchical_stats # JIT does not support calling kernel f htab_update # failed to attach: ERROR: strerror_r(- 524)=22 (trampoline) lookup_key # JIT does not support calling kernel function (kfunc) verify_pkcs7_sig # JIT does not support calling kernel function (kfunc) +kfunc_dynptr_param # JIT does not support calling kernel function (kfunc) diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c new file mode 100644 index 000000000000..ea655a5c9d8b --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-2.0

+/*

• • Copyright (c) 2022 Facebook
• • Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
• • Author: Roberto Sassu roberto.sassu@huawei.com
• */

+#include <test_progs.h> +#include "test_kfunc_dynptr_param.skel.h"

+static size_t log_buf_sz = 1048576; /* 1 MB */ +static char obj_log_buf[1048576];

+static struct {

•   const char *prog_name;
  

•   const char *expected_err_msg;
  

+} kfunc_dynptr_tests[] = {

•   {"dynptr_type_not_supp",
  

•    "arg#0 pointer type STRUCT bpf_dynptr_kern points to
  

unsupported dynamic pointer type"},

•   {"not_valid_dynptr",
  

•    "arg#0 pointer type STRUCT bpf_dynptr_kern must be valid
  

and initialized"},

•   {"not_ptr_to_stack", "arg#0 pointer type STRUCT
  

bpf_dynptr_kern not to stack"}, +};

+static bool kfunc_not_supported;

+static int libbpf_print_cb(enum libbpf_print_level level, const char *fmt,

•                      va_list args)
  

+{

•   if (strcmp(fmt, "libbpf: extern (func ksym) '%s': not found
  

in kernel or module BTFs\n"))

•           return 0;
  

•   if (strcmp(va_arg(args, char *),
  

"bpf_verify_pkcs7_signature"))

•           return 0;
  

•   kfunc_not_supported = true;
  

•   return 0;
  

+}

+static void verify_fail(const char *prog_name, const char *expected_err_msg) +{

•   struct test_kfunc_dynptr_param *skel;
  

•   LIBBPF_OPTS(bpf_object_open_opts, opts);
  

•   libbpf_print_fn_t old_print_cb;
  

•   struct bpf_program *prog;
  

•   int err;
  

•   opts.kernel_log_buf = obj_log_buf;
  

•   opts.kernel_log_size = log_buf_sz;
  

•   opts.kernel_log_level = 1;
  

•   skel = test_kfunc_dynptr_param__open_opts(&opts);
  

•   if (!ASSERT_OK_PTR(skel,
  

"test_kfunc_dynptr_param__open_opts"))

•           goto cleanup;
  

•   prog = bpf_object__find_program_by_name(skel->obj,
  

prog_name);

•   if (!ASSERT_OK_PTR(prog,
  

"bpf_object__find_program_by_name"))

•           goto cleanup;
  

•   bpf_program__set_autoload(prog, true);
  

•   bpf_map__set_max_entries(skel->maps.ringbuf,
  

getpagesize());

•   kfunc_not_supported = false;
  

•   old_print_cb = libbpf_set_print(libbpf_print_cb);
  

•   err = test_kfunc_dynptr_param__load(skel);
  

•   libbpf_set_print(old_print_cb);
  

•   if (err < 0 && kfunc_not_supported) {
  

•           fprintf(stderr,
  

•             "%s:SKIP:bpf_verify_pkcs7_signature() kfunc not
  

supported\n",

•             __func__);
  

•           test__skip();
  

•           goto cleanup;
  

•   }
  

•   if (!ASSERT_ERR(err, "unexpected load success"))
  

•           goto cleanup;
  

•   if (!ASSERT_OK_PTR(strstr(obj_log_buf, expected_err_msg),
  

"expected_err_msg")) {

•           fprintf(stderr, "Expected err_msg: %s\n",
  

expected_err_msg);

•           fprintf(stderr, "Verifier output: %s\n",
  

obj_log_buf);

•   }
  

+cleanup:

•   test_kfunc_dynptr_param__destroy(skel);
  

+}

+void test_kfunc_dynptr_param(void) +{

•   int i;
  

•   for (i = 0; i < ARRAY_SIZE(kfunc_dynptr_tests); i++) {
  

•           if
  

(!test__start_subtest(kfunc_dynptr_tests[i].prog_name))

•                   continue;
  

•           verify_fail(kfunc_dynptr_tests[i].prog_name,
  

•                       kfunc_dynptr_tests[i].expected_err_msg)
  

;

•   }
  

+} diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c new file mode 100644 index 000000000000..2f09f91a1576 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0

+/*

• • Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
• • Author: Roberto Sassu roberto.sassu@huawei.com
• */

+#include "vmlinux.h" +#include <errno.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h>

+struct bpf_dynptr {

•   __u64 :64;
  

•   __u64 :64;
  

+} __attribute__((aligned(8)));

+extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,

•                                 struct bpf_dynptr *sig_ptr,
  

•                                 struct bpf_key
  

*trusted_keyring) __ksym;

+struct {

•   __uint(type, BPF_MAP_TYPE_RINGBUF);
  

+} ringbuf SEC(".maps");

+char _license[] SEC("license") = "GPL";

+SEC("?lsm.s/bpf") +int BPF_PROG(dynptr_type_not_supp, int cmd, union bpf_attr *attr,

•        unsigned int size)
  

+{

•   char write_data[64] = "hello there, world!!";
  

•   struct bpf_dynptr ptr;
  

•   bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(write_data), 0,
  

&ptr);

•   return bpf_verify_pkcs7_signature(&ptr, &ptr, NULL);
  

+}

+SEC("?lsm.s/bpf") +int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) +{

•   unsigned long val;
  

•   return bpf_verify_pkcs7_signature((struct bpf_dynptr
  

*)&val,

•                                     (struct bpf_dynptr
  

*)&val, NULL); +}

+SEC("?lsm.s/bpf") +int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) +{

•   unsigned long val;
  

•   return bpf_verify_pkcs7_signature((struct bpf_dynptr *)val,
  

•                                     (struct bpf_dynptr *)val,
  

NULL);

Please also include a test where you cause the dynptr to be set to NULL, e.g. by passing invalid stuff to ringbuf_reserve_dynptr, and then try to pass it to bpf_verify_pkc7_signature.

Uhm, bpf_ringbuf_reserve_dynptr() is expecting a valid map. How else I can achieve it?

Thanks

Roberto

From: Roberto Sassu roberto.sassu@huawei.com

Add tests to ensure that only supported dynamic pointer types are accepted, that the passed argument is actually a dynamic pointer, that the passed argument is a pointer to the stack, and that bpf_verify_pkcs7_signature() correctly handles dynamic pointers with data set to NULL.

The tests are currently in the deny list for s390x (JIT does not support calling kernel function).

Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- tools/testing/selftests/bpf/DENYLIST.s390x | 1 + .../bpf/prog_tests/kfunc_dynptr_param.c | 164 ++++++++++++++++++ .../bpf/progs/test_kfunc_dynptr_param.c | 99 +++++++++++ 3 files changed, 264 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c create mode 100644 tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c

diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x index 4e305baa5277..9a6dc3671c65 100644 --- a/tools/testing/selftests/bpf/DENYLIST.s390x +++ b/tools/testing/selftests/bpf/DENYLIST.s390x @@ -71,3 +71,4 @@ cgroup_hierarchical_stats # JIT does not support calling kernel f htab_update # failed to attach: ERROR: strerror_r(-524)=22 (trampoline) lookup_key # JIT does not support calling kernel function (kfunc) verify_pkcs7_sig # JIT does not support calling kernel function (kfunc) +kfunc_dynptr_param # JIT does not support calling kernel function (kfunc) diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c new file mode 100644 index 000000000000..c210657d4d0a --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright (c) 2022 Facebook + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + */ + +#include <test_progs.h> +#include "test_kfunc_dynptr_param.skel.h" + +static size_t log_buf_sz = 1048576; /* 1 MB */ +static char obj_log_buf[1048576]; + +static struct { + const char *prog_name; + const char *expected_verifier_err_msg; + int expected_runtime_err; +} kfunc_dynptr_tests[] = { + {"dynptr_type_not_supp", + "arg#0 pointer type STRUCT bpf_dynptr_kern points to unsupported dynamic pointer type", 0}, + {"not_valid_dynptr", + "arg#0 pointer type STRUCT bpf_dynptr_kern must be valid and initialized", 0}, + {"not_ptr_to_stack", "arg#0 pointer type STRUCT bpf_dynptr_kern not to stack", 0}, + {"dynptr_data_null", NULL, -EBADMSG}, +}; + +static bool kfunc_not_supported; + +static int libbpf_print_cb(enum libbpf_print_level level, const char *fmt, + va_list args) +{ + if (strcmp(fmt, "libbpf: extern (func ksym) '%s': not found in kernel or module BTFs\n")) + return 0; + + if (strcmp(va_arg(args, char *), "bpf_verify_pkcs7_signature")) + return 0; + + kfunc_not_supported = true; + return 0; +} + +static void verify_fail(const char *prog_name, const char *expected_err_msg) +{ + struct test_kfunc_dynptr_param *skel; + LIBBPF_OPTS(bpf_object_open_opts, opts); + libbpf_print_fn_t old_print_cb; + struct bpf_program *prog; + int err; + + opts.kernel_log_buf = obj_log_buf; + opts.kernel_log_size = log_buf_sz; + opts.kernel_log_level = 1; + + skel = test_kfunc_dynptr_param__open_opts(&opts); + if (!ASSERT_OK_PTR(skel, "test_kfunc_dynptr_param__open_opts")) + goto cleanup; + + prog = bpf_object__find_program_by_name(skel->obj, prog_name); + if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) + goto cleanup; + + bpf_program__set_autoload(prog, true); + + bpf_map__set_max_entries(skel->maps.ringbuf, getpagesize()); + + kfunc_not_supported = false; + + old_print_cb = libbpf_set_print(libbpf_print_cb); + err = test_kfunc_dynptr_param__load(skel); + libbpf_set_print(old_print_cb); + + if (err < 0 && kfunc_not_supported) { + fprintf(stderr, + "%s:SKIP:bpf_verify_pkcs7_signature() kfunc not supported\n", + __func__); + test__skip(); + goto cleanup; + } + + if (!ASSERT_ERR(err, "unexpected load success")) + goto cleanup; + + if (!ASSERT_OK_PTR(strstr(obj_log_buf, expected_err_msg), "expected_err_msg")) { + fprintf(stderr, "Expected err_msg: %s\n", expected_err_msg); + fprintf(stderr, "Verifier output: %s\n", obj_log_buf); + } + +cleanup: + test_kfunc_dynptr_param__destroy(skel); +} + +static void verify_success(const char *prog_name, int expected_runtime_err) +{ + struct test_kfunc_dynptr_param *skel; + libbpf_print_fn_t old_print_cb; + struct bpf_program *prog; + struct bpf_link *link; + __u32 next_id; + int err; + + skel = test_kfunc_dynptr_param__open(); + if (!ASSERT_OK_PTR(skel, "test_kfunc_dynptr_param__open")) + return; + + skel->bss->pid = getpid(); + + bpf_map__set_max_entries(skel->maps.ringbuf, getpagesize()); + + kfunc_not_supported = false; + + old_print_cb = libbpf_set_print(libbpf_print_cb); + err = test_kfunc_dynptr_param__load(skel); + libbpf_set_print(old_print_cb); + + if (err < 0 && kfunc_not_supported) { + fprintf(stderr, + "%s:SKIP:bpf_verify_pkcs7_signature() kfunc not supported\n", + __func__); + test__skip(); + goto cleanup; + } + + if (!ASSERT_OK(err, "test_kfunc_dynptr_param__load")) + goto cleanup; + + prog = bpf_object__find_program_by_name(skel->obj, prog_name); + if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) + goto cleanup; + + link = bpf_program__attach(prog); + if (!ASSERT_OK_PTR(link, "bpf_program__attach")) + goto cleanup; + + err = bpf_prog_get_next_id(0, &next_id); + + bpf_link__destroy(link); + + if (!ASSERT_OK(err, "bpf_prog_get_next_id")) + goto cleanup; + + ASSERT_EQ(skel->bss->err, expected_runtime_err, "err"); + +cleanup: + test_kfunc_dynptr_param__destroy(skel); +} + +void test_kfunc_dynptr_param(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(kfunc_dynptr_tests); i++) { + if (!test__start_subtest(kfunc_dynptr_tests[i].prog_name)) + continue; + + if (kfunc_dynptr_tests[i].expected_verifier_err_msg) + verify_fail(kfunc_dynptr_tests[i].prog_name, + kfunc_dynptr_tests[i].expected_verifier_err_msg); + else + verify_success(kfunc_dynptr_tests[i].prog_name, + kfunc_dynptr_tests[i].expected_runtime_err); + } +} diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c new file mode 100644 index 000000000000..d7d39ea009a7 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + */ + +#include "vmlinux.h" +#include <errno.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +struct bpf_dynptr { + __u64 :64; + __u64 :64; +} __attribute__((aligned(8))); + +extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym; +extern void bpf_key_put(struct bpf_key *key) __ksym; +extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr, + struct bpf_dynptr *sig_ptr, + struct bpf_key *trusted_keyring) __ksym; + +struct { + __uint(type, BPF_MAP_TYPE_RINGBUF); +} ringbuf SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u32); +} array_map SEC(".maps"); + +int err, pid; + +char _license[] SEC("license") = "GPL"; + +SEC("?lsm.s/bpf") +int BPF_PROG(dynptr_type_not_supp, int cmd, union bpf_attr *attr, + unsigned int size) +{ + char write_data[64] = "hello there, world!!"; + struct bpf_dynptr ptr; + + bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(write_data), 0, &ptr); + + return bpf_verify_pkcs7_signature(&ptr, &ptr, NULL); +} + +SEC("?lsm.s/bpf") +int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size) +{ + unsigned long val; + + return bpf_verify_pkcs7_signature((struct bpf_dynptr *)&val, + (struct bpf_dynptr *)&val, NULL); +} + +SEC("?lsm.s/bpf") +int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size) +{ + unsigned long val; + + return bpf_verify_pkcs7_signature((struct bpf_dynptr *)val, + (struct bpf_dynptr *)val, NULL); +} + +SEC("lsm.s/bpf") +int BPF_PROG(dynptr_data_null, int cmd, union bpf_attr *attr, unsigned int size) +{ + struct bpf_key *trusted_keyring; + struct bpf_dynptr ptr; + __u32 *value; + int ret, zero = 0; + + if (bpf_get_current_pid_tgid() >> 32 != pid) + return 0; + + value = bpf_map_lookup_elem(&array_map, &zero); + if (!value) + return 0; + + /* Pass invalid flags. */ + ret = bpf_dynptr_from_mem(value, sizeof(*value), 1, &ptr); + if (ret != -EINVAL) + return 0; + + trusted_keyring = bpf_lookup_system_key(0); + if (!trusted_keyring) + return 0; + + err = bpf_verify_pkcs7_signature(&ptr, &ptr, trusted_keyring); + + bpf_key_put(trusted_keyring); + + return 0; +}