Message ID | 20220414104458.3097244-3-razor@blackwall.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | wireguard: device: fix metadata_dst xmit null pointer dereference | expand |
Hi Nikolay, These tests need to run in the minimal fast-to-compile test harness inside of tools/testing/selftests/wireguard/qemu, which you can try out with: $ make -C tools/testing/selftests/wireguard/qemu -j$(nproc) Currently iproute2 is built, but only ip(8) is used in the image, so you'll need to add tc(8) to there. Clang, however, seems a bit heavyweight. I suspect it'd make more sense to just base64 up that object file and include it as a string in the file? Or, alternatively, we could just not move ahead with this rather niche test, and revisit the issue if we wind up wanting to test for lots of bpf things. Thoughts on that? Jason
On 14/04/2022 15:06, Jason A. Donenfeld wrote: > Hi Nikolay, > > These tests need to run in the minimal fast-to-compile test harness > inside of tools/testing/selftests/wireguard/qemu, which you can try > out with: > > $ make -C tools/testing/selftests/wireguard/qemu -j$(nproc) > > Currently iproute2 is built, but only ip(8) is used in the image, so > you'll need to add tc(8) to there. Clang, however, seems a bit > heavyweight. I suspect it'd make more sense to just base64 up that > object file and include it as a string in the file? Or, alternatively, > we could just not move ahead with this rather niche test, and revisit > the issue if we wind up wanting to test for lots of bpf things. > Thoughts on that? > > Jason Hi Jason, My bad, I completely missed the qemu part. I'll look into including the ready object file. If it works out, looks compact and well I'll post v2. Thanks, Nik
On Thu, Apr 14, 2022 at 2:12 PM Nikolay Aleksandrov <razor@blackwall.org> wrote:
> My bad, I completely missed the qemu part.
If you're curious, by the way, there's this running all those tests
for every commit https://www.wireguard.com/build-status/
Jason
diff --git a/tools/testing/selftests/wireguard/netns.sh b/tools/testing/selftests/wireguard/netns.sh index 8a9461aa0878..b492dbb94245 100755 --- a/tools/testing/selftests/wireguard/netns.sh +++ b/tools/testing/selftests/wireguard/netns.sh @@ -156,6 +156,67 @@ tests() { done } +md_dst_test_cleanup() { + rm -rf /tmp/test_wg_tun.c /tmp/test_wg_tun.ll /tmp/test_wg_tun.o + n1 tc qdisc del dev wg0 clsact +} + +# test for md dst on wireguard's egress path +md_dst_test() { + # clang is required for the test + if [[ ! -x "$(command -v "clang")" ]]; then + return + fi + + # attach md dst to the skb on egress using bpf_skb_set_tunnel_key + n1 cat > /tmp/test_wg_tun.c <<EOF +#include <linux/bpf.h> + +#ifndef TC_ACT_OK +# define TC_ACT_OK 0 +#endif + +static long (*bpf_skb_set_tunnel_key)(struct __sk_buff *skb, struct bpf_tunnel_key *key, __u32 size, __u64 flags) = (void *) 21; + +__attribute__((section("egress"), used)) +int tc_egress(struct __sk_buff *skb) +{ + struct bpf_tunnel_key key = {}; + + bpf_skb_set_tunnel_key(skb, &key, sizeof(key), 0); + + return TC_ACT_OK; +} + +char __license[] __attribute__((section("license"), used)) = "GPL"; +EOF + + n1 clang -O2 -emit-llvm -c /tmp/test_wg_tun.c -o /tmp/test_wg_tun.ll + if [[ ! -f "/tmp/test_wg_tun.ll" ]]; then + md_dst_test_cleanup + return + fi + n1 llc -march=bpf -filetype=obj -o /tmp/test_wg_tun.o /tmp/test_wg_tun.ll + if [[ ! -f "/tmp/test_wg_tun.o" ]]; then + md_dst_test_cleanup + return + fi + + n1 tc qdisc add dev wg0 clsact + if [[ $? -ne 0 ]]; then + md_dst_test_cleanup + return + fi + n1 tc filter add dev wg0 egress basic action bpf obj /tmp/test_wg_tun.o sec egress + if [[ $? -ne 0 ]]; then + md_dst_test_cleanup + return + fi + n1 ping -c 2 -f -W 1 192.168.241.2 + # if we reach here without a crash the test passed + md_dst_test_cleanup +} + [[ $(ip1 link show dev wg0) =~ mtu\ ([0-9]+) ]] && orig_mtu="${BASH_REMATCH[1]}" big_mtu=$(( 34816 - 1500 + $orig_mtu )) @@ -175,6 +236,8 @@ read _ rx_bytes tx_bytes < <(n1 wg show wg0 transfer) read _ timestamp < <(n1 wg show wg0 latest-handshakes) (( timestamp != 0 )) +md_dst_test + tests ip1 link set wg0 mtu $big_mtu ip2 link set wg0 mtu $big_mtu
Add a selftest for transmitting skb with md_dst attached. It is done via a bpf program which uses bpf_skb_set_tunnel_key on wireguard's egress path. It requires clang and tc to be installed. If the test finishes without a crash it is considered successful. CC: wireguard@lists.zx2c4.com CC: Jason A. Donenfeld <Jason@zx2c4.com> CC: Daniel Borkmann <daniel@iogearbox.net> CC: Martynas Pumputis <m@lambda.lt> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> --- Executed the prep compilation commands with n1 to make them visible. tools/testing/selftests/wireguard/netns.sh | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+)