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

Message-ID: <20190626192126.qkwr7hv2leich5tk@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 26 Jun 2019 12:21:28 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Stanislav Fomichev <sdf@...ichev.me>
Cc:     Stanislav Fomichev <sdf@...gle.com>, 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 Wed, Jun 26, 2019 at 12:10:21PM -0700, Stanislav Fomichev wrote:
> On 06/26, Alexei Starovoitov wrote:
> > 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?
> Sure, I can take a look if you think that it would be useful in general.
> Looks like set_fs should do the trick.

Thanks. I think it's useful.
For example see the recent sack steam issue and Eric's workaround
for older kernel to add 128k to sk_sndbuf.
If we had an ability to do adjust SO_SNDBUF from cgroup-bpf prog
when user space is doing setsockopt we could have mitigated it by
rolling bpf prog instead of patching and rebooting the kernels.
That's a bit of a stretch use case, of course.
My feeling that if not today, but really soon people will find
solid use cases for adjusting sockopt values via cgroup-bpf.