From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH v5 bpf-next 2/2] bpf: add tests for direct packet access
 from CGROUP_SKB
Date: Thu, 18 Oct 2018 22:22:48 -0700
Message-ID: <0b32fa1d-a607-34a8-bea0-cd2bf4ef7512@gmail.com>
References: <20181019050107.3684711-1-songliubraving@fb.com>
 <20181019050107.3684711-3-songliubraving@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: ast@kernel.org, daniel@iogearbox.net, kernel-team@fb.com
To: Song Liu <songliubraving@fb.com>, netdev@vger.kernel.org
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pl1-f196.google.com ([209.85.214.196]:38246 "EHLO
        mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726924AbeJSN1S (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 19 Oct 2018 09:27:18 -0400
Received: by mail-pl1-f196.google.com with SMTP id q19-v6so12282397pll.5
        for <netdev@vger.kernel.org>; Thu, 18 Oct 2018 22:22:50 -0700 (PDT)
In-Reply-To: <20181019050107.3684711-3-songliubraving@fb.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>



On 10/18/2018 10:01 PM, Song Liu wrote:
> Tests are added to make sure CGROUP_SKB cannot access:
>   tc_classid, data_meta, flow_keys
> 
> and can read and write:
>   mark, prority, and cb[0-4]
> 
> and can read other fields.
> 
> To make selftest with skb->sk work, a dummy sk is added in
> bpf_prog_test_run_skb().
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  net/bpf/test_run.c                          |   7 +
>  tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
>  2 files changed, 178 insertions(+)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 0c423b8cd75c..8dccac305268 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -10,6 +10,8 @@
>  #include <linux/etherdevice.h>
>  #include <linux/filter.h>
>  #include <linux/sched/signal.h>
> +#include <net/sock.h>
> +#include <net/tcp.h>
>  
>  static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
>  		struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
> @@ -115,6 +117,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>  	u32 retval, duration;
>  	int hh_len = ETH_HLEN;
>  	struct sk_buff *skb;
> +	struct sock sk = {0};

Arg another dummy :/

>  	void *data;
>  	int ret;
>  
> @@ -137,11 +140,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>  		break;
>  	}
>  
> +	sock_net_set(&sk, &init_net);

A few lines later we use :

skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);

So it looks like you want instead for consistency :

sock_net_set(&sk, current->nsproxy->net_ns);

	

> +	sock_init_data(NULL, &sk);
> +
>  	skb = build_skb(data, 0);
>  	if (!skb) {
>  		kfree(data);
>  		return -ENOMEM;
>  	}
> +	skb->sk = &sk;
>  

Normally this would need a skb->destructor, but I guess nothing will call skb_orphan()
from this point.