Re: [PATCH v3 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB

2018-10-18 Thread Song Liu



> On Oct 17, 2018, at 11:25 PM, Alexei Starovoitov 
>  wrote:
> 
> On Wed, Oct 17, 2018 at 10:39:49PM -0700, 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 
>> ---
>> net/bpf/test_run.c  |   4 +
>> tools/testing/selftests/bpf/test_verifier.c | 170 
>> 2 files changed, 174 insertions(+)
>> 
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index 0c423b8cd75c..c7210e2f1ae9 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -10,6 +10,7 @@
>> #include 
>> #include 
>> #include 
>> +#include 
>> 
>> 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 +116,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;
>>  void *data;
>>  int ret;
>> 
>> @@ -142,6 +144,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const 
>> union bpf_attr *kattr,
>>  kfree(data);
>>  return -ENOMEM;
>>  }
>> +sock_init_data(NULL, );
>> +skb->sk = 
> 
> I was about to apply it, but it crashes as:
> [   16.830822] BUG: unable to handle kernel paging request at 00014427b974
> [   16.831363] PGD 800135ecf067 P4D 800135ecf067 PUD 0
> [   16.831792] Oops:  [#1] SMP PTI
> [   16.832061] CPU: 1 PID: 1965 Comm: test_verifier Not tainted 
> 4.19.0-rc7-02550-ga76dee97ff12 #1153
> [   16.832712] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.11.0-2.el7 04/01/2014
> [   16.833358] RIP: 0010:cmp_map_id+0x10/0x50
> [   16.835036] RSP: 0018:c980faf8 EFLAGS: 00010246
> [   16.835429] RAX:  RBX: 36069ee8 RCX: 
> 
> [   16.835958] RDX: 00014427b970 RSI: 00014427b970 RDI: 
> c980fb44
> [   16.836496] RBP: 000c R08: 810f7330 R09: 
> 36069ee8
> [   16.837026] R10:  R11: 0001 R12: 
> 
> [   16.837554] R13: 810f7330 R14: 00014427b970 R15: 
> 1b034f74
> [   16.838083] FS:  7fae50663700() GS:88013ba8() 
> knlGS:
> [   16.838677] CS:  0010 DS:  ES:  CR0: 80050033
> [   16.839105] CR2: 00014427b974 CR3: 000135934005 CR4: 
> 003606e0
> [   16.839632] DR0:  DR1:  DR2: 
> 
> [   16.840157] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [   16.840682] Call Trace:
> [   16.840897]  bsearch+0x50/0x90
> [   16.841144]  map_id_range_down+0x81/0xa0
> [   16.841438]  make_kuid+0xf/0x10
> [   16.841677]  sock_init_data+0x24f/0x260
> [   16.841979]  bpf_prog_test_run_skb+0x9e/0x270
> 
> I suspect sock_net_set(sk, _net) is necessary before sock_init_data() 
> call.

I am not able to repro this, even with CONFIG_KASAN and CONFIG_PAGE_POISONING. 

Let me try a better approach on this.

Thanks,
Song



Re: [PATCH v3 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB

2018-10-18 Thread Alexei Starovoitov
On Wed, Oct 17, 2018 at 10:39:49PM -0700, 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 
> ---
>  net/bpf/test_run.c  |   4 +
>  tools/testing/selftests/bpf/test_verifier.c | 170 
>  2 files changed, 174 insertions(+)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 0c423b8cd75c..c7210e2f1ae9 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  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 +116,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;
>   void *data;
>   int ret;
>  
> @@ -142,6 +144,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const 
> union bpf_attr *kattr,
>   kfree(data);
>   return -ENOMEM;
>   }
> + sock_init_data(NULL, );
> + skb->sk = 

I was about to apply it, but it crashes as:
[   16.830822] BUG: unable to handle kernel paging request at 00014427b974
[   16.831363] PGD 800135ecf067 P4D 800135ecf067 PUD 0
[   16.831792] Oops:  [#1] SMP PTI
[   16.832061] CPU: 1 PID: 1965 Comm: test_verifier Not tainted 
4.19.0-rc7-02550-ga76dee97ff12 #1153
[   16.832712] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.11.0-2.el7 04/01/2014
[   16.833358] RIP: 0010:cmp_map_id+0x10/0x50
[   16.835036] RSP: 0018:c980faf8 EFLAGS: 00010246
[   16.835429] RAX:  RBX: 36069ee8 RCX: 
[   16.835958] RDX: 00014427b970 RSI: 00014427b970 RDI: c980fb44
[   16.836496] RBP: 000c R08: 810f7330 R09: 36069ee8
[   16.837026] R10:  R11: 0001 R12: 
[   16.837554] R13: 810f7330 R14: 00014427b970 R15: 1b034f74
[   16.838083] FS:  7fae50663700() GS:88013ba8() 
knlGS:
[   16.838677] CS:  0010 DS:  ES:  CR0: 80050033
[   16.839105] CR2: 00014427b974 CR3: 000135934005 CR4: 003606e0
[   16.839632] DR0:  DR1:  DR2: 
[   16.840157] DR3:  DR6: fffe0ff0 DR7: 0400
[   16.840682] Call Trace:
[   16.840897]  bsearch+0x50/0x90
[   16.841144]  map_id_range_down+0x81/0xa0
[   16.841438]  make_kuid+0xf/0x10
[   16.841677]  sock_init_data+0x24f/0x260
[   16.841979]  bpf_prog_test_run_skb+0x9e/0x270

I suspect sock_net_set(sk, _net) is necessary before sock_init_data() call.



[PATCH v3 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB

2018-10-17 Thread Song Liu
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 
---
 net/bpf/test_run.c  |   4 +
 tools/testing/selftests/bpf/test_verifier.c | 170 
 2 files changed, 174 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..c7210e2f1ae9 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 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 +116,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;
void *data;
int ret;
 
@@ -142,6 +144,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const 
union bpf_attr *kattr,
kfree(data);
return -ENOMEM;
}
+   sock_init_data(NULL, );
+   skb->sk = 
 
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
__skb_put(skb, size);
diff --git a/tools/testing/selftests/bpf/test_verifier.c 
b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..5bfba7e8afd7 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,176 @@ static struct bpf_test tests[] = {
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+   {
+   "direct packet read test#1 for CGROUP_SKB",
+   .insns = {
+   BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+   offsetof(struct __sk_buff, data)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+   offsetof(struct __sk_buff, data_end)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+   offsetof(struct __sk_buff, len)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+   offsetof(struct __sk_buff, pkt_type)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+   offsetof(struct __sk_buff, mark)),
+   BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+   offsetof(struct __sk_buff, mark)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+   offsetof(struct __sk_buff, queue_mapping)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+   offsetof(struct __sk_buff, protocol)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+   offsetof(struct __sk_buff, vlan_present)),
+   BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+   BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+   BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+   BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+   BPF_MOV64_IMM(BPF_REG_0, 0),
+   BPF_EXIT_INSN(),
+   },
+   .result = ACCEPT,
+   .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+   },
+   {
+   "direct packet read test#2 for CGROUP_SKB",
+   .insns = {
+   BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+   offsetof(struct __sk_buff, vlan_tci)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+   offsetof(struct __sk_buff, vlan_proto)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+   offsetof(struct __sk_buff, priority)),
+   BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+   offsetof(struct __sk_buff, priority)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+   offsetof(struct __sk_buff, 
ingress_ifindex)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+   offsetof(struct __sk_buff, tc_index)),
+   BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+   offsetof(struct __sk_buff, hash)),
+   BPF_MOV64_IMM(BPF_REG_0, 0),
+   BPF_EXIT_INSN(),
+   },
+   .result = ACCEPT,
+   .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+   },
+   {
+   "direct packet read test#3 for CGROUP_SKB",
+   .insns