netdev - Re: [bpf-next PATCH] samples/bpf: xdp2skb_meta shows transferring info from XDP to SKB

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: <2394116c-8417-ddbc-797f-3a80e3d650b6@iogearbox.net>
Date:   Thu, 11 Jan 2018 01:15:48 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>, netdev@...r.kernel.org,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
Subject: Re: [bpf-next PATCH] samples/bpf: xdp2skb_meta shows transferring
 info from XDP to SKB

On 01/10/2018 06:21 PM, Jesper Dangaard Brouer wrote:
> Creating a bpf sample that shows howto use the XDP 'data_meta'
> infrastructure, created by Daniel Borkmann.  Very few drivers support
> this feature, but I wanted a functional sample to begin with, when
> working on adding driver support.
> 
> XDP data_meta is about creating a communication channel between BPF
> programs.  This can be XDP tail-progs, but also other SKB based BPF
> hooks, like in this case the TC clsact hook. In this sample I show
> that XDP can store info named "mark", and TC/clsact chooses to use
> this info and store it into the skb->mark.
> 
> It is a bit annoying that XDP and TC samples uses different tools/libs
> when attaching their BPF hooks.  As the XDP and TC programs need to
> cooperate and agree on a struct-layout, it is best/easiest if the two
> programs can be contained within the same BPF restricted-C file.
> 
> As the bpf-loader, I choose to not use bpf_load.c (or libbpf), but
> instead wrote a bash shell scripted named xdp2skb_meta.sh, which
> demonstrate howto use the iproute cmdline tools 'tc' and 'ip' for
> loading BPF programs.  To make it easy for first time users, the shell
> script have command line parsing, and support --verbose and --dry-run
> mode, if you just want to see/learn the tc+ip command syntax:
> 
>  # ./xdp2skb_meta.sh --dev ixgbe2 --dry-run
>  # Dry-run mode: enable VERBOSE and don't call TC+IP
>  tc qdisc del dev ixgbe2 clsact
>  tc qdisc add dev ixgbe2 clsact
>  tc filter add dev ixgbe2 ingress prio 1 handle 1 bpf da obj ./xdp2skb_meta_kern.o sec tc_mark
>  # Flush XDP on device: ixgbe2
>  ip link set dev ixgbe2 xdp off
>  ip link set dev ixgbe2 xdp obj ./xdp2skb_meta_kern.o sec xdp_mark
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>

Applied to bpf-next, thanks Jesper!

[...]
> +	/* Reserve space in-front data pointer for our meta info.
> +	 * (Notice drivers not supporting data_meta will fail here!)
> +	 */
> +	ret = bpf_xdp_adjust_meta(ctx, -(int)sizeof(*meta));
> +	if (ret < 0)
> +		return XDP_ABORTED;
> +
> +	/* For some unknown reason, these ctx pointers must be read
> +	 * after bpf_xdp_adjust_meta, else verifier will reject prog.
> +	 */

Reason is not unknown: bpf_xdp_adjust_meta() needs to invalidate pkt
pointers same way like other helpers that change them such as the
bpf_xdp_adjust_head() for example. Hence, they're all part of the
bpf_helper_changes_pkt_data() that forces reload.

> +	data = (void *)(unsigned long)ctx->data;
> +
> +	/* Check data_meta have room for meta_info struct */
> +	meta = (void *)(unsigned long)ctx->data_meta;
> +	if (meta + 1 > data)
> +		return XDP_ABORTED;
> +
> +	meta->mark = 42