* Re: [RFC v1 14/14] krsi: Pin arg pages only when needed
From: Yonghong Song @ 2019-09-15 19:45 UTC (permalink / raw)
To: KP Singh
Cc: linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org, Alexei Starovoitov,
Daniel Borkmann, James Morris, Kees Cook, Thomas Garnier,
Michael Halcrow, Paul Turner, Brendan Gregg, Jann Horn,
Matthew Garrett, Christian Brauner, Mickaël Salaün,
Florent Revest, Martin Lau, Song Liu, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer
In-Reply-To: <20190915014008.GA19558@chromium.org>
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,
>>>
^ permalink raw reply
* Здравствуйте! Вас интересуют клиентские базы данных?
From: linux-security-module @ 2019-09-15 6:05 UTC (permalink / raw)
To: linux-security-module
Здравствуйте! Вас интересуют клиентские базы данных?
^ permalink raw reply
* Re: [RFC v1 14/14] krsi: Pin arg pages only when needed
From: KP Singh @ 2019-09-15 1:40 UTC (permalink / raw)
To: Yonghong Song
Cc: linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org, Alexei Starovoitov,
Daniel Borkmann, James Morris, Kees Cook, Thomas Garnier,
Michael Halcrow, Paul Turner, Brendan Gregg, Jann Horn,
Matthew Garrett, Christian Brauner, Mickaël Salaün,
Florent Revest, Martin Lau, Song Liu, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer
In-Reply-To: <d9b7b968-c3a7-48a4-2eb0-85d5ac9dd196@fb.com>
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,
> >
^ permalink raw reply
* Re: Re: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution
From: Yonghong Song @ 2019-09-15 0:37 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <bb2d4453-f01f-8fb2-d901-a7a0a5eb4a4d@fb.com>
On 9/14/19 5:56 PM, Yonghong Song wrote:
>
>
> On 9/10/19 12:55 PM, KP Singh wrote:
>> From: KP Singh <kpsingh@google.com>
>>
>> A user space program can attach an eBPF program by:
>>
>> hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
>> prog_fd = bpf(BPF_PROG_LOAD, ...)
>> bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)
>>
>> When such an attach call is received, the attachment logic looks up the
>> dentry and appends the program to the bpf_prog_array.
>>
>> The BPF programs are stored in a bpf_prog_array and writes to the array
>> are guarded by a mutex. The eBPF programs are executed as a part of the
>> LSM hook they are attached to. If any of the eBPF programs return
>> an error (-ENOPERM) the action represented by the hook is denied.
>>
>> Signed-off-by: KP Singh <kpsingh@google.com>
>> ---
>> include/linux/krsi.h | 18 ++++++
>> kernel/bpf/syscall.c | 3 +-
>> security/krsi/include/krsi_init.h | 51 +++++++++++++++
>> security/krsi/krsi.c | 13 +++-
>> security/krsi/krsi_fs.c | 28 ++++++++
>> security/krsi/ops.c | 102 ++++++++++++++++++++++++++++++
>> 6 files changed, 213 insertions(+), 2 deletions(-)
>> create mode 100644 include/linux/krsi.h
>>
[...]
>>
>> +static inline int krsi_run_progs(enum krsi_hook_type t, struct krsi_ctx *ctx)
>> +{
>> + struct bpf_prog_array_item *item;
>> + struct bpf_prog *prog;
>> + struct krsi_hook *h = &krsi_hooks_list[t];
>> + int ret, retval = 0;
>
> Reverse christmas tree style?
>
>> +
>> + preempt_disable();
>
> Do we need preempt_disable() here?
From the following patches, I see perf_event_output() helper
and per-cpu array usage. So, indeed preempt_disable() is needed.
>
>> + rcu_read_lock();
>> +
>> + item = rcu_dereference(h->progs)->items;
>> + while ((prog = READ_ONCE(item->prog))) {
>> + ret = BPF_PROG_RUN(prog, ctx);
>> + if (ret < 0) {
>> + retval = ret;
>> + goto out;
>> + }
>> + item++;
>> + }
>> +
>> +out:
>> + rcu_read_unlock();
>> + preempt_enable();
>> + return IS_ENABLED(CONFIG_SECURITY_KRSI_ENFORCE) ? retval : 0;
>> +}
>> +
[...]
^ permalink raw reply
* Re: [RFC v1 14/14] krsi: Pin arg pages only when needed
From: Yonghong Song @ 2019-09-15 0:33 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-15-kpsingh@chromium.org>
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,
>
^ permalink raw reply
* Re: [RFC v1 13/14] krsi: Provide an example to read and log environment variables
From: Yonghong Song @ 2019-09-15 0:24 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-14-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> * The program takes the name of an environment variable as an
> argument.
> * An eBPF program is loaded and attached to the
> process_execution hook.
> * The name of the environment variable passed is updated in a
> eBPF per-cpu map.
> * The eBPF program uses the krsi_get_env_var helper to get the
> the value of this variable and logs the result to the perf
> events buffer.
> * The user-space program listens to the perf events and prints
> the values.
>
> Example execution:
>
> ./krsi LD_PRELOAD
>
> [p_pid=123] LD_PRELOAD is not set
> [p_pid=456] LD_PRELOAD=/lib/bad.so
> [p_pid=789] WARNING! LD_PRELOAD is set 2 times
> [p_pid=789] LD_PRELOAD=/lib/decoy.so
> [p_pid=789] LD_PRELOAD=/lib/bad.so
>
> In a separate session the following [1, 2, 3] exec system calls
> are made where:
>
> [1, 2, 3] char *argv[] = {"/bin/ls", 0};
>
> [1] char *envp = {0};
> [2] char *envp = {"LD_PRELOAD=/lib/bad.so", 0};
> [3] char *envp = {"LD_PRELOAD=/lib/decoy.so, "LD_PRELOAD=/lib/bad.so", 0};
>
> This example demonstrates that user-space is free to choose the format
> in which the data is logged and can use very specific helpers like
> krsi_get_env_var to populate only the data that is required.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> MAINTAINERS | 3 +
> samples/bpf/.gitignore | 1 +
> samples/bpf/Makefile | 3 +
> samples/bpf/krsi_helpers.h | 31 ++++++
> samples/bpf/krsi_kern.c | 52 ++++++++++
> samples/bpf/krsi_user.c | 202 +++++++++++++++++++++++++++++++++++++
The KRSI does not depend on kernel headers.
I would recommend to put this into tools/testing/selftests/bpf
as a selftest. Selftest is the place to test regressions with
bpf changes.
The krsi_kern.c can be put into tools/testing/selftests/bpf/progs
and krsi_user.c can be put into tools/testing/selftests/bpf/prog_tests.
> 6 files changed, 292 insertions(+)
> create mode 100644 samples/bpf/krsi_helpers.h
> create mode 100644 samples/bpf/krsi_kern.c
> create mode 100644 samples/bpf/krsi_user.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8e0364391d8b..ec378abb4c23 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9005,6 +9005,9 @@ F: kernel/kprobes.c
> KRSI SECURITY MODULE
> M: KP Singh <kpsingh@chromium.org>
> S: Supported
> +F: samples/bpf/krsi_helpers.h
> +F: samples/bpf/krsi_kern.c
> +F: samples/bpf/krsi_user.c
> F: security/krsi/
>
> KS0108 LCD CONTROLLER DRIVER
> diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
> index 74d31fd3c99c..6bbf5a04877f 100644
> --- a/samples/bpf/.gitignore
> +++ b/samples/bpf/.gitignore
> @@ -2,6 +2,7 @@ cpustat
> fds_example
> hbm
> ibumad
> +krsi
> lathist
> lwt_len_hist
> map_perf_test
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 1d9be26b4edd..33d3bef17549 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -8,6 +8,7 @@ hostprogs-y := test_lru_dist
> hostprogs-y += sock_example
> hostprogs-y += fds_example
> hostprogs-y += sockex1
> +hostprogs-y += krsi
> hostprogs-y += sockex2
> hostprogs-y += sockex3
> hostprogs-y += tracex1
> @@ -62,6 +63,7 @@ TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
>
> fds_example-objs := fds_example.o
> sockex1-objs := sockex1_user.o
> +krsi-objs := krsi_user.o $(TRACE_HELPERS)
> sockex2-objs := sockex2_user.o
> sockex3-objs := bpf_load.o sockex3_user.o
> tracex1-objs := bpf_load.o tracex1_user.o
> @@ -113,6 +115,7 @@ hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
> # Tell kbuild to always build the programs
> always := $(hostprogs-y)
> always += sockex1_kern.o
> +always += krsi_kern.o
> always += sockex2_kern.o
> always += sockex3_kern.o
> always += tracex1_kern.o
> diff --git a/samples/bpf/krsi_helpers.h b/samples/bpf/krsi_helpers.h
> new file mode 100644
> index 000000000000..3007bfd6212e
> --- /dev/null
> +++ b/samples/bpf/krsi_helpers.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _KRSI_HELPERS_H
> +#define _KRSI_HELPERS_H
> +
> +#define __bpf_percpu_val_align __aligned(8)
> +
> +#define ENV_VAR_NAME_MAX_LEN 48
> +#define ENV_VAR_VAL_MAX_LEN 4096
> +
> +#define MAX_CPUS 128
> +
> +#define __LOWER(x) (x & 0xffffffff)
> +#define __UPPER(x) (x >> 32)
> +
> +struct krsi_env_value {
> + // The name of the environment variable.
> + char name[ENV_VAR_NAME_MAX_LEN];
> + // The value of the environment variable (if set).
> + char value[ENV_VAR_VAL_MAX_LEN];
> + // Indicates if an overflow occurred while reading the value of the
> + // of the environment variable. This means that an -E2BIG was received
> + // from the krsi_get_env_var helper.
> + bool overflow;
> + // The number of times the environment variable was set.
> + __u32 times;
> + // The PID of the parent process.
> + __u32 p_pid;
> +} __bpf_percpu_val_align;
> +
> +#endif // _KRSI_HELPERS_H
> diff --git a/samples/bpf/krsi_kern.c b/samples/bpf/krsi_kern.c
> new file mode 100644
> index 000000000000..087a6f0cc81d
> --- /dev/null
> +++ b/samples/bpf/krsi_kern.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/ptrace.h>
> +#include <uapi/linux/bpf.h>
> +#include <uapi/linux/ip.h>
> +#include "bpf_helpers.h"
> +#include "krsi_helpers.h"
> +
> +#define MAX_CPUS 128
> +
> +struct bpf_map_def SEC("maps") env_map = {
> + .type = BPF_MAP_TYPE_PERCPU_ARRAY,
> + .key_size = sizeof(u32),
> + .value_size = sizeof(struct krsi_env_value),
> + .max_entries = 1,
> +};
> +
> +struct bpf_map_def SEC("maps") perf_map = {
> + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(u32),
> + .max_entries = MAX_CPUS,
> +};
> +
> +SEC("krsi")
> +int env_dumper(void *ctx)
> +{
> + u64 times_ret;
> + s32 ret;
> + u32 map_id = 0;
> + char *map_value;
> + struct krsi_env_value *env;
> +
> + env = bpf_map_lookup_elem(&env_map, &map_id);
> + if (!env)
> + return -ENOMEM;
> + times_ret = krsi_get_env_var(ctx, env->name, ENV_VAR_NAME_MAX_LEN,
> + env->value, ENV_VAR_VAL_MAX_LEN);
> + ret = __LOWER(times_ret);
> + if (ret == -E2BIG)
> + env->overflow = true;
> + else if (ret < 0)
> + return ret;
> +
> + env->times = __UPPER(times_ret);
> + env->p_pid = bpf_get_current_pid_tgid();
> + bpf_perf_event_output(ctx, &perf_map, BPF_F_CURRENT_CPU, env,
> + sizeof(struct krsi_env_value));
> +
> + return 0;
> +}
> +char _license[] SEC("license") = "GPL";
> diff --git a/samples/bpf/krsi_user.c b/samples/bpf/krsi_user.c
> new file mode 100644
> index 000000000000..1fad29bf017a
> --- /dev/null
> +++ b/samples/bpf/krsi_user.c
> @@ -0,0 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <err.h>
> +#include <assert.h>
> +#include <linux/limits.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf.h>
> +#include "bpf/libbpf.h"
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/sysinfo.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <sys/resource.h>
> +
> +#include "perf-sys.h"
> +#include "trace_helpers.h"
> +#include "krsi_helpers.h"
> +
> +#define LSM_HOOK_PATH "/sys/kernel/security/krsi/process_execution"
> +
> +static int pmu_fds[MAX_CPUS];
> +static struct perf_event_mmap_page *headers[MAX_CPUS];
> +
> +static int print_env(void *d, int size)
> +{
> + struct krsi_env_value *env = d;
> + int times = env->times;
> + char *next = env->value;
> + size_t total = 0;
> +
> + if (env->times > 1)
> + printf("[p_pid=%u] WARNING! %s is set %u times\n",
> + env->p_pid, env->name, env->times);
> +
> + /*
> + * krsi_get_env_var ensures that even overflows
> + * are null terminated. Incase of an overflow,
> + * this logic tries to print as much information
> + * that was gathered.
> + */
> + while (times && total < ENV_VAR_NAME_MAX_LEN) {
> + next += total;
> + if (env->overflow)
> + printf("[p_pid=%u] OVERFLOW! %s=%s\n",
> + env->p_pid, env->name, next);
> + else
> + printf("[p_pid=%u] %s=%s\n",
> + env->p_pid, env->name, next);
> + times--;
> + total += strlen(next) + 1;
> + }
> +
> + if (!env->times)
> + printf("p_pid=%u] %s is not set\n",
> + env->p_pid, env->name);
> +
> + return LIBBPF_PERF_EVENT_CONT;
> +}
> +
> +static int open_perf_events(int map_fd, int num)
> +{
> + int i;
> + struct perf_event_attr attr = {
> + .sample_type = PERF_SAMPLE_RAW,
> + .type = PERF_TYPE_SOFTWARE,
> + .config = PERF_COUNT_SW_BPF_OUTPUT,
> + .wakeup_events = 1, /* get an fd notification for every event */
> + };
> +
> + for (i = 0; i < num; i++) {
> + int key = i;
> + int ret;
> +
> + ret = sys_perf_event_open(&attr, -1 /*pid*/, i/*cpu*/,
> + -1/*group_fd*/, 0);
> + if (ret < 0)
> + return ret;
> + pmu_fds[i] = ret;
> + ret = bpf_map_update_elem(map_fd, &key, &pmu_fds[i], BPF_ANY);
> + if (ret < 0)
> + return ret;
> + ioctl(pmu_fds[i], PERF_EVENT_IOC_ENABLE, 0);
> + }
> + return 0;
> +}
> +
> +static int update_env_map(struct bpf_object *prog_obj, const char *env_var_name,
> + int numcpus)
> +{
> + struct bpf_map *map;
> + struct krsi_env_value *env;
> + int map_fd;
> + int key = 0, ret = 0, i;
> +
> + map = bpf_object__find_map_by_name(prog_obj, "env_map");
> + if (!map)
> + return -EINVAL;
> +
> + map_fd = bpf_map__fd(map);
> + if (map_fd < 0)
> + return map_fd;
> +
> + env = malloc(numcpus * sizeof(struct krsi_env_value));
> + if (!env) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + for (i = 0; i < numcpus; i++)
> + strcpy(env[i].name, env_var_name);
> +
> + ret = bpf_map_update_elem(map_fd, &key, env, BPF_ANY);
> + if (ret < 0)
> + goto out;
> +
> +out:
> + free(env);
> + return ret;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + struct bpf_object *prog_obj;
> + const char *env_var_name;
> + struct bpf_prog_load_attr attr;
> + int prog_fd, target_fd, map_fd;
> + int ret, i, numcpus;
> + struct bpf_map *map;
> + char filename[PATH_MAX];
> + struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> +
> +
> + if (argc != 2)
> + errx(EXIT_FAILURE, "Usage %s env_var_name\n", argv[0]);
> +
> + env_var_name = argv[1];
> + if (strlen(env_var_name) > ENV_VAR_NAME_MAX_LEN - 1)
> + errx(EXIT_FAILURE,
> + "<env_var_name> cannot be more than %d in length",
> + ENV_VAR_NAME_MAX_LEN - 1);
> +
> +
> + setrlimit(RLIMIT_MEMLOCK, &r);
> + snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
> +
> + memset(&attr, 0, sizeof(struct bpf_prog_load_attr));
> + attr.prog_type = BPF_PROG_TYPE_KRSI;
> + attr.expected_attach_type = BPF_KRSI;
> + attr.file = filename;
> +
> + /* Attach the BPF program to the given hook */
> + target_fd = open(LSM_HOOK_PATH, O_RDWR);
> + if (target_fd < 0)
> + err(EXIT_FAILURE, "Failed to open target file");
> +
> + if (bpf_prog_load_xattr(&attr, &prog_obj, &prog_fd))
> + err(EXIT_FAILURE, "Failed to load eBPF program");
> +
> + numcpus = get_nprocs();
> + if (numcpus > MAX_CPUS)
> + numcpus = MAX_CPUS;
> +
> + ret = update_env_map(prog_obj, env_var_name, numcpus);
> + if (ret < 0)
> + err(EXIT_FAILURE, "Failed to update env map");
> +
> + map = bpf_object__find_map_by_name(prog_obj, "perf_map");
> + if (!map)
> + err(EXIT_FAILURE,
> + "Finding the perf event map in obj file failed");
> +
> + map_fd = bpf_map__fd(map);
> + if (map_fd < 0)
> + err(EXIT_FAILURE, "Failed to get fd for perf events map");
> +
> + ret = bpf_prog_attach(prog_fd, target_fd, BPF_KRSI,
> + BPF_F_ALLOW_OVERRIDE);
> + if (ret < 0)
> + err(EXIT_FAILURE, "Failed to attach prog to LSM hook");
> +
> + ret = open_perf_events(map_fd, numcpus);
> + if (ret < 0)
> + err(EXIT_FAILURE, "Failed to open perf events handler");
> +
> + for (i = 0; i < numcpus; i++) {
> + ret = perf_event_mmap_header(pmu_fds[i], &headers[i]);
> + if (ret < 0)
> + err(EXIT_FAILURE, "perf_event_mmap_header");
> + }
> +
> + ret = perf_event_poller_multi(pmu_fds, headers, numcpus, print_env);
> + if (ret < 0)
> + err(EXIT_FAILURE, "Failed to poll perf events");
> +
> + return EXIT_SUCCESS;
> +}
>
^ permalink raw reply
* Re: [RFC v1 12/14] krsi: Add an eBPF helper function to get the value of an env variable
From: Yonghong Song @ 2019-09-15 0:16 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-13-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
This patch cannot apply cleanly.
-bash-4.4$ git apply ~/p12.txt
error: patch failed: include/uapi/linux/bpf.h:2715
error: include/uapi/linux/bpf.h: patch does not apply
error: patch failed: tools/include/uapi/linux/bpf.h:2715
error: tools/include/uapi/linux/bpf.h: patch does not apply
-bash-4.4$
>
> The helper returns the value of the environment variable in the buffer
> that is passed to it. If the var is set multiple times, the helper
> returns all the values as null separated strings.
>
> If the buffer is too short for these values, the helper tries to fill it
> the best it can and guarantees that the value returned in the buffer
> is always null terminated. After the buffer is filled, the helper keeps
> counting the number of times the environment variable is set in the
> envp.
>
> The return value of the helper is an u64 value which carries two pieces
> of information.
>
> * The upper 32 bits are a u32 value signifying the number of times
> the environment variable is set in the envp.
Not sure how useful this 'upper 32' bit value is. What user expected to do?
Another option is to have upper 32 bits encode the required buffer size
to hold all values. This may cause some kind of user space action, e.g.,
to replace the program with new program with larger per cpu map value size?
> * The lower 32 bits are a s32 value signifying the number of bytes
> written to the buffer or an error code. >
> Since the value of the environment variable can be very long and exceed
> what can be allocated on the BPF stack, a per-cpu array can be used
> instead:
>
> struct bpf_map_def SEC("maps") env_map = {
> .type = BPF_MAP_TYPE_PERCPU_ARRAY,
> .key_size = sizeof(u32),
> .value_size = 4096,
> .max_entries = 1,
> };
Could you use use map definition with SEC(".maps")?
>
> SEC("prgrm")
> int bpf_prog1(void *ctx)
> {
> u32 map_id = 0;
> u64 times_ret;
> s32 ret;
> char name[48] = "LD_PRELOAD";
Reverse Christmas tree coding style, here and other places?
>
> char *map_value = bpf_map_lookup_elem(&env_map, &map_id);
> if (!map_value)
> return 0;
>
> // Read the lower 32 bits for the return value
> times_ret = krsi_get_env_var(ctx, name, 48, map_value, 4096);
> ret = times_ret & 0xffffffff;
> if (ret < 0)
> return ret;
> return 0;
> }
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> include/uapi/linux/bpf.h | 42 ++++++-
> security/krsi/ops.c | 129 ++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 42 ++++++-
> tools/testing/selftests/bpf/bpf_helpers.h | 3 +
> 4 files changed, 214 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 32ab38f1a2fe..a4ef07956e07 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2715,6 +2715,45 @@ union bpf_attr {
> * **-EPERM** if no permission to send the *sig*.
> *
> * **-EAGAIN** if bpf program can try again.
> + *
> + * u64 krsi_get_env_var(void *ctx, char *name, char *buf,
> + * size_t name_len, size_t buf_len)
This signature is not the same as the later
krsi_get_env_var(...) helper definition.
BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32,
n_size,
char *, dest, u32, size)
> + * Description
> + * This helper can be used as a part of the
> + * process_execution hook of the KRSI LSM in
> + * programs of type BPF_PROG_TYPE_KRSI.
> + *
> + * The helper returns the value of the environment
> + * variable with the provided "name" for process that's
> + * going to be executed in the passed buffer, "buf". If the var
> + * is set multiple times, the helper returns all
> + * the values as null separated strings.
> + *
> + * If the buffer is too short for these values, the helper
> + * tries to fill it the best it can and guarantees that the value
> + * returned in the buffer is always null terminated.
> + * After the buffer is filled, the helper keeps counting the number
> + * of times the environment variable is set in the envp.
> + *
> + * Return:
> + *
> + * The return value of the helper is an u64 value
> + * which carries two pieces of information:
> + *
> + * The upper 32 bits are a u32 value signifying
> + * the number of times the environment variable
> + * is set in the envp.
> + * The lower 32 bits are an s32 value signifying
> + * the number of bytes written to the buffer or an error code:
> + *
> + * **-ENOMEM** if the kernel is unable to allocate memory
> + * for pinning the argv and envv.
> + *
> + * **-E2BIG** if the value is larger than the size of the
> + * destination buffer. The higher bits will still
> + * the number of times the variable was set in the envp.
The -E2BIG is returned because buffer sizee is not big enough.
Another possible error code is -ENOSPC, which typically indicates
buffer size not big enough.
> + *
> + * **-EINVAL** if name is not a NULL terminated string.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2826,7 +2865,8 @@ union bpf_attr {
> FN(strtoul), \
> FN(sk_storage_get), \
> FN(sk_storage_delete), \
> - FN(send_signal),
> + FN(send_signal), \
> + FN(krsi_get_env_var),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/security/krsi/ops.c b/security/krsi/ops.c
> index 1f4df920139c..1db94dfaac15 100644
> --- a/security/krsi/ops.c
> +++ b/security/krsi/ops.c
> @@ -6,6 +6,8 @@
> #include <linux/bpf.h>
> #include <linux/security.h>
> #include <linux/krsi.h>
> +#include <linux/binfmts.h>
> +#include <linux/highmem.h>
>
> #include "krsi_init.h"
> #include "krsi_fs.h"
> @@ -162,6 +164,131 @@ static bool krsi_prog_is_valid_access(int off, int size,
> return false;
> }
>
> +static char *array_next_entry(char *array, unsigned long *offset,
> + unsigned long end)
> +{
> + char *entry;
> + unsigned long current_offset = *offset;
> +
> + if (current_offset >= end)
> + return NULL;
> +
> + /*
> + * iterate on the array till the null byte is encountered
> + * and check for any overflows.
> + */
> + entry = array + current_offset;
> + while (array[current_offset]) {
> + if (unlikely(++current_offset >= end))
> + return NULL;
> + }
> +
> + /*
> + * Point the offset to the next element in the array.
> + */
> + *offset = current_offset + 1;
> +
> + return entry;
> +}
> +
> +static u64 get_env_var(struct krsi_ctx *ctx, char *name, char *dest,
> + u32 n_size, u32 size)
> +{
> + s32 ret = 0;
> + u32 num_vars = 0;
> + int i, name_len;
> + struct linux_binprm *bprm = ctx->bprm_ctx.bprm;
> + int argc = bprm->argc;
> + int envc = bprm->envc;
> + unsigned long end = ctx->bprm_ctx.max_arg_offset;
> + unsigned long offset = bprm->p % PAGE_SIZE;
why we need bprm->p % PAGE_SIZE instead of bprm->p?
> + char *buf = ctx->bprm_ctx.arg_pages;
> + char *curr_dest = dest;
> + char *entry;
> +
> + if (unlikely(!buf))
> + return -ENOMEM;
> +
> + for (i = 0; i < argc; i++) {
> + entry = array_next_entry(buf, &offset, end);
> + if (!entry)
> + return 0;
> + }
> +
> + name_len = strlen(name);
> + for (i = 0; i < envc; i++) {
> + entry = array_next_entry(buf, &offset, end);
> + if (!entry)
> + return 0;
If the buf is "LD_PRELOAD=a.so\0LD_PRELOAD=b.so" and argc=0,
we may skip the first entry?
> +
> + if (!strncmp(entry, name, name_len)) {
> + num_vars++;
There helper permits n_size = 0 (ARG_CONST_SIZE_OR_ZERO).
in this case, name_len = 0, strncmp(entry, name, name_len) will be always 0.
> +
> + /*
> + * There is no need to do further copying
> + * if the buffer is already full. Just count how many
> + * times the environment variable is set.
> + */
> + if (ret == -E2BIG)
> + continue;
> +
> + if (entry[name_len] != '=')
> + continue;
> +
> + /*
> + * Move the buf pointer by name_len + 1
> + * (for the "=" sign)
> + */
> + entry += name_len + 1;
> + ret = strlcpy(curr_dest, entry, size);
> +
> + if (ret >= size) {
> + ret = -E2BIG;
Here, we have a partial copy. Should you instead nullify (memset) it as
it is not really invalid one?
> + continue;
> + }
> +
> + /*
> + * strlcpy just returns the length of the string copied.
> + * The remaining space needs to account for the added
> + * null character.
> + */
> + curr_dest += ret + 1;
> + size -= ret + 1;
> + /*
> + * Update ret to be the current number of bytes written
> + * to the destination
> + */
> + ret = curr_dest - dest;
> + }
> + }
> +
> + return (u64) num_vars << 32 | (u32) ret;
> +}
> +
> +BPF_CALL_5(krsi_get_env_var, struct krsi_ctx *, ctx, char *, name, u32, n_size,
> + char *, dest, u32, size)
> +{
> + char *name_end;
> +
> + name_end = memchr(name, '\0', n_size);
> + if (!name_end)
> + return -EINVAL;
> +
> + memset(dest, 0, size);
> + return get_env_var(ctx, name, dest, n_size, size);
> +}
> +
> +static const struct bpf_func_proto krsi_get_env_var_proto = {
> + .func = krsi_get_env_var,
> + .gpl_only = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE_OR_ZERO,
> + .arg4_type = ARG_PTR_TO_UNINIT_MEM,
> + .arg5_type = ARG_CONST_SIZE_OR_ZERO,
> +};
> +
> BPF_CALL_5(krsi_event_output, void *, log,
> struct bpf_map *, map, u64, flags, void *, data, u64, size)
> {
> @@ -192,6 +319,8 @@ static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
> return &bpf_map_lookup_elem_proto;
> case BPF_FUNC_get_current_pid_tgid:
> return &bpf_get_current_pid_tgid_proto;
> + case BPF_FUNC_krsi_get_env_var:
> + return &krsi_get_env_var_proto;
> case BPF_FUNC_perf_event_output:
> return &krsi_event_output_proto;
> default:
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 32ab38f1a2fe..a4ef07956e07 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2715,6 +2715,45 @@ union bpf_attr {
> * **-EPERM** if no permission to send the *sig*.
> *
> * **-EAGAIN** if bpf program can try again.
> + *
> + * u64 krsi_get_env_var(void *ctx, char *name, char *buf,
> + * size_t name_len, size_t buf_len)
Inconsistent helper definitions.
> + * Description
> + * This helper can be used as a part of the
> + * process_execution hook of the KRSI LSM in
> + * programs of type BPF_PROG_TYPE_KRSI.
> + *
> + * The helper returns the value of the environment
> + * variable with the provided "name" for process that's
> + * going to be executed in the passed buffer, "buf". If the var
> + * is set multiple times, the helper returns all
> + * the values as null separated strings.
> + *
> + * If the buffer is too short for these values, the helper
> + * tries to fill it the best it can and guarantees that the value
> + * returned in the buffer is always null terminated.
> + * After the buffer is filled, the helper keeps counting the number
> + * of times the environment variable is set in the envp.
> + *
> + * Return:
> + *
> + * The return value of the helper is an u64 value
> + * which carries two pieces of information:
> + *
> + * The upper 32 bits are a u32 value signifying
> + * the number of times the environment variable
> + * is set in the envp.
> + * The lower 32 bits are an s32 value signifying
> + * the number of bytes written to the buffer or an error code:
> + *
> + * **-ENOMEM** if the kernel is unable to allocate memory
> + * for pinning the argv and envv.
> + *
> + * **-E2BIG** if the value is larger than the size of the
> + * destination buffer. The higher bits will still
> + * the number of times the variable was set in the envp.
> + *
> + * **-EINVAL** if name is not a NULL terminated string.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2826,7 +2865,8 @@ union bpf_attr {
> FN(strtoul), \
> FN(sk_storage_get), \
> FN(sk_storage_delete), \
> - FN(send_signal),
> + FN(send_signal), \
> + FN(krsi_get_env_var),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index f804f210244e..ecebdb772a9d 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -303,6 +303,9 @@ static int (*bpf_get_numa_node_id)(void) =
> static int (*bpf_probe_read_str)(void *ctx, __u32 size,
> const void *unsafe_ptr) =
> (void *) BPF_FUNC_probe_read_str;
> +static unsigned long long (*krsi_get_env_var)(void *ctx,
> + void *name, __u32 n_size, void *buf, __u32 size) =
> + (void *) BPF_FUNC_krsi_get_env_var;
> static unsigned int (*bpf_get_socket_uid)(void *ctx) =
> (void *) BPF_FUNC_get_socket_uid;
> static unsigned int (*bpf_set_hash)(void *ctx, __u32 hash) =
>
^ permalink raw reply
* Re: [RFC v1 09/14] krsi: Add a helper function for bpf_perf_event_output
From: Yonghong Song @ 2019-09-14 18:23 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-10-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> This helper is mapped to the existing operation
> BPF_FUNC_perf_event_output.
>
> An example usage of this function would be:
>
> #define BUF_SIZE 64;
>
> struct bpf_map_def SEC("maps") perf_map = {
> .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> .key_size = sizeof(int),
> .value_size = sizeof(u32),
> .max_entries = MAX_CPUS,
> };
could you use a map definition similar to
tools/testing/selftests/bpf/progs/test_perf_buffer.c?
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__uint(key_size, sizeof(int));
__uint(value_size, sizeof(u32));
} perf_map SEC(".maps");
>
> SEC("krsi")
> int bpf_prog1(void *ctx)
> {
> char buf[BUF_SIZE];
> int len;
> u64 flags = BPF_F_CURRENT_CPU;
>
> /* some logic that fills up buf with len data*/
> len = fill_up_buf(buf);
> if (len < 0)
> return len;
> if (len > BU)
BUF_SIZE?
> return 0;
>
> bpf_perf_event_output(ctx, &perf_map, flags, buf len);
buf, len?
> return 0;
> }
>
> A sample program that showcases the use of bpf_perf_event_output is
> added later.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> security/krsi/ops.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/security/krsi/ops.c b/security/krsi/ops.c
> index a61508b7018f..57bd304a03f4 100644
> --- a/security/krsi/ops.c
> +++ b/security/krsi/ops.c
> @@ -111,6 +111,26 @@ static bool krsi_prog_is_valid_access(int off, int size,
> return false;
> }
>
> +BPF_CALL_5(krsi_event_output, void *, log,
Maybe name the first argument as 'ctx' to follow typical helper convention?
> + struct bpf_map *, map, u64, flags, void *, data, u64, size)
> +{
> + if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
> + return -EINVAL;
> +
> + return bpf_event_output(map, flags, data, size, NULL, 0, NULL);
> +}
> +
> +static const struct bpf_func_proto krsi_event_output_proto = {
> + .func = krsi_event_output,
> + .gpl_only = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_CONST_MAP_PTR,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_PTR_TO_MEM,
> + .arg5_type = ARG_CONST_SIZE_OR_ZERO,
> +};
> +
> static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
> func_id,
> const struct bpf_prog
> @@ -121,6 +141,8 @@ static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
> return &bpf_map_lookup_elem_proto;
> case BPF_FUNC_get_current_pid_tgid:
> return &bpf_get_current_pid_tgid_proto;
> + case BPF_FUNC_perf_event_output:
> + return &krsi_event_output_proto;
> default:
> return NULL;
> }
>
^ permalink raw reply
* Re: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution
From: Yonghong Song @ 2019-09-14 16:56 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-7-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> A user space program can attach an eBPF program by:
>
> hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
> prog_fd = bpf(BPF_PROG_LOAD, ...)
> bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)
>
> When such an attach call is received, the attachment logic looks up the
> dentry and appends the program to the bpf_prog_array.
>
> The BPF programs are stored in a bpf_prog_array and writes to the array
> are guarded by a mutex. The eBPF programs are executed as a part of the
> LSM hook they are attached to. If any of the eBPF programs return
> an error (-ENOPERM) the action represented by the hook is denied.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> include/linux/krsi.h | 18 ++++++
> kernel/bpf/syscall.c | 3 +-
> security/krsi/include/krsi_init.h | 51 +++++++++++++++
> security/krsi/krsi.c | 13 +++-
> security/krsi/krsi_fs.c | 28 ++++++++
> security/krsi/ops.c | 102 ++++++++++++++++++++++++++++++
> 6 files changed, 213 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/krsi.h
>
> diff --git a/include/linux/krsi.h b/include/linux/krsi.h
> new file mode 100644
> index 000000000000..c7d1790d0c1f
> --- /dev/null
> +++ b/include/linux/krsi.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _KRSI_H
> +#define _KRSI_H
> +
> +#include <linux/bpf.h>
> +
> +#ifdef CONFIG_SECURITY_KRSI
> +int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog);
> +#else
> +static inline int krsi_prog_attach(const union bpf_attr *attr,
> + struct bpf_prog *prog)
> +{
> + return -EINVAL;
> +}
> +#endif /* CONFIG_SECURITY_KRSI */
> +
> +#endif /* _KRSI_H */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index f38a539f7e67..ab063ed84258 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4,6 +4,7 @@
> #include <linux/bpf.h>
> #include <linux/bpf_trace.h>
> #include <linux/bpf_lirc.h>
> +#include <linux/krsi.h>
> #include <linux/btf.h>
> #include <linux/syscalls.h>
> #include <linux/slab.h>
> @@ -1950,7 +1951,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> ret = lirc_prog_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_KRSI:
> - ret = -EINVAL;
> + ret = krsi_prog_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_FLOW_DISSECTOR:
> ret = skb_flow_dissector_bpf_prog_attach(attr, prog);
> diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h
> index 68755182a031..4e17ecacd4ed 100644
> --- a/security/krsi/include/krsi_init.h
> +++ b/security/krsi/include/krsi_init.h
> @@ -5,12 +5,29 @@
>
> #include "krsi_fs.h"
>
> +#include <linux/binfmts.h>
> +
> enum krsi_hook_type {
> PROCESS_EXECUTION,
> __MAX_KRSI_HOOK_TYPE, /* delimiter */
> };
>
> extern int krsi_fs_initialized;
> +
> +struct krsi_bprm_ctx {
> + struct linux_binprm *bprm;
> +};
> +
> +/*
> + * krsi_ctx is the context that is passed to all KRSI eBPF
> + * programs.
> + */
> +struct krsi_ctx {
> + union {
> + struct krsi_bprm_ctx bprm_ctx;
> + };
> +};
> +
> /*
> * The LSM creates one file per hook.
> *
> @@ -33,10 +50,44 @@ struct krsi_hook {
> * The dentry of the file created in securityfs.
> */
> struct dentry *h_dentry;
> + /*
> + * The mutex must be held when updating the progs attached to the hook.
> + */
> + struct mutex mutex;
> + /*
> + * The eBPF programs that are attached to this hook.
> + */
> + struct bpf_prog_array __rcu *progs;
> };
>
> extern struct krsi_hook krsi_hooks_list[];
>
> +static inline int krsi_run_progs(enum krsi_hook_type t, struct krsi_ctx *ctx)
> +{
> + struct bpf_prog_array_item *item;
> + struct bpf_prog *prog;
> + struct krsi_hook *h = &krsi_hooks_list[t];
> + int ret, retval = 0;
Reverse christmas tree style?
> +
> + preempt_disable();
Do we need preempt_disable() here?
> + rcu_read_lock();
> +
> + item = rcu_dereference(h->progs)->items;
> + while ((prog = READ_ONCE(item->prog))) {
> + ret = BPF_PROG_RUN(prog, ctx);
> + if (ret < 0) {
> + retval = ret;
> + goto out;
> + }
> + item++;
> + }
> +
> +out:
> + rcu_read_unlock();
> + preempt_enable();
> + return IS_ENABLED(CONFIG_SECURITY_KRSI_ENFORCE) ? retval : 0;
> +}
> +
> #define krsi_for_each_hook(hook) \
> for ((hook) = &krsi_hooks_list[0]; \
> (hook) < &krsi_hooks_list[__MAX_KRSI_HOOK_TYPE]; \
> diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c
> index 77d7e2f91172..d3a4a361c192 100644
> --- a/security/krsi/krsi.c
> +++ b/security/krsi/krsi.c
> @@ -1,6 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
>
> #include <linux/lsm_hooks.h>
> +#include <linux/filter.h>
> +#include <linux/bpf.h>
> +#include <linux/binfmts.h>
>
> #include "krsi_init.h"
>
> @@ -16,7 +19,15 @@ struct krsi_hook krsi_hooks_list[] = {
>
> static int krsi_process_execution(struct linux_binprm *bprm)
> {
> - return 0;
> + int ret;
> + struct krsi_ctx ctx;
> +
> + ctx.bprm_ctx = (struct krsi_bprm_ctx) {
> + .bprm = bprm,
> + };
> +
> + ret = krsi_run_progs(PROCESS_EXECUTION, &ctx);
> + return ret;
> }
>
> static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = {
> diff --git a/security/krsi/krsi_fs.c b/security/krsi/krsi_fs.c
> index 604f826cee5c..3ba18b52ce85 100644
> --- a/security/krsi/krsi_fs.c
> +++ b/security/krsi/krsi_fs.c
> @@ -5,6 +5,8 @@
> #include <linux/file.h>
> #include <linux/fs.h>
> #include <linux/types.h>
> +#include <linux/filter.h>
> +#include <linux/bpf.h>
> #include <linux/security.h>
>
> #include "krsi_fs.h"
> @@ -27,12 +29,29 @@ bool is_krsi_hook_file(struct file *f)
>
> static void __init krsi_free_hook(struct krsi_hook *h)
> {
> + struct bpf_prog_array_item *item;
> + /*
> + * This function is __init so we are guarranteed that there will be
> + * no concurrent access.
> + */
> + struct bpf_prog_array *progs = rcu_dereference_raw(h->progs);
> +
> + if (progs) {
bpf_prog_array itself should never be null?
> + item = progs->items;
> + while (item->prog) {
> + bpf_prog_put(item->prog);
> + item++;
> + }
> + bpf_prog_array_free(progs);
> + }
> +
> securityfs_remove(h->h_dentry);
> h->h_dentry = NULL;
> }
>
> static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
> {
> + struct bpf_prog_array __rcu *progs;
> struct dentry *h_dentry;
> int ret;
>
> @@ -41,6 +60,15 @@ static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
>
> if (IS_ERR(h_dentry))
> return PTR_ERR(h_dentry);
> +
> + mutex_init(&h->mutex);
> + progs = bpf_prog_array_alloc(0, GFP_KERNEL);
> + if (!progs) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + RCU_INIT_POINTER(h->progs, progs);
> h_dentry->d_fsdata = h;
> h->h_dentry = h_dentry;
> return 0;
> diff --git a/security/krsi/ops.c b/security/krsi/ops.c
> index f2de3bd9621e..cf4d06189aa1 100644
> --- a/security/krsi/ops.c
> +++ b/security/krsi/ops.c
> @@ -1,10 +1,112 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#include <linux/err.h>
> +#include <linux/types.h>
> #include <linux/filter.h>
> #include <linux/bpf.h>
> +#include <linux/security.h>
> +#include <linux/krsi.h>
> +
> +#include "krsi_init.h"
> +#include "krsi_fs.h"
> +
> +extern struct krsi_hook krsi_hooks_list[];
> +
> +static struct krsi_hook *get_hook_from_fd(int fd)
> +{
> + struct fd f = fdget(fd);
> + struct krsi_hook *h;
> + int ret;
> +
> + if (!f.file) {
> + ret = -EBADF;
> + goto error;
> + }
> +
> + if (!is_krsi_hook_file(f.file)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /*
> + * The securityfs dentry never disappears, so we don't need to take a
> + * reference to it.
> + */
> + h = file_dentry(f.file)->d_fsdata;
> + if (WARN_ON(!h)) {
> + ret = -EINVAL;
> + goto error;
> + }
> + fdput(f);
> + return h;
> +
> +error:
> + fdput(f);
> + return ERR_PTR(ret);
> +}
> +
> +int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> +{
> + struct bpf_prog_array *old_array;
> + struct bpf_prog_array *new_array;
> + struct krsi_hook *h;
> + int ret = 0;
> +
> + h = get_hook_from_fd(attr->target_fd);
> + if (IS_ERR(h))
> + return PTR_ERR(h);
> +
> + mutex_lock(&h->mutex);
> + old_array = rcu_dereference_protected(h->progs,
> + lockdep_is_held(&h->mutex));
> +
> + ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
> + if (ret < 0) {
> + ret = -ENOMEM;
> + goto unlock;
> + }
> +
> + rcu_assign_pointer(h->progs, new_array);
> + bpf_prog_array_free(old_array);
> +
> +unlock:
> + mutex_unlock(&h->mutex);
> + return ret;
> +}
>
> const struct bpf_prog_ops krsi_prog_ops = {
> };
>
> +static bool krsi_prog_is_valid_access(int off, int size,
> + enum bpf_access_type type,
> + const struct bpf_prog *prog,
> + struct bpf_insn_access_aux *info)
> +{
> + /*
> + * KRSI is conservative about any direct access in eBPF to
> + * prevent the users from depending on the internals of the kernel and
> + * aims at providing a rich eco-system of safe eBPF helpers as an API
> + * for accessing relevant information from the context.
> + */
> + return false;
> +}
> +
> +static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
> + func_id,
> + const struct bpf_prog
> + *prog)
> +{
> + switch (func_id) {
> + case BPF_FUNC_map_lookup_elem:
> + return &bpf_map_lookup_elem_proto;
> + case BPF_FUNC_get_current_pid_tgid:
> + return &bpf_get_current_pid_tgid_proto;
> + default:
> + return NULL;
> + }
> +}
> +
> const struct bpf_verifier_ops krsi_verifier_ops = {
> + .get_func_proto = krsi_prog_func_proto,
> + .is_valid_access = krsi_prog_is_valid_access,
> };
>
^ permalink raw reply
* Re: [RFC v1 05/14] krsi: Initialize KRSI hooks and create files in securityfs
From: Yonghong Song @ 2019-09-14 16:26 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-6-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> The LSM creates files in securityfs for each hook registered with the
> LSM.
>
> /sys/kernel/security/bpf/<h_name>
>
> The initialization of the hooks is done collectively in an internal
> header "hooks.h" which results in:
>
> * Creation of a file for the hook in the securityfs.
> * Allocation of a krsi_hook data structure which stores a pointer to the
> dentry of the newly created file in securityfs.
> * A pointer to the krsi_hook data structure is stored in the private
> d_fsdata of dentry of the file created in securityFS.
>
> These files will later be used to specify an attachment target during
> BPF_PROG_LOAD.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> security/krsi/Makefile | 4 +-
> security/krsi/include/hooks.h | 21 ++++++++
> security/krsi/include/krsi_fs.h | 19 +++++++
> security/krsi/include/krsi_init.h | 45 ++++++++++++++++
> security/krsi/krsi.c | 16 +++++-
> security/krsi/krsi_fs.c | 88 +++++++++++++++++++++++++++++++
> 6 files changed, 191 insertions(+), 2 deletions(-)
> create mode 100644 security/krsi/include/hooks.h
> create mode 100644 security/krsi/include/krsi_fs.h
> create mode 100644 security/krsi/include/krsi_init.h
> create mode 100644 security/krsi/krsi_fs.c
>
> diff --git a/security/krsi/Makefile b/security/krsi/Makefile
> index 660cc1f422fd..4586241f16e1 100644
> --- a/security/krsi/Makefile
> +++ b/security/krsi/Makefile
> @@ -1 +1,3 @@
> -obj-$(CONFIG_SECURITY_KRSI) := krsi.o ops.o
> +obj-$(CONFIG_SECURITY_KRSI) := krsi.o krsi_fs.o ops.o
> +
> +ccflags-y := -I$(srctree)/security/krsi -I$(srctree)/security/krsi/include
> diff --git a/security/krsi/include/hooks.h b/security/krsi/include/hooks.h
> new file mode 100644
> index 000000000000..e070c452b5de
> --- /dev/null
> +++ b/security/krsi/include/hooks.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * The hooks for the KRSI LSM are declared in this file.
> + *
> + * This header MUST NOT be included directly and should
> + * be only used to initialize the hooks lists.
> + *
> + * Format:
> + *
> + * KRSI_HOOK_INIT(TYPE, NAME, LSM_HOOK, KRSI_HOOK_FN)
> + *
> + * 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
> + * breaking in case the name of the hook changes in the kernel or if there's
> + * another LSM hook that maps better to the represented security behaviour.
> + */
> +KRSI_HOOK_INIT(PROCESS_EXECUTION,
> + process_execution,
> + bprm_check_security,
> + krsi_process_execution)
> diff --git a/security/krsi/include/krsi_fs.h b/security/krsi/include/krsi_fs.h
> new file mode 100644
> index 000000000000..38134661d8d6
> --- /dev/null
> +++ b/security/krsi/include/krsi_fs.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _KRSI_FS_H
> +#define _KRSI_FS_H
> +
> +#include <linux/bpf.h>
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +
> +bool is_krsi_hook_file(struct file *f);
> +
> +/*
> + * The name of the directory created in securityfs
> + *
> + * /sys/kernel/security/<dir_name>
> + */
> +#define KRSI_SFS_NAME "krsi"
> +
> +#endif /* _KRSI_FS_H */
> diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h
> new file mode 100644
> index 000000000000..68755182a031
> --- /dev/null
> +++ b/security/krsi/include/krsi_init.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _KRSI_INIT_H
> +#define _KRSI_INIT_H
> +
> +#include "krsi_fs.h"
> +
> +enum krsi_hook_type {
> + PROCESS_EXECUTION,
> + __MAX_KRSI_HOOK_TYPE, /* delimiter */
> +};
> +
> +extern int krsi_fs_initialized;
> +/*
> + * The LSM creates one file per hook.
> + *
> + * A pointer to krsi_hook data structure is stored in the
> + * private fsdata of the dentry of the per-hook file created
> + * in securityfs.
> + */
> +struct krsi_hook {
> + /*
> + * The name of the security hook, a file with this name will be created
> + * in the securityfs.
> + */
> + const char *name;
> + /*
> + * The type of the LSM hook, the LSM uses this to index the list of the
> + * hooks to run the eBPF programs that may have been attached.
> + */
> + enum krsi_hook_type h_type;
> + /*
> + * The dentry of the file created in securityfs.
> + */
> + struct dentry *h_dentry;
> +};
> +
> +extern struct krsi_hook krsi_hooks_list[];
> +
> +#define krsi_for_each_hook(hook) \
> + for ((hook) = &krsi_hooks_list[0]; \
> + (hook) < &krsi_hooks_list[__MAX_KRSI_HOOK_TYPE]; \
> + (hook)++)
> +
> +#endif /* _KRSI_INIT_H */
> diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c
> index 9ce4f56fb78d..77d7e2f91172 100644
> --- a/security/krsi/krsi.c
> +++ b/security/krsi/krsi.c
> @@ -2,13 +2,27 @@
>
> #include <linux/lsm_hooks.h>
>
> +#include "krsi_init.h"
> +
> +struct krsi_hook krsi_hooks_list[] = {
> + #define KRSI_HOOK_INIT(TYPE, NAME, H, I) \
> + [TYPE] = { \
> + .h_type = TYPE, \
> + .name = #NAME, \
> + },
> + #include "hooks.h"
> + #undef KRSI_HOOK_INIT
> +};
> +
> static int krsi_process_execution(struct linux_binprm *bprm)
> {
> return 0;
> }
>
> static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = {
> - LSM_HOOK_INIT(bprm_check_security, krsi_process_execution),
> + #define KRSI_HOOK_INIT(T, N, HOOK, IMPL) LSM_HOOK_INIT(HOOK, IMPL),
> + #include "hooks.h"
> + #undef KRSI_HOOK_INIT
> };
>
> static int __init krsi_init(void)
> diff --git a/security/krsi/krsi_fs.c b/security/krsi/krsi_fs.c
> new file mode 100644
> index 000000000000..604f826cee5c
> --- /dev/null
> +++ b/security/krsi/krsi_fs.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +#include <linux/security.h>
> +
> +#include "krsi_fs.h"
> +#include "krsi_init.h"
> +
> +extern struct krsi_hook krsi_hooks_list[];
> +
> +static struct dentry *krsi_dir;
> +
> +static const struct file_operations krsi_hook_ops = {
> + .llseek = generic_file_llseek,
> +};
> +
> +int krsi_fs_initialized;
> +
> +bool is_krsi_hook_file(struct file *f)
> +{
> + return f->f_op == &krsi_hook_ops;
> +}
> +
> +static void __init krsi_free_hook(struct krsi_hook *h)
> +{
> + securityfs_remove(h->h_dentry);
> + h->h_dentry = NULL;
> +}
> +
> +static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
> +{
> + struct dentry *h_dentry;
> + int ret;
> +
> + h_dentry = securityfs_create_file(h->name, 0600, parent,
> + NULL, &krsi_hook_ops);
> +
> + if (IS_ERR(h_dentry))
> + return PTR_ERR(h_dentry);
> + h_dentry->d_fsdata = h;
> + h->h_dentry = h_dentry;
> + return 0;
> +
> +error:
The 'error' label is not used here.
> + securityfs_remove(h_dentry);
> + return ret;
> +}
> +
> +static int __init krsi_fs_init(void)
> +{
> +
> + struct krsi_hook *hook;
> + int ret;
> +
> + krsi_dir = securityfs_create_dir(KRSI_SFS_NAME, NULL);
> + if (IS_ERR(krsi_dir)) {
> + ret = PTR_ERR(krsi_dir);
> + pr_err("Unable to create krsi sysfs dir: %d\n", ret);
> + krsi_dir = NULL;
> + return ret;
> + }
> +
> + /*
> + * If there is an error in initializing a hook, the initialization
> + * logic makes sure that it has been freed, but this means that
> + * cleanup should be called for all the other hooks. The cleanup
> + * logic handles uninitialized data.
> + */
> + krsi_for_each_hook(hook) {
> + ret = krsi_init_hook(hook, krsi_dir);
> + if (ret < 0)
> + goto error;
> + }
> +
> + krsi_fs_initialized = 1;
> + return 0;
> +error:
> + krsi_for_each_hook(hook)
> + krsi_free_hook(hook);
> + securityfs_remove(krsi_dir);
> + return ret;
> +}
> +
> +late_initcall(krsi_fs_init);
>
^ permalink raw reply
* Re: [RFC v1 04/14] krsi: Add support in libbpf for BPF_PROG_TYPE_KRSI
From: Yonghong Song @ 2019-09-14 16:09 UTC (permalink / raw)
To: KP Singh, linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-security-module@vger.kernel.org
Cc: Alexei Starovoitov, Daniel Borkmann, James Morris, Kees Cook,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Martin Lau, Song Liu,
Serge E. Hallyn, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Nicolas Ferre, Stanislav Fomichev,
Quentin Monnet, Andrey Ignatov, Joe Stringer
In-Reply-To: <20190910115527.5235-5-kpsingh@chromium.org>
On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> Update the libbpf library with functionality to load and
> attach a program type BPF_PROG_TYPE_KRSI.
>
> Since the bpf_prog_load does not allow the specification of an
> expected attach type, it's recommended to use bpf_prog_load_xattr and
> set the expected attach type as KRSI.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> tools/lib/bpf/libbpf.c | 4 ++++
> tools/lib/bpf/libbpf.h | 2 ++
> tools/lib/bpf/libbpf.map | 2 ++
> tools/lib/bpf/libbpf_probes.c | 1 +
> 4 files changed, 9 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2b57d7ea7836..3cc86bbc68cd 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2676,6 +2676,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
> case BPF_PROG_TYPE_PERF_EVENT:
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> + case BPF_PROG_TYPE_KRSI:
> return false;
> case BPF_PROG_TYPE_KPROBE:
> default:
> @@ -3536,6 +3537,7 @@ bool bpf_program__is_##NAME(const struct bpf_program *prog) \
> } \
>
> BPF_PROG_TYPE_FNS(socket_filter, BPF_PROG_TYPE_SOCKET_FILTER);
> +BPF_PROG_TYPE_FNS(krsi, BPF_PROG_TYPE_KRSI);
> BPF_PROG_TYPE_FNS(kprobe, BPF_PROG_TYPE_KPROBE);
> BPF_PROG_TYPE_FNS(sched_cls, BPF_PROG_TYPE_SCHED_CLS);
> BPF_PROG_TYPE_FNS(sched_act, BPF_PROG_TYPE_SCHED_ACT);
> @@ -3590,6 +3592,8 @@ static const struct {
> BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT),
> BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT),
> BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL),
> + BPF_APROG_SEC("krsi", BPF_PROG_TYPE_KRSI,
> + BPF_KRSI),
> BPF_APROG_SEC("cgroup_skb/ingress", BPF_PROG_TYPE_CGROUP_SKB,
> BPF_CGROUP_INET_INGRESS),
> BPF_APROG_SEC("cgroup_skb/egress", BPF_PROG_TYPE_CGROUP_SKB,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5cbf459ece0b..8781d29b4035 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -261,6 +261,7 @@ LIBBPF_API int bpf_program__set_sched_cls(struct bpf_program *prog);
> LIBBPF_API int bpf_program__set_sched_act(struct bpf_program *prog);
> LIBBPF_API int bpf_program__set_xdp(struct bpf_program *prog);
> LIBBPF_API int bpf_program__set_perf_event(struct bpf_program *prog);
> +LIBBPF_API int bpf_program__set_krsi(struct bpf_program *prog);
> LIBBPF_API void bpf_program__set_type(struct bpf_program *prog,
> enum bpf_prog_type type);
> LIBBPF_API void
> @@ -275,6 +276,7 @@ LIBBPF_API bool bpf_program__is_sched_cls(const struct bpf_program *prog);
> LIBBPF_API bool bpf_program__is_sched_act(const struct bpf_program *prog);
> LIBBPF_API bool bpf_program__is_xdp(const struct bpf_program *prog);
> LIBBPF_API bool bpf_program__is_perf_event(const struct bpf_program *prog);
> +LIBBPF_API bool bpf_program__is_krsi(const struct bpf_program *prog);
>
> /*
> * No need for __attribute__((packed)), all members of 'bpf_map_def'
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index f9d316e873d8..75b8fe419c11 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -68,6 +68,7 @@ LIBBPF_0.0.1 {
> bpf_prog_test_run_xattr;
> bpf_program__fd;
> bpf_program__is_kprobe;
> + bpf_program__is_krsi;
> bpf_program__is_perf_event;
> bpf_program__is_raw_tracepoint;
> bpf_program__is_sched_act;
> @@ -85,6 +86,7 @@ LIBBPF_0.0.1 {
> bpf_program__set_expected_attach_type;
> bpf_program__set_ifindex;
> bpf_program__set_kprobe;
> + bpf_program__set_krsi;
> bpf_program__set_perf_event;
> bpf_program__set_prep;
> bpf_program__set_priv;
Please put the above two new API functions in version LIBBPF_0.0.5.
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index ace1a0708d99..cc515a36794d 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -102,6 +102,7 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
> case BPF_PROG_TYPE_FLOW_DISSECTOR:
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> + case BPF_PROG_TYPE_KRSI:
> default:
> break;
> }
>
^ permalink raw reply
* Re: [PATCH v4] KEYS: trusted: correctly initialize digests and fix locking issue
From: Jarkko Sakkinen @ 2019-09-14 13:04 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar, jsnitsel, linux-integrity, linux-security-module, keyrings,
linux-kernel, silviu.vlasceanu
In-Reply-To: <20190913185136.780-1-roberto.sassu@huawei.com>
On Fri, Sep 13, 2019 at 08:51:36PM +0200, Roberto Sassu wrote:
> Commit 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to
> tpm_pcr_extend()") modifies tpm_pcr_extend() to accept a digest for each
> PCR bank. After modification, tpm_pcr_extend() expects that digests are
> passed in the same order as the algorithms set in chip->allocated_banks.
>
> This patch fixes two issues introduced in the last iterations of the patch
> set: missing initialization of the TPM algorithm ID in the tpm_digest
> structures passed to tpm_pcr_extend() by the trusted key module, and
> unreleased locks in the TPM driver due to returning from tpm_pcr_extend()
> without calling tpm_put_ops().
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
Reviewed-by: <jarkko.sakkinen@linux.intel.com>
I picked up this patch to my tree.
/Jarkko
^ permalink raw reply
* [PATCH v4] KEYS: trusted: correctly initialize digests and fix locking issue
From: Roberto Sassu @ 2019-09-13 18:51 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jsnitsel
Cc: linux-integrity, linux-security-module, keyrings, linux-kernel,
silviu.vlasceanu, Roberto Sassu
Commit 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to
tpm_pcr_extend()") modifies tpm_pcr_extend() to accept a digest for each
PCR bank. After modification, tpm_pcr_extend() expects that digests are
passed in the same order as the algorithms set in chip->allocated_banks.
This patch fixes two issues introduced in the last iterations of the patch
set: missing initialization of the TPM algorithm ID in the tpm_digest
structures passed to tpm_pcr_extend() by the trusted key module, and
unreleased locks in the TPM driver due to returning from tpm_pcr_extend()
without calling tpm_put_ops().
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
---
drivers/char/tpm/tpm-interface.c | 14 +++++++++-----
security/keys/trusted.c | 5 +++++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1b4f95c13e00..d9ace5480665 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -320,18 +320,22 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
if (!chip)
return -ENODEV;
- for (i = 0; i < chip->nr_allocated_banks; i++)
- if (digests[i].alg_id != chip->allocated_banks[i].alg_id)
- return -EINVAL;
+ for (i = 0; i < chip->nr_allocated_banks; i++) {
+ if (digests[i].alg_id != chip->allocated_banks[i].alg_id) {
+ rc = EINVAL;
+ goto out;
+ }
+ }
if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_pcr_extend(chip, pcr_idx, digests);
- tpm_put_ops(chip);
- return rc;
+ goto out;
}
rc = tpm1_pcr_extend(chip, pcr_idx, digests[0].digest,
"attempting extend a PCR value");
+
+out:
tpm_put_ops(chip);
return rc;
}
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ade699131065..1fbd77816610 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1228,11 +1228,16 @@ static int __init trusted_shash_alloc(void)
static int __init init_digests(void)
{
+ int i;
+
digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
GFP_KERNEL);
if (!digests)
return -ENOMEM;
+ for (i = 0; i < chip->nr_allocated_banks; i++)
+ digests[i].alg_id = chip->allocated_banks[i].alg_id;
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH] security: ima: make use of kmemdup
From: Saiyam Doshi @ 2019-09-13 16:35 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, jmorris, serge
Cc: linux-integrity, linux-security-module, linux-kernel
Replace call to kmalloc followed by memcpy with a direct call
to kmemdup to achieve same functionality.
Signed-off-by: Saiyam Doshi <saiyamdoshi.in@gmail.com>
---
security/integrity/ima/ima_policy.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 6df7f641ff66..1bd77c5eaeb7 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -262,15 +262,14 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
struct ima_rule_entry *nentry;
int i, result;
- nentry = kmalloc(sizeof(*nentry), GFP_KERNEL);
- if (!nentry)
- return NULL;
-
/*
* Immutable elements are copied over as pointers and data; only
* lsm rules can change
*/
- memcpy(nentry, entry, sizeof(*nentry));
+ nentry = kmemdup(entry, sizeof(*nentry), GFP_KERNEL);
+ if (!nentry)
+ return NULL;
+
memset(nentry->lsm, 0, FIELD_SIZEOF(struct ima_rule_entry, lsm));
for (i = 0; i < MAX_LSM_RULES; i++) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: Tetsuo Handa @ 2019-09-13 13:41 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: James Morris, linux-security-module
In-Reply-To: <289ebc65-8444-37e3-e54e-21b55d2c9192@i-love.sakura.ne.jp>
Hello, Stephen Rothwell.
Can you add https://scm.osdn.net/gitroot/tomoyo/tomoyo-test1.git#master
into the list for linux-next.git tree?
On 2019/09/03 15:52, Tetsuo Handa wrote:
> On 2019/07/07 11:50, James Morris wrote:
>> On Sat, 6 Jul 2019, James Morris wrote:
>>
>>> On Thu, 4 Jul 2019, Tetsuo Handa wrote:
>>>
>>>> Hello.
>>>>
>>>> Since it seems that Al has no comments, I'd like to send this patch to
>>>> linux-next.git . What should I do? Do I need to set up a git tree?
>>>
>>> Yes, you can create one at github or similar.
>>
>> Also notify Stephen Rothwell of the location of your -next branch, so it
>> gets pulled into his tree.
>>
>
> I executed commands shown below. Since I'm not familiar with git management,
> I want to use only master branch. Is this sequence correct?
>
> # Upon initialization
> git clone https://scm.osdn.net/gitroot/tomoyo/tomoyo-test1.git
> cd tomoyo-test1/
> git remote add upstream git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git remote update upstream
> git merge upstream/master
> git push -u origin master
>
> # When making changes
> git remote update upstream
> git merge upstream/master
> git am 0001-tomoyo-Don-t-check-open-getattr-permission-on-socket.patch
> git push -u origin master
>
^ permalink raw reply
* [GIT PULL] integrity subsystem updates for v5.4
From: Mimi Zohar @ 2019-09-11 21:29 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-integrity, linux-kernel
Hi Linus,
The major feature in this pull request is IMA support for measuring
and appraising appended file signatures. In addition are a couple of
bug fixes and code cleanup to use struct_size().
In addition to the PE/COFF and IMA xattr signatures, the kexec kernel
image may be signed with an appended signature, using the same
scripts/sign-file tool that is used to sign kernel modules.
Similarly, the initramfs may contain an appended signature.
(Stephen is carrying a patch to address a merge conflict with the
security tree.)
thanks,
Mimi
The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b:
Linux 5.3-rc2 (2019-07-28 12:47:02 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
for you to fetch changes up to 2a7f0e53daf29ca6dc9fbe2a27158f13474ec1b5:
ima: ima_api: Use struct_size() in kzalloc() (2019-08-29 14:23:30 -0400)
----------------------------------------------------------------
Gustavo A. R. Silva (2):
ima: use struct_size() in kzalloc()
ima: ima_api: Use struct_size() in kzalloc()
Mimi Zohar (2):
ima: initialize the "template" field with the default template
sefltest/ima: support appended signatures (modsig)
Sascha Hauer (2):
ima: always return negative code for error
ima: fix freeing ongoing ahash_request
Stephen Rothwell (1):
MODSIGN: make new include file self contained
Thiago Jung Bauermann (11):
MODSIGN: Export module signature definitions
PKCS#7: Refactor verify_pkcs7_signature()
PKCS#7: Introduce pkcs7_get_digest()
integrity: Select CONFIG_KEYS instead of depending on it
ima: Add modsig appraise_type option for module-style appended signatures
ima: Factor xattr_verify() out of ima_appraise_measurement()
ima: Implement support for module-style appended signatures
ima: Collect modsig
ima: Define ima-modsig template
ima: Store the measurement again when appraising a modsig
ima: Fix use after free in ima_read_modsig()
Documentation/ABI/testing/ima_policy | 6 +-
Documentation/security/IMA-templates.rst | 3 +
arch/s390/Kconfig | 2 +-
arch/s390/kernel/machine_kexec_file.c | 24 +--
certs/system_keyring.c | 61 +++++--
crypto/asymmetric_keys/pkcs7_verify.c | 33 ++++
include/crypto/pkcs7.h | 4 +
include/linux/module.h | 3 -
include/linux/module_signature.h | 46 +++++
include/linux/verification.h | 10 ++
init/Kconfig | 6 +-
kernel/Makefile | 1 +
kernel/module.c | 1 +
kernel/module_signature.c | 46 +++++
kernel/module_signing.c | 56 +-----
scripts/Makefile | 2 +-
security/integrity/Kconfig | 2 +-
security/integrity/digsig.c | 43 ++++-
security/integrity/ima/Kconfig | 13 ++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 60 ++++++-
security/integrity/ima/ima_api.c | 27 ++-
security/integrity/ima/ima_appraise.c | 194 ++++++++++++++-------
security/integrity/ima/ima_crypto.c | 10 +-
security/integrity/ima/ima_main.c | 24 ++-
security/integrity/ima/ima_modsig.c | 168 ++++++++++++++++++
security/integrity/ima/ima_policy.c | 71 ++++++--
security/integrity/ima/ima_template.c | 31 +++-
security/integrity/ima/ima_template_lib.c | 64 ++++++-
security/integrity/ima/ima_template_lib.h | 4 +
security/integrity/integrity.h | 20 +++
.../selftests/kexec/test_kexec_file_load.sh | 38 +++-
32 files changed, 871 insertions(+), 203 deletions(-)
create mode 100644 include/linux/module_signature.h
create mode 100644 kernel/module_signature.c
create mode 100644 security/integrity/ima/ima_modsig.c
^ permalink raw reply
* Inquiry 11/Sept/2019
From: Julian Smith @ 2019-09-11 12:11 UTC (permalink / raw)
To: linux-security-module
Hi,friend,
This is Julian Smith and i am purchasing manager from E-cloth Co.,LTD in the UK.
We are glad to know about your company from the web and we are interested in your products.
Could you kindly send us your Latest catalog and price list for our trial order.
Thanks and Best Regards,
Ms Julian Smith
Purchasing Manager
E-cloth Co.,LTD
^ permalink raw reply
* Re: [PATCH v6 00/12] add integrity and security to TPM2 transactions
From: Jarkko Sakkinen @ 2019-09-11 9:40 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190911084249.GA7436@linux.intel.com>
On Wed, Sep 11, 2019 at 09:42:49AM +0100, Jarkko Sakkinen wrote:
> On Tue, Sep 10, 2019 at 05:21:32PM +0100, Jarkko Sakkinen wrote:
> > On Mon, Sep 09, 2019 at 01:16:48PM +0100, James Bottomley wrote:
> > > Link to previous cover letter:
> > >
> > > https://lore.kernel.org/linux-integrity/1540193596.3202.7.camel@HansenPartnership.com/
> > >
> > > This is marked v6 instead of v5 because I did a v5 after feedback on v4
> > > but didn't get around to posting it and then had to rework the whole of
> > > the kernel space handling while I was on holiday. I also added the
> > > documentation of how the whole thing works and the rationale for doing
> > > it in tpm-security.rst (patch 11). The main reason for doing this now
> > > is so we have something to discuss at Plumbers.
> > >
> > > The new patch set implements the various splits requested, but the main
> > > changes are that the kernel space is gone and is replaced by a context
> > > save and restore of the generated null seed. This is easier to handle
> > > than a full kernel space given the new threading for TPM spaces, but
> > > conceptually it is still very like a space. I've also made whether
> > > integrity and encryption is turned on a Kconfig option.
> > >
> > > James
> >
> > So... is there a changelog for the revisions?
>
> This also desperately needs a cover letter with the full rationale and
> not just a link to an aged cover letter. I have bigger problems with the
> form than the function ATM.
>
> TPM's threat model does not cover hardware attacks. It is hardware
> designed to give some protection against software attacks. If I were
> sending these patches I would start to look for an angle from that
> perspective.
The rationale can be essentially just that since there is often lots of
*software* running outside the CPU on different cores all around the HW
platform, this will add to defense in depth. I'm not looking for
anything more rockety sciency than that.
I think that was the key lesson from TPM Genie.
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 00/12] add integrity and security to TPM2 transactions
From: Jarkko Sakkinen @ 2019-09-11 8:42 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190910162132.GA11338@linux.intel.com>
On Tue, Sep 10, 2019 at 05:21:32PM +0100, Jarkko Sakkinen wrote:
> On Mon, Sep 09, 2019 at 01:16:48PM +0100, James Bottomley wrote:
> > Link to previous cover letter:
> >
> > https://lore.kernel.org/linux-integrity/1540193596.3202.7.camel@HansenPartnership.com/
> >
> > This is marked v6 instead of v5 because I did a v5 after feedback on v4
> > but didn't get around to posting it and then had to rework the whole of
> > the kernel space handling while I was on holiday. I also added the
> > documentation of how the whole thing works and the rationale for doing
> > it in tpm-security.rst (patch 11). The main reason for doing this now
> > is so we have something to discuss at Plumbers.
> >
> > The new patch set implements the various splits requested, but the main
> > changes are that the kernel space is gone and is replaced by a context
> > save and restore of the generated null seed. This is easier to handle
> > than a full kernel space given the new threading for TPM spaces, but
> > conceptually it is still very like a space. I've also made whether
> > integrity and encryption is turned on a Kconfig option.
> >
> > James
>
> So... is there a changelog for the revisions?
This also desperately needs a cover letter with the full rationale and
not just a link to an aged cover letter. I have bigger problems with the
form than the function ATM.
TPM's threat model does not cover hardware attacks. It is hardware
designed to give some protection against software attacks. If I were
sending these patches I would start to look for an angle from that
perspective.
/Jarkko
^ permalink raw reply
* Re: [PATCH 2/2] mtd: phram,slram: Disable when the kernel is locked down
From: Ben Hutchings @ 2019-09-10 23:43 UTC (permalink / raw)
To: Richard Weinberger, James Morris
Cc: Matthew Garrett, David Howells, Joern Engel, LSM List, linux-mtd
In-Reply-To: <CAFLxGvxRVwt0=wtKJnZB6s+VDCoGT3vsW27P2MECO999sJKAHw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]
On Wed, 2019-09-11 at 00:18 +0200, Richard Weinberger wrote:
> On Tue, Sep 10, 2019 at 5:17 PM James Morris <jmorris@namei.org> wrote:
> > On Tue, 10 Sep 2019, Matthew Garrett wrote:
> >
> > > On Fri, Aug 30, 2019 at 11:47 AM Ben Hutchings <ben@decadent.org.uk> wrote:
> > > > These drivers allow mapping arbitrary memory ranges as MTD devices.
> > > > This should be disabled to preserve the kernel's integrity when it is
> > > > locked down.
> > > >
> > > > * Add the HWPARAM flag to the module parameters
> > > > * When slram is built-in, it uses __setup() to read kernel parameters,
> > > > so add an explicit check security_locked_down() check
> > > >
> > > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > > > Cc: Matthew Garrett <mjg59@google.com>
> > > > Cc: David Howells <dhowells@redhat.com>
> > > > Cc: Joern Engel <joern@lazybastard.org>
> > > > Cc: linux-mtd@lists.infradead.org
> > >
> > > Reviewed-by: Matthew Garrett <mjg59@google.com>
> > >
> > > James, should I pick patches like this up and send them to you, or
> > > will you queue them directly after they're acked?
> >
> > As long as I'm on the to or cc when they're acked, I can grab them.
>
> Acked-by: Richard Weinberger <richard@nod.at>
>
> BTW: I don't have 1/2 in my inbox, is it also MTD related?
No, that was for some other drivers (comedi) that allow setting I/O
addresses from user-space.
Ben.
--
Ben Hutchings
The obvious mathematical breakthrough [to break modern encryption]
would be development of an easy way to factor large prime numbers.
- Bill Gates
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] mtd: phram,slram: Disable when the kernel is locked down
From: Richard Weinberger @ 2019-09-10 22:18 UTC (permalink / raw)
To: James Morris
Cc: Matthew Garrett, David Howells, Joern Engel, LSM List,
Ben Hutchings, linux-mtd
In-Reply-To: <alpine.LRH.2.21.1909100816170.3709@namei.org>
On Tue, Sep 10, 2019 at 5:17 PM James Morris <jmorris@namei.org> wrote:
>
> On Tue, 10 Sep 2019, Matthew Garrett wrote:
>
> > On Fri, Aug 30, 2019 at 11:47 AM Ben Hutchings <ben@decadent.org.uk> wrote:
> > >
> > > These drivers allow mapping arbitrary memory ranges as MTD devices.
> > > This should be disabled to preserve the kernel's integrity when it is
> > > locked down.
> > >
> > > * Add the HWPARAM flag to the module parameters
> > > * When slram is built-in, it uses __setup() to read kernel parameters,
> > > so add an explicit check security_locked_down() check
> > >
> > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > > Cc: Matthew Garrett <mjg59@google.com>
> > > Cc: David Howells <dhowells@redhat.com>
> > > Cc: Joern Engel <joern@lazybastard.org>
> > > Cc: linux-mtd@lists.infradead.org
> >
> > Reviewed-by: Matthew Garrett <mjg59@google.com>
> >
> > James, should I pick patches like this up and send them to you, or
> > will you queue them directly after they're acked?
>
> As long as I'm on the to or cc when they're acked, I can grab them.
Acked-by: Richard Weinberger <richard@nod.at>
BTW: I don't have 1/2 in my inbox, is it also MTD related?
--
Thanks,
//richard
^ permalink raw reply
* [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: James Morris @ 2019-09-10 22:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-kernel, linux-security-module, Matthew Garrett,
David Howells
Hi Linus,
This is the latest iteration of the kernel lockdown patchset, from Matthew
Garrett, David Howells and others.
From the original description:
This patchset introduces an optional kernel lockdown feature, intended
to strengthen the boundary between UID 0 and the kernel. When enabled,
various pieces of kernel functionality are restricted. Applications that
rely on low-level access to either hardware or the kernel may cease
working as a result - therefore this should not be enabled without
appropriate evaluation beforehand.
The majority of mainstream distributions have been carrying variants of
this patchset for many years now, so there's value in providing a
doesn't meet every distribution requirement, but gets us much closer to
not requiring external patches.
There are two major changes since this was last proposed for mainline:
1. Separating lockdown from EFI secure boot. Background discussion is
covered here: https://lwn.net/Articles/751061/
2. Implementation as an LSM, with a default stackable lockdown LSM module.
This allows the lockdown feature to be policy-driven, rather than encoding
an implicit policy within the mechanism.
A new locked_down LSM hook is provided to
allow LSMs to make a policy decision around whether kernel functionality
that would allow tampering with or examining the runtime state of the
kernel should be permitted.
The included lockdown LSM provides an implementation with a simple policy
intended for general purpose use. This policy provides a coarse level of
granularity, controllable via the kernel command line:
lockdown={integrity|confidentiality}
Enable the kernel lockdown feature. If set to integrity, kernel features
that allow userland to modify the running kernel are disabled. If set to
confidentiality, kernel features that allow userland to extract
confidential information from the kernel are also disabled.
This may also be controlled via /sys/kernel/security/lockdown and
overriden by kernel configuration.
New or existing LSMs may implement finer-grained controls of the lockdown
features. Refer to the lockdown_reason documentation in
include/linux/security.h for details.
The lockdown feature has had signficant design feedback and review across
many subsystems. This code has been in linux-next for some weeks, with a
few fixes applied along the way.
Stephen Rothwell noted that commit 9d1f8be5cf42 ("bpf: Restrict bpf when
kernel lockdown is in confidentiality mode") is missing a Signed-off-by
from its author. Matthew responded that he is providing this under
category (c) of the DCO.
Several simple conflicts have been identified by Stephen in linux-next:
vfs tree:
https://www.lkml.org/lkml/2019/8/12/21
Linus' tree:
https://www.lkml.org/lkml/2019/8/12/26
https://lkml.org/lkml/2019/8/20/1415
https://lore.kernel.org/lkml/20190821130513.0038df28@canb.auug.org.au/
https://lore.kernel.org/lkml/20190821130957.407d9c10@canb.auug.org.au/
Keys tree:
https://lore.kernel.org/lkml/20190829150609.7ae3c4ee@canb.auug.org.au/
Please consider merging for v5.4.
The following changes since commit
0ecfebd2b52404ae0c54a878c872bb93363ada36:
Linux 5.2 (2019-07-07 15:41:56 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-lockdown
for you to fetch changes up to 45893a0abee6b5fd52994a3a1095735aeaec472b:
kexec: Fix file verification on S390 (2019-09-10 13:27:51 +0100)
----------------------------------------------------------------
Dave Young (1):
lockdown: Copy secure_boot flag in boot params across kexec reboot
David Howells (10):
lockdown: Enforce module signatures if the kernel is locked down
lockdown: Prohibit PCMCIA CIS storage when the kernel is locked down
lockdown: Lock down TIOCSSERIAL
lockdown: Lock down module params that specify hardware parameters (eg. ioport)
x86/mmiotrace: Lock down the testmmiotrace module
lockdown: Lock down /proc/kcore
lockdown: Lock down tracing and perf kprobes when in confidentiality mode
bpf: Restrict bpf when kernel lockdown is in confidentiality mode
lockdown: Lock down perf when in confidentiality mode
debugfs: Restrict debugfs when the kernel is locked down
Jiri Bohac (2):
kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
kexec_file: Restrict at runtime if the kernel is locked down
Josh Boyer (2):
hibernate: Disable when the kernel is locked down
acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
Linn Crosetto (1):
acpi: Disable ACPI table override if the kernel is locked down
Matthew Garrett (15):
security: Support early LSMs
security: Add a "locked down" LSM hook
security: Add a static lockdown policy LSM
lockdown: Restrict /dev/{mem,kmem,port} when the kernel is locked down
kexec_load: Disable at runtime if the kernel is locked down
PCI: Lock down BAR access when the kernel is locked down
x86: Lock down IO port access when the kernel is locked down
x86/msr: Restrict MSR access when the kernel is locked down
ACPI: Limit access to custom_method when the kernel is locked down
kexec: Allow kexec_file() with appropriate IMA policy when locked down
tracefs: Restrict tracefs when the kernel is locked down
efi: Restrict efivar_ssdt_load when the kernel is locked down
lockdown: Print current->comm in restriction messages
security: constify some arrays in lockdown LSM
kexec: Fix file verification on S390
Documentation/admin-guide/kernel-parameters.txt | 9 ++
arch/arm64/Kconfig | 6 +-
arch/s390/Kconfig | 2 +-
arch/s390/configs/debug_defconfig | 2 +-
arch/s390/configs/defconfig | 2 +-
arch/s390/configs/performance_defconfig | 2 +-
arch/s390/kernel/kexec_elf.c | 4 +-
arch/s390/kernel/kexec_image.c | 4 +-
arch/s390/kernel/machine_kexec_file.c | 4 +-
arch/x86/Kconfig | 20 ++-
arch/x86/boot/compressed/acpi.c | 19 ++-
arch/x86/include/asm/acpi.h | 9 ++
arch/x86/include/asm/x86_init.h | 2 +
arch/x86/kernel/acpi/boot.c | 5 +
arch/x86/kernel/ima_arch.c | 4 +-
arch/x86/kernel/ioport.c | 7 +-
arch/x86/kernel/kexec-bzimage64.c | 1 +
arch/x86/kernel/msr.c | 8 +
arch/x86/kernel/x86_init.c | 1 +
arch/x86/mm/testmmiotrace.c | 5 +
crypto/asymmetric_keys/verify_pefile.c | 4 +-
drivers/acpi/custom_method.c | 6 +
drivers/acpi/osl.c | 14 +-
drivers/acpi/tables.c | 6 +
drivers/char/mem.c | 7 +-
drivers/firmware/efi/efi.c | 6 +
drivers/pci/pci-sysfs.c | 16 ++
drivers/pci/proc.c | 14 +-
drivers/pci/syscall.c | 4 +-
drivers/pcmcia/cistpl.c | 5 +
drivers/tty/serial/serial_core.c | 5 +
fs/debugfs/file.c | 30 ++++
fs/debugfs/inode.c | 32 +++-
fs/proc/kcore.c | 6 +
fs/tracefs/inode.c | 42 +++++-
include/asm-generic/vmlinux.lds.h | 8 +-
include/linux/acpi.h | 6 +
include/linux/ima.h | 9 ++
include/linux/kexec.h | 4 +-
include/linux/lsm_hooks.h | 13 ++
include/linux/security.h | 59 ++++++++
init/Kconfig | 5 +
init/main.c | 1 +
kernel/events/core.c | 7 +
kernel/kexec.c | 8 +
kernel/kexec_file.c | 68 +++++++--
kernel/module.c | 37 ++++-
kernel/params.c | 21 ++-
kernel/power/hibernate.c | 3 +-
kernel/trace/bpf_trace.c | 10 ++
kernel/trace/trace_kprobe.c | 5 +
security/Kconfig | 11 +-
security/Makefile | 2 +
security/integrity/ima/Kconfig | 2 +-
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_main.c | 4 +-
security/integrity/ima/ima_policy.c | 50 +++++++
security/lockdown/Kconfig | 47 ++++++
security/lockdown/Makefile | 1 +
security/lockdown/lockdown.c | 191 ++++++++++++++++++++++++
security/security.c | 56 ++++++-
61 files changed, 864 insertions(+), 79 deletions(-)
create mode 100644 security/lockdown/Kconfig
create mode 100644 security/lockdown/Makefile
create mode 100644 security/lockdown/lockdown.c
^ permalink raw reply
* Re: [PATCH v6 00/12] add integrity and security to TPM2 transactions
From: James Bottomley @ 2019-09-10 16:29 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190910162132.GA11338@linux.intel.com>
On Tue, 2019-09-10 at 17:21 +0100, Jarkko Sakkinen wrote:
> On Mon, Sep 09, 2019 at 01:16:48PM +0100, James Bottomley wrote:
> > Link to previous cover letter:
> >
> > https://lore.kernel.org/linux-integrity/1540193596.3202.7.camel@Han
> > senPartnership.com/
> >
> > This is marked v6 instead of v5 because I did a v5 after feedback
> > on v4
> > but didn't get around to posting it and then had to rework the
> > whole of
> > the kernel space handling while I was on holiday. I also added the
> > documentation of how the whole thing works and the rationale for
> > doing
> > it in tpm-security.rst (patch 11). The main reason for doing this
> > now
> > is so we have something to discuss at Plumbers.
> >
> > The new patch set implements the various splits requested, but the
> > main
> > changes are that the kernel space is gone and is replaced by a
> > context
> > save and restore of the generated null seed. This is easier to
> > handle
> > than a full kernel space given the new threading for TPM spaces,
> > but
> > conceptually it is still very like a space. I've also made whether
> > integrity and encryption is turned on a Kconfig option.
> >
> > James
>
> So... is there a changelog for the revisions?
Well, yes, standard way: they're in the individual patches under the '-
--' prefixed with v6:
James
^ permalink raw reply
* Re: [PATCH v6 00/12] add integrity and security to TPM2 transactions
From: Jarkko Sakkinen @ 2019-09-10 16:21 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <1568031408.6613.29.camel@HansenPartnership.com>
On Mon, Sep 09, 2019 at 01:16:48PM +0100, James Bottomley wrote:
> Link to previous cover letter:
>
> https://lore.kernel.org/linux-integrity/1540193596.3202.7.camel@HansenPartnership.com/
>
> This is marked v6 instead of v5 because I did a v5 after feedback on v4
> but didn't get around to posting it and then had to rework the whole of
> the kernel space handling while I was on holiday. I also added the
> documentation of how the whole thing works and the rationale for doing
> it in tpm-security.rst (patch 11). The main reason for doing this now
> is so we have something to discuss at Plumbers.
>
> The new patch set implements the various splits requested, but the main
> changes are that the kernel space is gone and is replaced by a context
> save and restore of the generated null seed. This is easier to handle
> than a full kernel space given the new threading for TPM spaces, but
> conceptually it is still very like a space. I've also made whether
> integrity and encryption is turned on a Kconfig option.
>
> James
So... is there a changelog for the revisions?
/Jarkko
^ permalink raw reply
* Re: [PATCH 2/2] mtd: phram,slram: Disable when the kernel is locked down
From: James Morris @ 2019-09-10 15:17 UTC (permalink / raw)
To: Matthew Garrett
Cc: Ben Hutchings, LSM List, David Howells, Joern Engel, linux-mtd
In-Reply-To: <CACdnJuutzv+0nPKeizsiaix5YtYHU4RSoH-hPFfG1Z8sW_yy2w@mail.gmail.com>
On Tue, 10 Sep 2019, Matthew Garrett wrote:
> On Fri, Aug 30, 2019 at 11:47 AM Ben Hutchings <ben@decadent.org.uk> wrote:
> >
> > These drivers allow mapping arbitrary memory ranges as MTD devices.
> > This should be disabled to preserve the kernel's integrity when it is
> > locked down.
> >
> > * Add the HWPARAM flag to the module parameters
> > * When slram is built-in, it uses __setup() to read kernel parameters,
> > so add an explicit check security_locked_down() check
> >
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > Cc: Matthew Garrett <mjg59@google.com>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Joern Engel <joern@lazybastard.org>
> > Cc: linux-mtd@lists.infradead.org
>
> Reviewed-by: Matthew Garrett <mjg59@google.com>
>
> James, should I pick patches like this up and send them to you, or
> will you queue them directly after they're acked?
As long as I'm on the to or cc when they're acked, I can grab them.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox