diff mbox series

[net-next,v2,3/5] bpf: Split lwt inout verifier structures

Message ID 1a00b0c169c46a6444dea2b9f76e9c83430faa87.1524591163.git.m.xhonneux@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series ipv6: sr: introduce seg6local End.BPF action | expand

Commit Message

Mathieu Xhonneux April 24, 2018, 5:44 p.m. UTC 
The new bpf_lwt_push_encap helper should only be accessible within the
LWT BPF IN hook, and not the OUT one, as this may lead to a skb under
panic.

At the moment, both LWT BPF IN and OUT share the same list of helpers,
whose calls are authorized by the verifier. This patch separates the
verifier ops for the IN and OUT hooks, and allows the IN hook to call the
bpf_lwt_push_encap helper.

This patch is also the occasion to put all lwt_*_func_proto functions
together for clarity. At the moment, socks_op_func_proto is in the middle
of lwt_inout_func_proto and lwt_xmit_func_proto.

Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
Acked-by: David Lebrun <dlebrun@google.com>
---
 include/linux/bpf_types.h |  3 +-
 net/core/filter.c         | 83 +++++++++++++++++++++++++++++------------------
 2 files changed, 53 insertions(+), 33 deletions(-)

Comments

kernel test robot April 26, 2018, 3:29 p.m. UTC | #1 
Hi Mathieu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Mathieu-Xhonneux/ipv6-sr-introduce-seg6local-End-BPF-action/20180426-082209
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   net/core/filter.c:110:48: sparse: expression using sizeof(void)
   net/core/filter.c:110:48: sparse: expression using sizeof(void)
   net/core/filter.c:323:33: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:326:33: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:329:33: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:332:33: sparse: subtraction of functions? Share your drugs
   net/core/filter.c:335:33: sparse: subtraction of functions? Share your drugs
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   net/core/filter.c:1189:39: sparse: incorrect type in argument 1 (different address spaces) @@    expected struct sock_filter const *filter @@    got struct sockstruct sock_filter const *filter @@
   net/core/filter.c:1189:39:    expected struct sock_filter const *filter
   net/core/filter.c:1189:39:    got struct sock_filter [noderef] <asn:1>*filter
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   net/core/filter.c:1291:39: sparse: incorrect type in argument 1 (different address spaces) @@    expected struct sock_filter const *filter @@    got struct sockstruct sock_filter const *filter @@
   net/core/filter.c:1291:39:    expected struct sock_filter const *filter
   net/core/filter.c:1291:39:    got struct sock_filter [noderef] <asn:1>*filter
   include/linux/filter.h:612:16: sparse: expression using sizeof(void)
   net/core/filter.c:1552:43: sparse: incorrect type in argument 2 (different base types) @@    expected restricted __wsum [usertype] diff @@    got unsigned lonrestricted __wsum [usertype] diff @@
   net/core/filter.c:1552:43:    expected restricted __wsum [usertype] diff
   net/core/filter.c:1552:43:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1555:36: sparse: incorrect type in argument 2 (different base types) @@    expected restricted __be16 [usertype] old @@    got unsigned lonrestricted __be16 [usertype] old @@
   net/core/filter.c:1555:36:    expected restricted __be16 [usertype] old
   net/core/filter.c:1555:36:    got unsigned long long [unsigned] [usertype] from
   net/core/filter.c:1555:42: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be16 [usertype] new @@    got unsigned lonrestricted __be16 [usertype] new @@
   net/core/filter.c:1555:42:    expected restricted __be16 [usertype] new
   net/core/filter.c:1555:42:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1558:36: sparse: incorrect type in argument 2 (different base types) @@    expected restricted __be32 [usertype] from @@    got unsigned lonrestricted __be32 [usertype] from @@
   net/core/filter.c:1558:36:    expected restricted __be32 [usertype] from
   net/core/filter.c:1558:36:    got unsigned long long [unsigned] [usertype] from
   net/core/filter.c:1558:42: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be32 [usertype] to @@    got unsigned lonrestricted __be32 [usertype] to @@
   net/core/filter.c:1558:42:    expected restricted __be32 [usertype] to
   net/core/filter.c:1558:42:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1603:59: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __wsum [usertype] diff @@    got unsigned lonrestricted __wsum [usertype] diff @@
   net/core/filter.c:1603:59:    expected restricted __wsum [usertype] diff
   net/core/filter.c:1603:59:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1606:52: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be16 [usertype] from @@    got unsigned lonrestricted __be16 [usertype] from @@
   net/core/filter.c:1606:52:    expected restricted __be16 [usertype] from
   net/core/filter.c:1606:52:    got unsigned long long [unsigned] [usertype] from
   net/core/filter.c:1606:58: sparse: incorrect type in argument 4 (different base types) @@    expected restricted __be16 [usertype] to @@    got unsigned lonrestricted __be16 [usertype] to @@
   net/core/filter.c:1606:58:    expected restricted __be16 [usertype] to
   net/core/filter.c:1606:58:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1609:52: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be32 [usertype] from @@    got unsigned lonrestricted __be32 [usertype] from @@
   net/core/filter.c:1609:52:    expected restricted __be32 [usertype] from
   net/core/filter.c:1609:52:    got unsigned long long [unsigned] [usertype] from
   net/core/filter.c:1609:58: sparse: incorrect type in argument 4 (different base types) @@    expected restricted __be32 [usertype] to @@    got unsigned lonrestricted __be32 [usertype] to @@
   net/core/filter.c:1609:58:    expected restricted __be32 [usertype] to
   net/core/filter.c:1609:58:    got unsigned long long [unsigned] [usertype] to
   net/core/filter.c:1655:28: sparse: incorrect type in return expression (different base types) @@    expected unsigned long long @@    got nsigned long long @@
   net/core/filter.c:1655:28:    expected unsigned long long
   net/core/filter.c:1655:28:    got restricted __wsum
   net/core/filter.c:1677:35: sparse: incorrect type in return expression (different base types) @@    expected unsigned long long @@    got restricted unsigned long long @@
   net/core/filter.c:1677:35:    expected unsigned long long
   net/core/filter.c:1677:35:    got restricted __wsum [usertype] csum
   net/core/filter.c:3462:41: sparse: expression using sizeof(void)
   net/core/filter.c:3466:41: sparse: expression using sizeof(void)
   net/core/filter.c:3470:46: sparse: expression using sizeof(void)
   net/core/filter.c:3470:46: sparse: expression using sizeof(void)
   net/core/filter.c:3538:47: sparse: expression using sizeof(void)
   net/core/filter.c:3728:5: sparse: symbol 'bpf_push_seg6_encap' was not declared. Should it be static?
>> net/core/filter.c:5806:31: sparse: symbol 'lwt_out_verifier_ops' was not declared. Should it be static?
>> net/core/filter.c:5812:27: sparse: symbol 'lwt_out_prog_ops' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 2b28fcf6f6ae..799c8bb6416f 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -9,8 +9,7 @@  BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
-BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_inout)
-BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_inout)
+BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
 BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
 BPF_PROG_TYPE(BPF_PROG_TYPE_SOCK_OPS, sock_ops)
 BPF_PROG_TYPE(BPF_PROG_TYPE_SK_SKB, sk_skb)
diff --git a/net/core/filter.c b/net/core/filter.c
index 8e67c423db35..8a2331c47881 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4136,33 +4136,6 @@  xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	}
 }
 
-static const struct bpf_func_proto *
-lwt_inout_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
-{
-	switch (func_id) {
-	case BPF_FUNC_skb_load_bytes:
-		return &bpf_skb_load_bytes_proto;
-	case BPF_FUNC_skb_pull_data:
-		return &bpf_skb_pull_data_proto;
-	case BPF_FUNC_csum_diff:
-		return &bpf_csum_diff_proto;
-	case BPF_FUNC_get_cgroup_classid:
-		return &bpf_get_cgroup_classid_proto;
-	case BPF_FUNC_get_route_realm:
-		return &bpf_get_route_realm_proto;
-	case BPF_FUNC_get_hash_recalc:
-		return &bpf_get_hash_recalc_proto;
-	case BPF_FUNC_perf_event_output:
-		return &bpf_skb_event_output_proto;
-	case BPF_FUNC_get_smp_processor_id:
-		return &bpf_get_smp_processor_id_proto;
-	case BPF_FUNC_skb_under_cgroup:
-		return &bpf_skb_under_cgroup_proto;
-	default:
-		return bpf_base_func_proto(func_id);
-	}
-}
-
 static const struct bpf_func_proto *
 sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -4222,6 +4195,44 @@  sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	}
 }
 
+static const struct bpf_func_proto *
+lwt_out_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	switch (func_id) {
+	case BPF_FUNC_skb_load_bytes:
+		return &bpf_skb_load_bytes_proto;
+	case BPF_FUNC_skb_pull_data:
+		return &bpf_skb_pull_data_proto;
+	case BPF_FUNC_csum_diff:
+		return &bpf_csum_diff_proto;
+	case BPF_FUNC_get_cgroup_classid:
+		return &bpf_get_cgroup_classid_proto;
+	case BPF_FUNC_get_route_realm:
+		return &bpf_get_route_realm_proto;
+	case BPF_FUNC_get_hash_recalc:
+		return &bpf_get_hash_recalc_proto;
+	case BPF_FUNC_perf_event_output:
+		return &bpf_skb_event_output_proto;
+	case BPF_FUNC_get_smp_processor_id:
+		return &bpf_get_smp_processor_id_proto;
+	case BPF_FUNC_skb_under_cgroup:
+		return &bpf_skb_under_cgroup_proto;
+	default:
+		return bpf_base_func_proto(func_id);
+	}
+}
+
+static const struct bpf_func_proto *
+lwt_in_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	switch (func_id) {
+	case BPF_FUNC_lwt_push_encap:
+		return &bpf_lwt_push_encap_proto;
+	default:
+		return lwt_out_func_proto(func_id, prog);
+	}
+}
+
 static const struct bpf_func_proto *
 lwt_xmit_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -4253,7 +4264,7 @@  lwt_xmit_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_set_hash_invalid:
 		return &bpf_set_hash_invalid_proto;
 	default:
-		return lwt_inout_func_proto(func_id, prog);
+		return lwt_out_func_proto(func_id, prog);
 	}
 }
 
@@ -5782,13 +5793,23 @@  const struct bpf_prog_ops cg_skb_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
-const struct bpf_verifier_ops lwt_inout_verifier_ops = {
-	.get_func_proto		= lwt_inout_func_proto,
+const struct bpf_verifier_ops lwt_in_verifier_ops = {
+	.get_func_proto		= lwt_in_func_proto,
+	.is_valid_access	= lwt_is_valid_access,
+	.convert_ctx_access	= bpf_convert_ctx_access,
+};
+
+const struct bpf_prog_ops lwt_in_prog_ops = {
+	.test_run		= bpf_prog_test_run_skb,
+};
+
+const struct bpf_verifier_ops lwt_out_verifier_ops = {
+	.get_func_proto		= lwt_out_func_proto,
 	.is_valid_access	= lwt_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-const struct bpf_prog_ops lwt_inout_prog_ops = {
+const struct bpf_prog_ops lwt_out_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };