| 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: Fri, 17 Feb 2023 10:14:02 -0800
From: Joanne Koong <joannelkoong@...il.com>
To: Toke Høiland-Jørgensen <toke@...nel.org>
Cc: bpf@...r.kernel.org, martin.lau@...nel.org, andrii@...nel.org,
memxor@...il.com, ast@...nel.org, daniel@...earbox.net,
netdev@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v10 bpf-next 9/9] selftests/bpf: tests for using dynptrs
to parse skb and xdp buffers
On Fri, Feb 17, 2023 at 5:05 AM Toke Høiland-Jørgensen <toke@...nel.org> wrote:
>
> Joanne Koong <joannelkoong@...il.com> writes:
>
> > Test skb and xdp dynptr functionality in the following ways:
>
> One question on one of the usage examples:
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> > new file mode 100644
> > index 000000000000..4c49fd42da6f
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_xdp_dynptr.c
> > @@ -0,0 +1,257 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2022 Meta */
> > +#include <stddef.h>
> > +#include <string.h>
> > +#include <linux/bpf.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/if_packet.h>
> > +#include <linux/ip.h>
> > +#include <linux/ipv6.h>
> > +#include <linux/in.h>
> > +#include <linux/udp.h>
> > +#include <linux/tcp.h>
> > +#include <linux/pkt_cls.h>
> > +#include <sys/socket.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_endian.h>
> > +#include "test_iptunnel_common.h"
> > +
> > +extern int bpf_dynptr_from_xdp(struct xdp_md *xdp, __u64 flags,
> > + struct bpf_dynptr *ptr) __ksym;
> > +extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, __u32 offset,
> > + void *buffer, __u32 buffer__sz) __ksym;
> > +extern void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *ptr, __u32 offset,
> > + void *buffer, __u32 buffer__sz) __ksym;
> > +
> > +const size_t tcphdr_sz = sizeof(struct tcphdr);
> > +const size_t udphdr_sz = sizeof(struct udphdr);
> > +const size_t ethhdr_sz = sizeof(struct ethhdr);
> > +const size_t iphdr_sz = sizeof(struct iphdr);
> > +const size_t ipv6hdr_sz = sizeof(struct ipv6hdr);
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
> > + __uint(max_entries, 256);
> > + __type(key, __u32);
> > + __type(value, __u64);
> > +} rxcnt SEC(".maps");
> > +
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_HASH);
> > + __uint(max_entries, MAX_IPTNL_ENTRIES);
> > + __type(key, struct vip);
> > + __type(value, struct iptnl_info);
> > +} vip2tnl SEC(".maps");
> > +
> > +static __always_inline void count_tx(__u32 protocol)
> > +{
> > + __u64 *rxcnt_count;
> > +
> > + rxcnt_count = bpf_map_lookup_elem(&rxcnt, &protocol);
> > + if (rxcnt_count)
> > + *rxcnt_count += 1;
> > +}
> > +
> > +static __always_inline int get_dport(void *trans_data, __u8 protocol)
> > +{
> > + struct tcphdr *th;
> > + struct udphdr *uh;
> > +
> > + switch (protocol) {
> > + case IPPROTO_TCP:
> > + th = (struct tcphdr *)trans_data;
> > + return th->dest;
> > + case IPPROTO_UDP:
> > + uh = (struct udphdr *)trans_data;
> > + return uh->dest;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static __always_inline void set_ethhdr(struct ethhdr *new_eth,
> > + const struct ethhdr *old_eth,
> > + const struct iptnl_info *tnl,
> > + __be16 h_proto)
> > +{
> > + memcpy(new_eth->h_source, old_eth->h_dest, sizeof(new_eth->h_source));
> > + memcpy(new_eth->h_dest, tnl->dmac, sizeof(new_eth->h_dest));
> > + new_eth->h_proto = h_proto;
> > +}
> > +
> > +static __always_inline int handle_ipv4(struct xdp_md *xdp, struct bpf_dynptr *xdp_ptr)
> > +{
> > + __u8 eth_buffer[ethhdr_sz + iphdr_sz + ethhdr_sz];
> > + __u8 iph_buffer_tcp[iphdr_sz + tcphdr_sz];
> > + __u8 iph_buffer_udp[iphdr_sz + udphdr_sz];
> > + struct bpf_dynptr new_xdp_ptr;
> > + struct iptnl_info *tnl;
> > + struct ethhdr *new_eth;
> > + struct ethhdr *old_eth;
> > + __u32 transport_hdr_sz;
> > + struct iphdr *iph;
> > + __u16 *next_iph;
> > + __u16 payload_len;
> > + struct vip vip = {};
> > + int dport;
> > + __u32 csum = 0;
> > + int i;
> > +
> > + __builtin_memset(eth_buffer, 0, sizeof(eth_buffer));
> > + __builtin_memset(iph_buffer_tcp, 0, sizeof(iph_buffer_tcp));
> > + __builtin_memset(iph_buffer_udp, 0, sizeof(iph_buffer_udp));
> > +
> > + if (ethhdr_sz + iphdr_sz + tcphdr_sz > xdp->data_end - xdp->data)
> > + iph = bpf_dynptr_slice(xdp_ptr, ethhdr_sz, iph_buffer_udp, sizeof(iph_buffer_udp));
> > + else
> > + iph = bpf_dynptr_slice(xdp_ptr, ethhdr_sz, iph_buffer_tcp, sizeof(iph_buffer_tcp));
> > +
> > + if (!iph)
> > + return XDP_DROP;
> > +
> > + dport = get_dport(iph + 1, iph->protocol);
> > + if (dport == -1)
> > + return XDP_DROP;
> > +
> > + vip.protocol = iph->protocol;
> > + vip.family = AF_INET;
> > + vip.daddr.v4 = iph->daddr;
> > + vip.dport = dport;
> > + payload_len = bpf_ntohs(iph->tot_len);
> > +
> > + tnl = bpf_map_lookup_elem(&vip2tnl, &vip);
> > + /* It only does v4-in-v4 */
> > + if (!tnl || tnl->family != AF_INET)
> > + return XDP_PASS;
> > +
> > + if (bpf_xdp_adjust_head(xdp, 0 - (int)iphdr_sz))
> > + return XDP_DROP;
> > +
> > + bpf_dynptr_from_xdp(xdp, 0, &new_xdp_ptr);
> > + new_eth = bpf_dynptr_slice_rdwr(&new_xdp_ptr, 0, eth_buffer, sizeof(eth_buffer));
> > + if (!new_eth)
> > + return XDP_DROP;
>
> Here the program just gets a new pointer using bpf_dynptr_slice_rdwr()
> and proceeds to write to it directly without any further checks. But
> what happens if the pointer points to the eth_buffer? Presumably the
> program would need to check this and do a bpf_dynptr_write() at the end
> in this case? Would be good to have this in the example, as this is
> basically the only documentation that exists, and people are probably
> going to copy-paste code from here :)
>
> -Toke
>
Ooh good catch, thanks! i forgot to change this for the other examples
as well; I will update this :)
>
>
> > + iph = (struct iphdr *)(new_eth + 1);
> > + old_eth = (struct ethhdr *)(iph + 1);
[...]
Powered by blists - more mailing lists