netdev - Re: [PATCH bpf-next v8 1/9] bpf: implement getsockopt and setsockopt hooks

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: <20190626185420.wzsb7v6rawn4wtzd@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 26 Jun 2019 11:54:22 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
        ast@...nel.org, daniel@...earbox.net, Martin Lau <kafai@...com>
Subject: Re: [PATCH bpf-next v8 1/9] bpf: implement getsockopt and setsockopt
 hooks

On Mon, Jun 24, 2019 at 09:24:21AM -0700, Stanislav Fomichev wrote:
> Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
> BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
> 
> BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
> BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
> Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.

getsockopt side looks good to me.
I tried to convince myself that readonly setsockopt is fine for now,
but it feels we need to make it writeable from the start.
I agree with your reasoning that doing copy_to_user is no good,
but we can do certainly do set_fs(KERNEL_DS) game.
The same way as kernel_setsockopt() is doing.
It seems quite useful to modify 'optval' before passing it to kernel.
Then bpf prog would be able to specify sane values for SO_SNDBUF
instead of rejecting them.
The alternative would be to allow bpf prog to call setsockopt
from inside, but sock is locked when prog is running,
so unlocking within helper is not going to be clean.
wdyt?