Re: Re: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution

[Yonghong Song]

On 9/14/19 5:56 PM, Yonghong Song wrote:
> 
> 
> On 9/10/19 12:55 PM, KP Singh wrote:
>> From: KP Singh <kpsingh@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>>    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;
>> +}
>> +
[...]