netdev - Re: [PATCH v3 net] bpf: add bpf_sk_netns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <58991030.1010102@iogearbox.net>
Date:   Tue, 07 Feb 2017 01:09:20 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Alexei Starovoitov <ast@...com>,
        "David S . Miller" <davem@...emloft.net>
CC:     David Ahern <dsa@...ulusnetworks.com>, Tejun Heo <tj@...nel.org>,
        Andy Lutomirski <luto@...capital.net>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH v3 net] bpf: add bpf_sk_netns_id() helper

On 02/07/2017 01:02 AM, Alexei Starovoitov wrote:
> On 2/6/17 3:39 PM, Daniel Borkmann wrote:
>> On 02/04/2017 04:34 AM, Alexei Starovoitov wrote:
>> [...]
>>> +BPF_CALL_1(bpf_skb_netns_id, struct sk_buff *, skb)
>>> +{
>>> +    struct net_device *dev = skb->dev;
>>> +
>>> +    if (!dev)
>>> +        return 0;
>>> +    return proc_get_ns_devid_inum(&dev_net(dev)->ns);
>>> +}
>>> +
>>> +static const struct bpf_func_proto bpf_skb_netns_id_proto = {
>>> +    .func        = bpf_skb_netns_id,
>>> +    .gpl_only    = false,
>>> +    .ret_type    = RET_INTEGER,
>>> +    .arg1_type    = ARG_PTR_TO_CTX,
>>> +};
>>> +
>>>   static const struct bpf_func_proto *
>>>   sk_filter_func_proto(enum bpf_func_id func_id)
>>>   {
>>> @@ -2620,6 +2649,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
>>>       case BPF_FUNC_trace_printk:
>>>           if (capable(CAP_SYS_ADMIN))
>>>               return bpf_get_trace_printk_proto();
>>> +    case BPF_FUNC_sk_netns_id:
>>> +        return &bpf_skb_netns_id_proto;
>>>       default:
>>>           return NULL;
>>>       }
>>
>> Btw, I think here's an oversight that would still need to be
>> fixed. Above would mean that trace printk from unprivileged would
>> fall through and use &bpf_skb_netns_id_proto as proto now instead
>> of NULL. So BPF_FUNC_sk_netns_id needs to be placed above the
>> BPF_FUNC_trace_printk case, not in its fall-through path. Looks
>> like Chenbo in his get_socket_cookie missed this, too. Other than
>> that BPF bits seem good to me.
>
> Ahh, right. Good catch.
> I'll add 'else return NULL;' otherwise somebody might step on it again.
> Thanks Daniel!

I guess an explicit comment "/* fall-through */" would also be fine
and get noticed. Thanks!

> Eric,
> still waiting for your review of nsfs.c bits.
>