On Tue, Aug 15, 2023 at 11:40 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
>
>
> On 8/14/23 7:45 PM, Yafang Shao wrote:
> > On Tue, Aug 15, 2023 at 8:28 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
> >>
> >>
> >>
> >> On 8/14/23 7:33 AM, Yafang Shao wrote:
> >>> Add a new bpf_current_capable kfunc to check whether the current task
> >>> has a specific capability. In our use case, we will use it in a lsm bpf
> >>> program to help identify if the user operation is permitted.
> >>>
> >>> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> >>> ---
> >>> kernel/bpf/helpers.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> >>> index eb91cae..bbee7ea 100644
> >>> --- a/kernel/bpf/helpers.c
> >>> +++ b/kernel/bpf/helpers.c
> >>> @@ -2429,6 +2429,11 @@ __bpf_kfunc void bpf_rcu_read_unlock(void)
> >>> rcu_read_unlock();
> >>> }
> >>>
> >>> +__bpf_kfunc bool bpf_current_capable(int cap)
> >>> +{
> >>> + return has_capability(current, cap);
> >>> +}
> >>
> >> Since you are testing against 'current' capabilities, I assume
> >> that the context should be process. Otherwise, you are testing
> >> against random task which does not make much sense.
> >
> > It is in the process context.
> >
> >>
> >> Since you are testing against 'current' cap, and if the capability
> >> for that task is stable, you do not need this kfunc.
> >> You can test cap in user space and pass it into the bpf program.
> >>
> >> But if the cap for your process may change in the middle of
> >> run, then you indeed need bpf prog to test capability in real time.
> >> Is this your use case and could you describe in in more detail?
> >
> > After we convert the capability of our networking bpf program from
> > CAP_SYS_ADMIN to CAP_BPF+CAP_NET_ADMIN to enhance the security, we
> > encountered the "pointer comparison prohibited" error, because
>
> Could you give a reproducible test case for this verifier failure
> with CAP_BPF+CAP_NET_ADMIN capability? Is this due to packet pointer
> or something else? Maybe verifier needs to be improved in these
> cases without violating the promise of allow_ptr_leaks?
Here it is.
SEC("cls-ingress")
int ingress(struct __sk_buff *skb)
{
struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr);
if ((long)(iph + 1) > (long)skb->data_end)
return TC_ACT_STOLEN;
return TC_ACT_OK;
}
In this bpf prog, it will compare the pointer iph with skb->data_end,
and thus it will fail the verifier.
--
Regards
Yafang
