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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 13 Jan 2022 05:12:34 +0000 From: Tyler Wear <twear@...cinc.com> To: Alexei Starovoitov <alexei.starovoitov@...il.com>, "Tyler Wear (QUIC)" <quic_twear@...cinc.com> CC: Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Maciej Żenczykowski <maze@...gle.com>, Yonghong Song <yhs@...com>, Martin KaFai Lau <kafai@...com>, Toke Høiland-Jørgensen <toke@...hat.com>, Daniel Borkmann <daniel@...earbox.net>, Song Liu <song@...nel.org> Subject: RE: [PATCH bpf-next v6 1/2] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB > -----Original Message----- > From: Alexei Starovoitov <alexei.starovoitov@...il.com> > Sent: Wednesday, January 12, 2022 6:14 PM > To: Tyler Wear (QUIC) <quic_twear@...cinc.com> > Cc: Network Development <netdev@...r.kernel.org>; bpf > <bpf@...r.kernel.org>; Maciej Żenczykowski <maze@...gle.com>; > Yonghong Song <yhs@...com>; Martin KaFai Lau <kafai@...com>; Toke > Høiland-Jørgensen <toke@...hat.com>; Daniel Borkmann > <daniel@...earbox.net>; Song Liu <song@...nel.org> > Subject: Re: [PATCH bpf-next v6 1/2] Add skb_store_bytes() for > BPF_PROG_TYPE_CGROUP_SKB > > WARNING: This email originated from outside of Qualcomm. Please be wary > of any links or attachments, and do not enable macros. > > On Wed, Jan 12, 2022 at 5:15 PM Tyler Wear <quic_twear@...cinc.com> > wrote: > > > > Need to modify the ds field to support upcoming Wifi QoS Alliance spec. > > Instead of adding generic function for just modifying the ds field, > > add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. > > This allows other fields in the network and transport header to be > > modified in the future. > > > > Checksum API's also need to be added for completeness. > > > > It is not possible to use CGROUP_(SET|GET)SOCKOPT since the policy may > > change during runtime and would result in a large number of entries > > with wildcards. > > > > V4 patch fixes warnings and errors from checkpatch. > > > > The existing check for bpf_try_make_writable() should mean that > > skb_share_check() is not needed. > > > > Signed-off-by: Tyler Wear <quic_twear@...cinc.com> > > --- > > net/core/filter.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/net/core/filter.c b/net/core/filter.c index > > 6102f093d59a..f30d939cb4cf 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -7299,6 +7299,18 @@ cg_skb_func_proto(enum bpf_func_id func_id, > const struct bpf_prog *prog) > > return &bpf_sk_storage_delete_proto; > > case BPF_FUNC_perf_event_output: > > return &bpf_skb_event_output_proto; > > + case BPF_FUNC_skb_store_bytes: > > + return &bpf_skb_store_bytes_proto; > > + case BPF_FUNC_csum_update: > > + return &bpf_csum_update_proto; > > + case BPF_FUNC_csum_level: > > + return &bpf_csum_level_proto; > > + case BPF_FUNC_l3_csum_replace: > > + return &bpf_l3_csum_replace_proto; > > + case BPF_FUNC_l4_csum_replace: > > + return &bpf_l4_csum_replace_proto; > > + case BPF_FUNC_csum_diff: > > + return &bpf_csum_diff_proto; > > This is wrong. > CGROUP_INET_EGRESS bpf prog cannot arbitrary change packet data. > The networking stack populated the IP header at that point. > If the prog changes it to something else it will be confusing other layers of > stack. neigh(L2) will be wrong, etc. > We can still change certain things in the packet, but not arbitrary bytes. > > We cannot change the DS field directly in the packet either. > It can only be changed by changing its value in the socket. Why is the DS field unchangeable, but ecn is changeable? > > TC layer is where packet modifications are allowed.
Powered by blists - more mailing lists