lkml.org 
[lkml]   [2020]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH bpf-next v7 4/8] bpf: lsm: Implement attach, detach and execution
On 26-Mär 20:12, Alexei Starovoitov wrote:
> On Thu, Mar 26, 2020 at 03:28:19PM +0100, KP Singh wrote:
> >
> > if (arg == nr_args) {
> > - if (prog->expected_attach_type == BPF_TRACE_FEXIT) {
> > + /* BPF_LSM_MAC programs only have int and void functions they
> > + * can be attached to. When they are attached to a void function
> > + * they result in the creation of an FEXIT trampoline and when
> > + * to a function that returns an int, a MODIFY_RETURN
> > + * trampoline.
> > + */
> > + if (prog->expected_attach_type == BPF_TRACE_FEXIT ||
> > + prog->expected_attach_type == BPF_LSM_MAC) {
> > if (!t)
> > return true;
> > t = btf_type_by_id(btf, t->type);
>
> Could you add a comment here that though BPF_MODIFY_RETURN-like check
> if (ret_type != 'int') return -EINVAL;
> is _not_ done here. It is still safe, since LSM hooks have only
> void and int return types.

Good idea, I reworded the comment to make this explicit and moved
the comment to inside the if condition.

>
> > + case BPF_LSM_MAC:
> > + if (!prog->aux->attach_func_proto->type)
> > + /* The function returns void, we cannot modify its
> > + * return value.
> > + */
> > + return BPF_TRAMP_FEXIT;
> > + else
> > + return BPF_TRAMP_MODIFY_RETURN;
>
> I was thinking whether it would help performance significantly enough
> if we add a flavor of BPF_TRAMP_FEXIT that doesn't have
> BPF_TRAMP_F_CALL_ORIG.

Agreed.

> That will save the cost of nop call, but I guess indirect call due
> to lsm infra is slow enough, so this extra few cycles won't be noticeable.
> So I'm fine with it as-is. When lsm hooks will get rid of indirect call
> we can optimize it further.

Also agreed, that's the next step. :)

- KP

\
 
 \ /
  Last update: 2020-03-27 16:07    [W:0.067 / U:8.752 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site