| Message ID | 20190910115527.5235-15-kpsingh@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Kernel Runtime Security Instrumentation | expand |
On 9/10/19 12:55 PM, KP Singh wrote: > From: KP Singh <kpsingh@google.com> > > Adds a callback which is called when a new program is attached > to a hook. The callback registered by the process_exection hook > checks if a program that has calls to a helper that requires pages to > be pinned (eg. krsi_get_env_var). > > Signed-off-by: KP Singh <kpsingh@google.com> > --- > include/linux/krsi.h | 1 + > security/krsi/include/hooks.h | 5 ++- > security/krsi/include/krsi_init.h | 7 ++++ > security/krsi/krsi.c | 62 ++++++++++++++++++++++++++++--- > security/krsi/ops.c | 10 ++++- > 5 files changed, 77 insertions(+), 8 deletions(-) > > diff --git a/include/linux/krsi.h b/include/linux/krsi.h > index c7d1790d0c1f..e443d0309764 100644 > --- a/include/linux/krsi.h > +++ b/include/linux/krsi.h > @@ -7,6 +7,7 @@ > > #ifdef CONFIG_SECURITY_KRSI > int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog); > +extern const struct bpf_func_proto krsi_get_env_var_proto; > #else > static inline int krsi_prog_attach(const union bpf_attr *attr, > struct bpf_prog *prog) > diff --git a/security/krsi/include/hooks.h b/security/krsi/include/hooks.h > index e070c452b5de..38293125ff99 100644 > --- a/security/krsi/include/hooks.h > +++ b/security/krsi/include/hooks.h > @@ -8,7 +8,7 @@ > * > * Format: > * > - * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN) > + * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN, CALLBACK) > * > * KRSI adds one layer of indirection between the name of the hook and the name > * it exposes to the userspace in Security FS to prevent the userspace from > @@ -18,4 +18,5 @@ > KRSI_HOOK_INIT(PROCESS_EXECUTION, > process_execution, > bprm_check_security, > - krsi_process_execution) > + krsi_process_execution, > + krsi_process_execution_cb) > diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h > index 6152847c3b08..99801d5b273a 100644 > --- a/security/krsi/include/krsi_init.h > +++ b/security/krsi/include/krsi_init.h > @@ -31,6 +31,8 @@ struct krsi_ctx { > }; > }; > > +typedef int (*krsi_prog_attach_t) (struct bpf_prog_array *); > + > /* > * The LSM creates one file per hook. > * > @@ -61,6 +63,11 @@ struct krsi_hook { > * The eBPF programs that are attached to this hook. > */ > struct bpf_prog_array __rcu *progs; > + /* > + * The attach callback is called before a new program is attached > + * to the hook and is passed the updated bpf_prog_array as an argument. > + */ > + krsi_prog_attach_t attach_callback; > }; > > extern struct krsi_hook krsi_hooks_list[]; > diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c > index 00a7150c1b22..a4443d7aa150 100644 > --- a/security/krsi/krsi.c > +++ b/security/krsi/krsi.c > @@ -5,15 +5,65 @@ > #include <linux/bpf.h> > #include <linux/binfmts.h> > #include <linux/highmem.h> > +#include <linux/krsi.h> > #include <linux/mm.h> > > #include "krsi_init.h" > > +/* > + * need_arg_pages is only updated in bprm_check_security_cb > + * when a mutex on krsi_hook for bprm_check_security is already > + * held. need_arg_pages avoids pinning pages when no program > + * that needs them is attached to the hook. > + */ > +static bool need_arg_pages; > + > +/* > + * Checks if the instruction is a BPF_CALL to an eBPF helper located > + * at the given address. > + */ > +static inline bool bpf_is_call_to_func(struct bpf_insn *insn, > + void *func_addr) > +{ > + u8 opcode = BPF_OP(insn->code); > + > + if (opcode != BPF_CALL) > + return false; > + > + if (insn->src_reg == BPF_PSEUDO_CALL) > + return false; > + > + /* > + * The BPF verifier updates the value of insn->imm from the > + * enum bpf_func_id to the offset of the address of helper > + * from the __bpf_call_base. > + */ > + return __bpf_call_base + insn->imm == func_addr; In how many cases, krsi program does not have krsi_get_env_var() helper? > +} > + > +static int krsi_process_execution_cb(struct bpf_prog_array *array) > +{ > + struct bpf_prog_array_item *item = array->items; > + struct bpf_prog *p; > + const struct bpf_func_proto *proto = &krsi_get_env_var_proto; > + int i; > + > + while ((p = READ_ONCE(item->prog))) { > + for (i = 0; i < p->len; i++) { > + if (bpf_is_call_to_func(&p->insnsi[i], proto->func)) > + need_arg_pages = true; > + } > + item++; > + } > + return 0; > +} > + > struct krsi_hook krsi_hooks_list[] = { > - #define KRSI_HOOK_INIT(TYPE, NAME, H, I) \ > + #define KRSI_HOOK_INIT(TYPE, NAME, H, I, CB) \ > [TYPE] = { \ > .h_type = TYPE, \ > .name = #NAME, \ > + .attach_callback = CB, \ > }, > #include "hooks.h" > #undef KRSI_HOOK_INIT > @@ -75,9 +125,11 @@ static int krsi_process_execution(struct linux_binprm *bprm) > .bprm = bprm, > }; > > - ret = pin_arg_pages(&ctx.bprm_ctx); > - if (ret < 0) > - goto out_arg_pages; > + if (READ_ONCE(need_arg_pages)) { > + ret = pin_arg_pages(&ctx.bprm_ctx); > + if (ret < 0) > + goto out_arg_pages; > + } > > ret = krsi_run_progs(PROCESS_EXECUTION, &ctx); > kfree(ctx.bprm_ctx.arg_pages); > @@ -87,7 +139,7 @@ static int krsi_process_execution(struct linux_binprm *bprm) > } > > static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = { > - #define KRSI_HOOK_INIT(T, N, HOOK, IMPL) LSM_HOOK_INIT(HOOK, IMPL), > + #define KRSI_HOOK_INIT(T, N, HOOK, IMPL, CB) LSM_HOOK_INIT(HOOK, IMPL), > #include "hooks.h" > #undef KRSI_HOOK_INIT > }; > diff --git a/security/krsi/ops.c b/security/krsi/ops.c > index 1db94dfaac15..2de682371eff 100644 > --- a/security/krsi/ops.c > +++ b/security/krsi/ops.c > @@ -139,6 +139,14 @@ int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog) > goto unlock; > } > > + if (h->attach_callback) { > + ret = h->attach_callback(new_array); > + if (ret < 0) { > + bpf_prog_array_free(new_array); > + goto unlock; > + } > + } > + > rcu_assign_pointer(h->progs, new_array); > bpf_prog_array_free(old_array); > > @@ -278,7 +286,7 @@ BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32, n_size, > return get_env_var(ctx, name, dest, n_size, size); > } > > -static const struct bpf_func_proto krsi_get_env_var_proto = { > +const struct bpf_func_proto krsi_get_env_var_proto = { > .func = krsi_get_env_var, > .gpl_only = true, > .ret_type = RET_INTEGER, >
On 15-Sep 00:33, Yonghong Song wrote: > > > On 9/10/19 12:55 PM, KP Singh wrote: > > From: KP Singh <kpsingh@google.com> > > > > Adds a callback which is called when a new program is attached > > to a hook. The callback registered by the process_exection hook > > checks if a program that has calls to a helper that requires pages to > > be pinned (eg. krsi_get_env_var). > > > > Signed-off-by: KP Singh <kpsingh@google.com> > > --- > > include/linux/krsi.h | 1 + > > security/krsi/include/hooks.h | 5 ++- > > security/krsi/include/krsi_init.h | 7 ++++ > > security/krsi/krsi.c | 62 ++++++++++++++++++++++++++++--- > > security/krsi/ops.c | 10 ++++- > > 5 files changed, 77 insertions(+), 8 deletions(-) > > > > diff --git a/include/linux/krsi.h b/include/linux/krsi.h > > index c7d1790d0c1f..e443d0309764 100644 > > --- a/include/linux/krsi.h > > +++ b/include/linux/krsi.h > > @@ -7,6 +7,7 @@ > > > > #ifdef CONFIG_SECURITY_KRSI > > int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog); > > +extern const struct bpf_func_proto krsi_get_env_var_proto; > > #else > > static inline int krsi_prog_attach(const union bpf_attr *attr, > > struct bpf_prog *prog) > > diff --git a/security/krsi/include/hooks.h b/security/krsi/include/hooks.h > > index e070c452b5de..38293125ff99 100644 > > --- a/security/krsi/include/hooks.h > > +++ b/security/krsi/include/hooks.h > > @@ -8,7 +8,7 @@ > > * > > * Format: > > * > > - * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN) > > + * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN, CALLBACK) > > * > > * KRSI adds one layer of indirection between the name of the hook and the name > > * it exposes to the userspace in Security FS to prevent the userspace from > > @@ -18,4 +18,5 @@ > > KRSI_HOOK_INIT(PROCESS_EXECUTION, > > process_execution, > > bprm_check_security, > > - krsi_process_execution) > > + krsi_process_execution, > > + krsi_process_execution_cb) > > diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h > > index 6152847c3b08..99801d5b273a 100644 > > --- a/security/krsi/include/krsi_init.h > > +++ b/security/krsi/include/krsi_init.h > > @@ -31,6 +31,8 @@ struct krsi_ctx { > > }; > > }; > > > > +typedef int (*krsi_prog_attach_t) (struct bpf_prog_array *); > > + > > /* > > * The LSM creates one file per hook. > > * > > @@ -61,6 +63,11 @@ struct krsi_hook { > > * The eBPF programs that are attached to this hook. > > */ > > struct bpf_prog_array __rcu *progs; > > + /* > > + * The attach callback is called before a new program is attached > > + * to the hook and is passed the updated bpf_prog_array as an argument. > > + */ > > + krsi_prog_attach_t attach_callback; > > }; > > > > extern struct krsi_hook krsi_hooks_list[]; > > diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c > > index 00a7150c1b22..a4443d7aa150 100644 > > --- a/security/krsi/krsi.c > > +++ b/security/krsi/krsi.c > > @@ -5,15 +5,65 @@ > > #include <linux/bpf.h> > > #include <linux/binfmts.h> > > #include <linux/highmem.h> > > +#include <linux/krsi.h> > > #include <linux/mm.h> > > > > #include "krsi_init.h" > > > > +/* > > + * need_arg_pages is only updated in bprm_check_security_cb > > + * when a mutex on krsi_hook for bprm_check_security is already > > + * held. need_arg_pages avoids pinning pages when no program > > + * that needs them is attached to the hook. > > + */ > > +static bool need_arg_pages; > > + > > +/* > > + * Checks if the instruction is a BPF_CALL to an eBPF helper located > > + * at the given address. > > + */ > > +static inline bool bpf_is_call_to_func(struct bpf_insn *insn, > > + void *func_addr) > > +{ > > + u8 opcode = BPF_OP(insn->code); > > + > > + if (opcode != BPF_CALL) > > + return false; > > + > > + if (insn->src_reg == BPF_PSEUDO_CALL) > > + return false; > > + > > + /* > > + * The BPF verifier updates the value of insn->imm from the > > + * enum bpf_func_id to the offset of the address of helper > > + * from the __bpf_call_base. > > + */ > > + return __bpf_call_base + insn->imm == func_addr; > > In how many cases, krsi program does not have krsi_get_env_var() helper? It depends, if the user does not choose to use log environment variables or use the the value as a part of their MAC policy, the pinning of the pages is not needed. Also, the pinning is needed since eBPF helpers cannot run a non-atomic context. It would not be needed if "sleepable eBPF" becomes a thing. - KP > > > +} > > + > > +static int krsi_process_execution_cb(struct bpf_prog_array *array) > > +{ > > + struct bpf_prog_array_item *item = array->items; > > + struct bpf_prog *p; > > + const struct bpf_func_proto *proto = &krsi_get_env_var_proto; > > + int i; > > + > > + while ((p = READ_ONCE(item->prog))) { > > + for (i = 0; i < p->len; i++) { > > + if (bpf_is_call_to_func(&p->insnsi[i], proto->func)) > > + need_arg_pages = true; > > + } > > + item++; > > + } > > + return 0; > > +} > > + > > struct krsi_hook krsi_hooks_list[] = { > > - #define KRSI_HOOK_INIT(TYPE, NAME, H, I) \ > > + #define KRSI_HOOK_INIT(TYPE, NAME, H, I, CB) \ > > [TYPE] = { \ > > .h_type = TYPE, \ > > .name = #NAME, \ > > + .attach_callback = CB, \ > > }, > > #include "hooks.h" > > #undef KRSI_HOOK_INIT > > @@ -75,9 +125,11 @@ static int krsi_process_execution(struct linux_binprm *bprm) > > .bprm = bprm, > > }; > > > > - ret = pin_arg_pages(&ctx.bprm_ctx); > > - if (ret < 0) > > - goto out_arg_pages; > > + if (READ_ONCE(need_arg_pages)) { > > + ret = pin_arg_pages(&ctx.bprm_ctx); > > + if (ret < 0) > > + goto out_arg_pages; > > + } > > > > ret = krsi_run_progs(PROCESS_EXECUTION, &ctx); > > kfree(ctx.bprm_ctx.arg_pages); > > @@ -87,7 +139,7 @@ static int krsi_process_execution(struct linux_binprm *bprm) > > } > > > > static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = { > > - #define KRSI_HOOK_INIT(T, N, HOOK, IMPL) LSM_HOOK_INIT(HOOK, IMPL), > > + #define KRSI_HOOK_INIT(T, N, HOOK, IMPL, CB) LSM_HOOK_INIT(HOOK, IMPL), > > #include "hooks.h" > > #undef KRSI_HOOK_INIT > > }; > > diff --git a/security/krsi/ops.c b/security/krsi/ops.c > > index 1db94dfaac15..2de682371eff 100644 > > --- a/security/krsi/ops.c > > +++ b/security/krsi/ops.c > > @@ -139,6 +139,14 @@ int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog) > > goto unlock; > > } > > > > + if (h->attach_callback) { > > + ret = h->attach_callback(new_array); > > + if (ret < 0) { > > + bpf_prog_array_free(new_array); > > + goto unlock; > > + } > > + } > > + > > rcu_assign_pointer(h->progs, new_array); > > bpf_prog_array_free(old_array); > > > > @@ -278,7 +286,7 @@ BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32, n_size, > > return get_env_var(ctx, name, dest, n_size, size); > > } > > > > -static const struct bpf_func_proto krsi_get_env_var_proto = { > > +const struct bpf_func_proto krsi_get_env_var_proto = { > > .func = krsi_get_env_var, > > .gpl_only = true, > > .ret_type = RET_INTEGER, > >
On 9/15/19 2:40 AM, KP Singh wrote: > On 15-Sep 00:33, Yonghong Song wrote: >> >> >> On 9/10/19 12:55 PM, KP Singh wrote: >>> From: KP Singh <kpsingh@google.com> >>> >>> Adds a callback which is called when a new program is attached >>> to a hook. The callback registered by the process_exection hook >>> checks if a program that has calls to a helper that requires pages to >>> be pinned (eg. krsi_get_env_var). >>> >>> Signed-off-by: KP Singh <kpsingh@google.com> >>> --- >>> include/linux/krsi.h | 1 + >>> security/krsi/include/hooks.h | 5 ++- >>> security/krsi/include/krsi_init.h | 7 ++++ >>> security/krsi/krsi.c | 62 ++++++++++++++++++++++++++++--- >>> security/krsi/ops.c | 10 ++++- >>> 5 files changed, 77 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/linux/krsi.h b/include/linux/krsi.h >>> index c7d1790d0c1f..e443d0309764 100644 >>> --- a/include/linux/krsi.h >>> +++ b/include/linux/krsi.h >>> @@ -7,6 +7,7 @@ >>> >>> #ifdef CONFIG_SECURITY_KRSI >>> int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog); >>> +extern const struct bpf_func_proto krsi_get_env_var_proto; >>> #else >>> static inline int krsi_prog_attach(const union bpf_attr *attr, >>> struct bpf_prog *prog) >>> diff --git a/security/krsi/include/hooks.h b/security/krsi/include/hooks.h >>> index e070c452b5de..38293125ff99 100644 >>> --- a/security/krsi/include/hooks.h >>> +++ b/security/krsi/include/hooks.h >>> @@ -8,7 +8,7 @@ >>> * >>> * Format: >>> * >>> - * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN) >>> + * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN, CALLBACK) >>> * >>> * KRSI adds one layer of indirection between the name of the hook and the name >>> * it exposes to the userspace in Security FS to prevent the userspace from >>> @@ -18,4 +18,5 @@ >>> KRSI_HOOK_INIT(PROCESS_EXECUTION, >>> process_execution, >>> bprm_check_security, >>> - krsi_process_execution) >>> + krsi_process_execution, >>> + krsi_process_execution_cb) >>> diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h >>> index 6152847c3b08..99801d5b273a 100644 >>> --- a/security/krsi/include/krsi_init.h >>> +++ b/security/krsi/include/krsi_init.h >>> @@ -31,6 +31,8 @@ struct krsi_ctx { >>> }; >>> }; >>> >>> +typedef int (*krsi_prog_attach_t) (struct bpf_prog_array *); >>> + >>> /* >>> * The LSM creates one file per hook. >>> * >>> @@ -61,6 +63,11 @@ struct krsi_hook { >>> * The eBPF programs that are attached to this hook. >>> */ >>> struct bpf_prog_array __rcu *progs; >>> + /* >>> + * The attach callback is called before a new program is attached >>> + * to the hook and is passed the updated bpf_prog_array as an argument. >>> + */ >>> + krsi_prog_attach_t attach_callback; >>> }; >>> >>> extern struct krsi_hook krsi_hooks_list[]; >>> diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c >>> index 00a7150c1b22..a4443d7aa150 100644 >>> --- a/security/krsi/krsi.c >>> +++ b/security/krsi/krsi.c >>> @@ -5,15 +5,65 @@ >>> #include <linux/bpf.h> >>> #include <linux/binfmts.h> >>> #include <linux/highmem.h> >>> +#include <linux/krsi.h> >>> #include <linux/mm.h> >>> >>> #include "krsi_init.h" >>> >>> +/* >>> + * need_arg_pages is only updated in bprm_check_security_cb >>> + * when a mutex on krsi_hook for bprm_check_security is already >>> + * held. need_arg_pages avoids pinning pages when no program >>> + * that needs them is attached to the hook. >>> + */ >>> +static bool need_arg_pages; >>> + >>> +/* >>> + * Checks if the instruction is a BPF_CALL to an eBPF helper located >>> + * at the given address. >>> + */ >>> +static inline bool bpf_is_call_to_func(struct bpf_insn *insn, >>> + void *func_addr) >>> +{ >>> + u8 opcode = BPF_OP(insn->code); >>> + >>> + if (opcode != BPF_CALL) >>> + return false; >>> + >>> + if (insn->src_reg == BPF_PSEUDO_CALL) >>> + return false; >>> + >>> + /* >>> + * The BPF verifier updates the value of insn->imm from the >>> + * enum bpf_func_id to the offset of the address of helper >>> + * from the __bpf_call_base. >>> + */ >>> + return __bpf_call_base + insn->imm == func_addr; >> >> In how many cases, krsi program does not have krsi_get_env_var() helper? > > It depends, if the user does not choose to use log environment > variables or use the the value as a part of their MAC policy, the > pinning of the pages is not needed. Thanks. I just want to know whether we want to optimize such cases. I am not a security expert, so I am okay with whatever decision you made. > > Also, the pinning is needed since eBPF helpers cannot run a non-atomic > context. It would not be needed if "sleepable eBPF" becomes a thing. Indeed. 'sleepable BPF' might be also a good thing for realtime linux. Some works need to address bpf area spin lock, per cpu variables, etc. before that can happen. > > - KP > >> >>> +} >>> + >>> +static int krsi_process_execution_cb(struct bpf_prog_array *array) >>> +{ >>> + struct bpf_prog_array_item *item = array->items; >>> + struct bpf_prog *p; >>> + const struct bpf_func_proto *proto = &krsi_get_env_var_proto; >>> + int i; >>> + >>> + while ((p = READ_ONCE(item->prog))) { >>> + for (i = 0; i < p->len; i++) { >>> + if (bpf_is_call_to_func(&p->insnsi[i], proto->func)) >>> + need_arg_pages = true; >>> + } >>> + item++; >>> + } >>> + return 0; >>> +} >>> + >>> struct krsi_hook krsi_hooks_list[] = { >>> - #define KRSI_HOOK_INIT(TYPE, NAME, H, I) \ >>> + #define KRSI_HOOK_INIT(TYPE, NAME, H, I, CB) \ >>> [TYPE] = { \ >>> .h_type = TYPE, \ >>> .name = #NAME, \ >>> + .attach_callback = CB, \ >>> }, >>> #include "hooks.h" >>> #undef KRSI_HOOK_INIT >>> @@ -75,9 +125,11 @@ static int krsi_process_execution(struct linux_binprm *bprm) >>> .bprm = bprm, >>> }; >>> >>> - ret = pin_arg_pages(&ctx.bprm_ctx); >>> - if (ret < 0) >>> - goto out_arg_pages; >>> + if (READ_ONCE(need_arg_pages)) { >>> + ret = pin_arg_pages(&ctx.bprm_ctx); >>> + if (ret < 0) >>> + goto out_arg_pages; >>> + } >>> >>> ret = krsi_run_progs(PROCESS_EXECUTION, &ctx); >>> kfree(ctx.bprm_ctx.arg_pages); >>> @@ -87,7 +139,7 @@ static int krsi_process_execution(struct linux_binprm *bprm) >>> } >>> >>> static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = { >>> - #define KRSI_HOOK_INIT(T, N, HOOK, IMPL) LSM_HOOK_INIT(HOOK, IMPL), >>> + #define KRSI_HOOK_INIT(T, N, HOOK, IMPL, CB) LSM_HOOK_INIT(HOOK, IMPL), >>> #include "hooks.h" >>> #undef KRSI_HOOK_INIT >>> }; >>> diff --git a/security/krsi/ops.c b/security/krsi/ops.c >>> index 1db94dfaac15..2de682371eff 100644 >>> --- a/security/krsi/ops.c >>> +++ b/security/krsi/ops.c >>> @@ -139,6 +139,14 @@ int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog) >>> goto unlock; >>> } >>> >>> + if (h->attach_callback) { >>> + ret = h->attach_callback(new_array); >>> + if (ret < 0) { >>> + bpf_prog_array_free(new_array); >>> + goto unlock; >>> + } >>> + } >>> + >>> rcu_assign_pointer(h->progs, new_array); >>> bpf_prog_array_free(old_array); >>> >>> @@ -278,7 +286,7 @@ BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32, n_size, >>> return get_env_var(ctx, name, dest, n_size, size); >>> } >>> >>> -static const struct bpf_func_proto krsi_get_env_var_proto = { >>> +const struct bpf_func_proto krsi_get_env_var_proto = { >>> .func = krsi_get_env_var, >>> .gpl_only = true, >>> .ret_type = RET_INTEGER, >>>
diff --git a/include/linux/krsi.h b/include/linux/krsi.h index c7d1790d0c1f..e443d0309764 100644 --- a/include/linux/krsi.h +++ b/include/linux/krsi.h @@ -7,6 +7,7 @@ #ifdef CONFIG_SECURITY_KRSI int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog); +extern const struct bpf_func_proto krsi_get_env_var_proto; #else static inline int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog) diff --git a/security/krsi/include/hooks.h b/security/krsi/include/hooks.h index e070c452b5de..38293125ff99 100644 --- a/security/krsi/include/hooks.h +++ b/security/krsi/include/hooks.h @@ -8,7 +8,7 @@ * * Format: * - * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN) + * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN, CALLBACK) * * KRSI adds one layer of indirection between the name of the hook and the name * it exposes to the userspace in Security FS to prevent the userspace from @@ -18,4 +18,5 @@ KRSI_HOOK_INIT(PROCESS_EXECUTION, process_execution, bprm_check_security, - krsi_process_execution) + krsi_process_execution, + krsi_process_execution_cb) diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h index 6152847c3b08..99801d5b273a 100644 --- a/security/krsi/include/krsi_init.h +++ b/security/krsi/include/krsi_init.h @@ -31,6 +31,8 @@ struct krsi_ctx { }; }; +typedef int (*krsi_prog_attach_t) (struct bpf_prog_array *); + /* * The LSM creates one file per hook. * @@ -61,6 +63,11 @@ struct krsi_hook { * The eBPF programs that are attached to this hook. */ struct bpf_prog_array __rcu *progs; + /* + * The attach callback is called before a new program is attached + * to the hook and is passed the updated bpf_prog_array as an argument. + */ + krsi_prog_attach_t attach_callback; }; extern struct krsi_hook krsi_hooks_list[]; diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c index 00a7150c1b22..a4443d7aa150 100644 --- a/security/krsi/krsi.c +++ b/security/krsi/krsi.c @@ -5,15 +5,65 @@ #include <linux/bpf.h> #include <linux/binfmts.h> #include <linux/highmem.h> +#include <linux/krsi.h> #include <linux/mm.h> #include "krsi_init.h" +/* + * need_arg_pages is only updated in bprm_check_security_cb + * when a mutex on krsi_hook for bprm_check_security is already + * held. need_arg_pages avoids pinning pages when no program + * that needs them is attached to the hook. + */ +static bool need_arg_pages; + +/* + * Checks if the instruction is a BPF_CALL to an eBPF helper located + * at the given address. + */ +static inline bool bpf_is_call_to_func(struct bpf_insn *insn, + void *func_addr) +{ + u8 opcode = BPF_OP(insn->code); + + if (opcode != BPF_CALL) + return false; + + if (insn->src_reg == BPF_PSEUDO_CALL) + return false; + + /* + * The BPF verifier updates the value of insn->imm from the + * enum bpf_func_id to the offset of the address of helper + * from the __bpf_call_base. + */ + return __bpf_call_base + insn->imm == func_addr; +} + +static int krsi_process_execution_cb(struct bpf_prog_array *array) +{ + struct bpf_prog_array_item *item = array->items; + struct bpf_prog *p; + const struct bpf_func_proto *proto = &krsi_get_env_var_proto; + int i; + + while ((p = READ_ONCE(item->prog))) { + for (i = 0; i < p->len; i++) { + if (bpf_is_call_to_func(&p->insnsi[i], proto->func)) + need_arg_pages = true; + } + item++; + } + return 0; +} + struct krsi_hook krsi_hooks_list[] = { - #define KRSI_HOOK_INIT(TYPE, NAME, H, I) \ + #define KRSI_HOOK_INIT(TYPE, NAME, H, I, CB) \ [TYPE] = { \ .h_type = TYPE, \ .name = #NAME, \ + .attach_callback = CB, \ }, #include "hooks.h" #undef KRSI_HOOK_INIT @@ -75,9 +125,11 @@ static int krsi_process_execution(struct linux_binprm *bprm) .bprm = bprm, }; - ret = pin_arg_pages(&ctx.bprm_ctx); - if (ret < 0) - goto out_arg_pages; + if (READ_ONCE(need_arg_pages)) { + ret = pin_arg_pages(&ctx.bprm_ctx); + if (ret < 0) + goto out_arg_pages; + } ret = krsi_run_progs(PROCESS_EXECUTION, &ctx); kfree(ctx.bprm_ctx.arg_pages); @@ -87,7 +139,7 @@ static int krsi_process_execution(struct linux_binprm *bprm) } static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = { - #define KRSI_HOOK_INIT(T, N, HOOK, IMPL) LSM_HOOK_INIT(HOOK, IMPL), + #define KRSI_HOOK_INIT(T, N, HOOK, IMPL, CB) LSM_HOOK_INIT(HOOK, IMPL), #include "hooks.h" #undef KRSI_HOOK_INIT }; diff --git a/security/krsi/ops.c b/security/krsi/ops.c index 1db94dfaac15..2de682371eff 100644 --- a/security/krsi/ops.c +++ b/security/krsi/ops.c @@ -139,6 +139,14 @@ int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog) goto unlock; } + if (h->attach_callback) { + ret = h->attach_callback(new_array); + if (ret < 0) { + bpf_prog_array_free(new_array); + goto unlock; + } + } + rcu_assign_pointer(h->progs, new_array); bpf_prog_array_free(old_array); @@ -278,7 +286,7 @@ BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32, n_size, return get_env_var(ctx, name, dest, n_size, size); } -static const struct bpf_func_proto krsi_get_env_var_proto = { +const struct bpf_func_proto krsi_get_env_var_proto = { .func = krsi_get_env_var, .gpl_only = true, .ret_type = RET_INTEGER,