netdev - Re: [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <660435c4-6cea-9648-0106-afb3ab6910fc@cumulusnetworks.com>
Date:   Tue, 25 Oct 2016 20:05:39 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org
Cc:     daniel@...que.org, ast@...com
Subject: Re: [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable
 sock modifications

On 10/25/16 5:28 PM, Daniel Borkmann wrote:
>> +BPF_CALL_3(bpf_sock_store_u32, struct sock *, sk, u32, offset, u32, val)
>> +{
>> +    u8 *ptr = (u8 *)sk;
>> +
>> +    if (unlikely(offset > sizeof(*sk)))
>> +        return -EFAULT;
>> +
>> +    *((u32 *)ptr) = val;
>> +
>> +    return 0;
>> +}
> 
> Seems strange to me. So, this helper allows to overwrite arbitrary memory
> of a struct sock instance. Potentially we could crash the kernel.
> 
> And in your sock_filter_convert_ctx_access(), you already implement inline
> read/write for the context ...
> 
> Your demo code does in pseudocode:
> 
>   r1 = sk
>   r2 = offsetof(struct bpf_sock, bound_dev_if)
>   r3 = idx
>   r1->sk_bound_dev_if = idx
>   sock_store_u32(r1, r2, r3) // updates sk_bound_dev_if again to idx
>   return 1
> 
> Dropping that helper from the patch, the only thing a program can do here
> is to read/write the sk_bound_dev_if helper per cgroup. Hmm ... dunno. So
> this really has to be for cgroups v2, right?

Showing my inexperience with the bpf code. The helper can be dropped. I'll do that for v2.

Yes, Daniel's patch set provides the infra for this one and it has a cgroups v2 limitation.