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
| ||
|
Date: Tue, 25 Jan 2022 15:53:20 -0800 From: Martin KaFai Lau <kafai@...com> To: Alexei Starovoitov <alexei.starovoitov@...il.com> CC: Jakub Sitnicki <jakub@...udflare.com>, Menglong Dong <menglong8.dong@...il.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, Mengen Sun <mengensun@...cent.com>, <flyingpeng@...cent.com>, <mungerjiang@...cent.com>, Menglong Dong <imagedong@...cent.com> Subject: Re: [PATCH bpf-next] bpf: Add document for 'dst_port' of 'struct bpf_sock' On Tue, Jan 25, 2022 at 03:02:37PM -0800, Alexei Starovoitov wrote: > On Tue, Jan 25, 2022 at 2:45 PM Martin KaFai Lau <kafai@...com> wrote: > > > > On Tue, Jan 25, 2022 at 08:24:27PM +0100, Jakub Sitnicki wrote: > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > > index b0383d371b9a..891a182a749a 100644 > > > > --- a/include/uapi/linux/bpf.h > > > > +++ b/include/uapi/linux/bpf.h > > > > @@ -5500,7 +5500,11 @@ struct bpf_sock { > > > > __u32 src_ip4; > > > > __u32 src_ip6[4]; > > > > __u32 src_port; /* host byte order */ > > > > - __u32 dst_port; /* network byte order */ > > > > + __u32 dst_port; /* low 16-bits are in network byte order, > > > > + * and high 16-bits are filled by 0. > > > > + * So the real port in host byte order is > > > > + * bpf_ntohs((__u16)dst_port). > > > > + */ > > > > __u32 dst_ip4; > > > > __u32 dst_ip6[4]; > > > > __u32 state; > > > > > > I'm probably missing something obvious, but is there anything stopping > > > us from splitting the field, so that dst_ports is 16-bit wide? > > > > > > I gave a quick check to the change below and it seems to pass verifier > > > checks and sock_field tests. > > > > > > IDK, just an idea. Didn't give it a deeper thought. > > > > > > --8<-- > > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > index 4a2f7041ebae..344d62ccafba 100644 > > > --- a/include/uapi/linux/bpf.h > > > +++ b/include/uapi/linux/bpf.h > > > @@ -5574,7 +5574,8 @@ struct bpf_sock { > > > __u32 src_ip4; > > > __u32 src_ip6[4]; > > > __u32 src_port; /* host byte order */ > > > - __u32 dst_port; /* network byte order */ > > > + __u16 unused; > > > + __u16 dst_port; /* network byte order */ > > This will break the existing bpf prog. > > I think Jakub's idea is partially expressed: > + case offsetof(struct bpf_sock, dst_port): > + bpf_ctx_record_field_size(info, sizeof(__u16)); > + return bpf_ctx_narrow_access_ok(off, size, sizeof(__u16)); > > Either 'unused' needs to be after dst_port or > bpf_sock_is_valid_access() needs to allow offset at 'unused' > and at 'dst_port'. > And allow u32 access though the size is actually u16. > Then the existing bpf progs (without recompiling) should work? Yes, I think that should work with the existing bpf progs. I suspect putting 'dst_port' first and then followed by 'unused' may be easier. That will also serve as a natural doc for the current behavior (the value is in the lower 16 bits). It can be extended to bpf_sk_lookup? bpf_sk_lookup can read at any offset of these 4 bytes, so may need to read 0 during convert_ctx_accesses?
Powered by blists - more mailing lists