[PATCH bpf-next 00/11] Add socket lookup support

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH bpf-next 00/11] Add socket lookup support
@ 2018-09-12  0:36 Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 01/11] bpf: Add iterator for spilled registers Joe Stringer
                   ` (10 more replies)
  0 siblings, 11 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

This series proposes a new helper for the BPF API which allows BPF programs to
perform lookups for sockets in a network namespace. This would allow programs
to determine early on in processing whether the stack is expecting to receive
the packet, and perform some action (eg drop, forward somewhere) based on this
information.

The series is structured roughly into:
* Misc refactor
* Add the socket pointer type
* Add reference tracking to ensure that socket references are freed
* Extend the BPF API to add sk_lookup_xxx() / sk_release() functions
* Add tests/documentation

The helper proposed in this series includes a parameter for a tuple which must
be filled in by the caller to determine the socket to look up. The simplest
case would be filling with the contents of the packet, ie mapping the packet's
5-tuple into the parameter. In common cases, it may alternatively be useful to
reverse the direction of the tuple and perform a lookup, to find the socket
that initiates this connection; and if the BPF program ever performs a form of
IP address translation, it may further be useful to be able to look up
arbitrary tuples that are not based upon the packet, but instead based on state
held in BPF maps or hardcoded in the BPF program.

Currently, access into the socket's fields are limited to those which are
otherwise already accessible, and are restricted to read-only access.

Changes since RFC:
* Split up sk_lookup() into sk_lookup_tcp(), sk_lookup_udp().
* Only take references on the socket when necessary.
  * Make sk_release() only free the socket reference in this case.
* Fix some runtime reference leaks:
  * Disallow BPF_LD_[ABS|IND] instructions while holding a reference.
  * Disallow bpf_tail_call() while holding a reference.
* Prevent the same instruction being used for reference and other
  pointer type.
* Simplify locating copies of a reference during helper calls by caching
  the pointer id from the caller.
* Fix kbuild compilation warnings with particular configs.
* Improve code comments describing the new verifier pieces.
* Testing courtesy of Nitin
* Rebase

This tree is also available at:
https://github.com/joestringer/linux/commits/submit/sk-lookup-v1

Joe Stringer (11):
  bpf: Add iterator for spilled registers
  bpf: Simplify ptr_min_max_vals adjustment
  bpf: Generalize ptr_or_null regs check
  bpf: Add PTR_TO_SOCKET verifier type
  bpf: Macrofy stack state copy
  bpf: Add reference tracking to verifier
  bpf: Add helper to retrieve socket in BPF
  selftests/bpf: Add tests for reference tracking
  libbpf: Support loading individual progs
  selftests/bpf: Add C tests for reference tracking
  Documentation: Describe bpf reference tracking

 Documentation/networking/filter.txt           |  64 ++
 include/linux/bpf.h                           |  17 +
 include/linux/bpf_verifier.h                  |  37 +-
 include/uapi/linux/bpf.h                      |  54 +-
 kernel/bpf/verifier.c                         | 599 ++++++++++++++----
 net/core/filter.c                             | 175 ++++-
 tools/include/uapi/linux/bpf.h                |  54 +-
 tools/lib/bpf/libbpf.c                        |   4 +-
 tools/lib/bpf/libbpf.h                        |   3 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |  12 +
 tools/testing/selftests/bpf/test_progs.c      |  38 ++
 .../selftests/bpf/test_sk_lookup_kern.c       | 128 ++++
 tools/testing/selftests/bpf/test_verifier.c   | 373 ++++++++++-
 14 files changed, 1426 insertions(+), 134 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_sk_lookup_kern.c

-- 
2.17.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 01/11] bpf: Add iterator for spilled registers
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 02/11] bpf: Simplify ptr_min_max_vals adjustment Joe Stringer
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Add this iterator for spilled registers, it concentrates the details of
how to get the current frame's spilled registers into a single macro
while clarifying the intention of the code which is calling the macro.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/bpf_verifier.h | 11 +++++++++++
 kernel/bpf/verifier.c        | 16 +++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b42b60a83e19..af262b97f586 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -131,6 +131,17 @@ struct bpf_verifier_state {
 	u32 curframe;
 };
 
+#define __get_spilled_reg(slot, frame)					\
+	(((slot < frame->allocated_stack / BPF_REG_SIZE) &&		\
+	  (frame->stack[slot].slot_type[0] == STACK_SPILL))		\
+	 ? &frame->stack[slot].spilled_ptr : NULL)
+
+/* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
+#define for_each_spilled_reg(iter, frame, reg)				\
+	for (iter = 0, reg = __get_spilled_reg(iter, frame);		\
+	     iter < frame->allocated_stack / BPF_REG_SIZE;		\
+	     iter++, reg = __get_spilled_reg(iter, frame))
+
 /* linked list of verifier states used to prune search */
 struct bpf_verifier_state_list {
 	struct bpf_verifier_state state;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6ff1bac1795d..97aac6ac1b0d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2219,10 +2219,9 @@ static void __clear_all_pkt_pointers(struct bpf_verifier_env *env,
 		if (reg_is_pkt_pointer_any(&regs[i]))
 			mark_reg_unknown(env, regs, i);
 
-	for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
-		if (state->stack[i].slot_type[0] != STACK_SPILL)
+	for_each_spilled_reg(i, state, reg) {
+		if (!reg)
 			continue;
-		reg = &state->stack[i].spilled_ptr;
 		if (reg_is_pkt_pointer_any(reg))
 			__mark_reg_unknown(reg);
 	}
@@ -3362,10 +3361,9 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate,
 
 	for (j = 0; j <= vstate->curframe; j++) {
 		state = vstate->frame[j];
-		for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
-			if (state->stack[i].slot_type[0] != STACK_SPILL)
+		for_each_spilled_reg(i, state, reg) {
+			if (!reg)
 				continue;
-			reg = &state->stack[i].spilled_ptr;
 			if (reg->type == type && reg->id == dst_reg->id)
 				reg->range = max(reg->range, new_range);
 		}
@@ -3610,7 +3608,7 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
 			  bool is_null)
 {
 	struct bpf_func_state *state = vstate->frame[vstate->curframe];
-	struct bpf_reg_state *regs = state->regs;
+	struct bpf_reg_state *reg, *regs = state->regs;
 	u32 id = regs[regno].id;
 	int i, j;
 
@@ -3619,8 +3617,8 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
 
 	for (j = 0; j <= vstate->curframe; j++) {
 		state = vstate->frame[j];
-		for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
-			if (state->stack[i].slot_type[0] != STACK_SPILL)
+		for_each_spilled_reg(i, state, reg) {
+			if (!reg)
 				continue;
 			mark_map_reg(&state->stack[i].spilled_ptr, 0, id, is_null);
 		}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 02/11] bpf: Simplify ptr_min_max_vals adjustment
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 01/11] bpf: Add iterator for spilled registers Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 03/11] bpf: Generalize ptr_or_null regs check Joe Stringer
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

An upcoming commit will add another two pointer types that need very
similar behaviour, so generalise this function now.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/verifier.c                       | 22 ++++++++++-----------
 tools/testing/selftests/bpf/test_verifier.c | 14 ++++++-------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 97aac6ac1b0d..61b60e141b6a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2636,20 +2636,18 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	if (ptr_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
-		verbose(env, "R%d pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL prohibited, null-check it first\n",
-			dst);
-		return -EACCES;
-	}
-	if (ptr_reg->type == CONST_PTR_TO_MAP) {
-		verbose(env, "R%d pointer arithmetic on CONST_PTR_TO_MAP prohibited\n",
-			dst);
+	switch (ptr_reg->type) {
+	case PTR_TO_MAP_VALUE_OR_NULL:
+		verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n",
+			dst, reg_type_str[ptr_reg->type]);
 		return -EACCES;
-	}
-	if (ptr_reg->type == PTR_TO_PACKET_END) {
-		verbose(env, "R%d pointer arithmetic on PTR_TO_PACKET_END prohibited\n",
-			dst);
+	case CONST_PTR_TO_MAP:
+	case PTR_TO_PACKET_END:
+		verbose(env, "R%d pointer arithmetic on %s prohibited\n",
+			dst, reg_type_str[ptr_reg->type]);
 		return -EACCES;
+	default:
+		break;
 	}
 
 	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 67c412d19c09..ceb55a9f3da9 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -3637,7 +3637,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
-		.errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+		.errstr = "R3 pointer arithmetic on pkt_end",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 	},
@@ -4780,7 +4780,7 @@ static struct bpf_test tests[] = {
 			BPF_EXIT_INSN(),
 		},
 		.fixup_map1 = { 4 },
-		.errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+		.errstr = "R4 pointer arithmetic on map_value_or_null",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS
 	},
@@ -4801,7 +4801,7 @@ static struct bpf_test tests[] = {
 			BPF_EXIT_INSN(),
 		},
 		.fixup_map1 = { 4 },
-		.errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+		.errstr = "R4 pointer arithmetic on map_value_or_null",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS
 	},
@@ -4822,7 +4822,7 @@ static struct bpf_test tests[] = {
 			BPF_EXIT_INSN(),
 		},
 		.fixup_map1 = { 4 },
-		.errstr = "R4 pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL",
+		.errstr = "R4 pointer arithmetic on map_value_or_null",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS
 	},
@@ -7137,7 +7137,7 @@ static struct bpf_test tests[] = {
 			BPF_EXIT_INSN(),
 		},
 		.fixup_map_in_map = { 3 },
-		.errstr = "R1 pointer arithmetic on CONST_PTR_TO_MAP prohibited",
+		.errstr = "R1 pointer arithmetic on map_ptr prohibited",
 		.result = REJECT,
 	},
 	{
@@ -8811,7 +8811,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
-		.errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+		.errstr = "R3 pointer arithmetic on pkt_end",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_XDP,
 	},
@@ -8830,7 +8830,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
-		.errstr = "R3 pointer arithmetic on PTR_TO_PACKET_END",
+		.errstr = "R3 pointer arithmetic on pkt_end",
 		.result = REJECT,
 		.prog_type = BPF_PROG_TYPE_XDP,
 	},
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 03/11] bpf: Generalize ptr_or_null regs check
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 01/11] bpf: Add iterator for spilled registers Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 02/11] bpf: Simplify ptr_min_max_vals adjustment Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type Joe Stringer
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

This check will be reused by an upcoming commit for conditional jump
checks for sockets. Refactor it a bit to simplify the later commit.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/verifier.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 61b60e141b6a..f2357c8c90de 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -249,6 +249,11 @@ static bool type_is_pkt_pointer(enum bpf_reg_type type)
 	       type == PTR_TO_PACKET_META;
 }
 
+static bool reg_type_may_be_null(enum bpf_reg_type type)
+{
+	return type == PTR_TO_MAP_VALUE_OR_NULL;
+}
+
 /* string representation of 'enum bpf_reg_type' */
 static const char * const reg_type_str[] = {
 	[NOT_INIT]		= "?",
@@ -3567,12 +3572,10 @@ static void reg_combine_min_max(struct bpf_reg_state *true_src,
 	}
 }
 
-static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
-			 bool is_null)
+static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
+				 bool is_null)
 {
-	struct bpf_reg_state *reg = &regs[regno];
-
-	if (reg->type == PTR_TO_MAP_VALUE_OR_NULL && reg->id == id) {
+	if (reg_type_may_be_null(reg->type) && reg->id == id) {
 		/* Old offset (both fixed and variable parts) should
 		 * have been known-zero, because we don't allow pointer
 		 * arithmetic on pointers that might be NULL.
@@ -3585,11 +3588,13 @@ static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
 		}
 		if (is_null) {
 			reg->type = SCALAR_VALUE;
-		} else if (reg->map_ptr->inner_map_meta) {
-			reg->type = CONST_PTR_TO_MAP;
-			reg->map_ptr = reg->map_ptr->inner_map_meta;
-		} else {
-			reg->type = PTR_TO_MAP_VALUE;
+		} else if (reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
+			if (reg->map_ptr->inner_map_meta) {
+				reg->type = CONST_PTR_TO_MAP;
+				reg->map_ptr = reg->map_ptr->inner_map_meta;
+			} else {
+				reg->type = PTR_TO_MAP_VALUE;
+			}
 		}
 		/* We don't need id from this point onwards anymore, thus we
 		 * should better reset it, so that state pruning has chances
@@ -3602,8 +3607,8 @@ static void mark_map_reg(struct bpf_reg_state *regs, u32 regno, u32 id,
 /* The logic is similar to find_good_pkt_pointers(), both could eventually
  * be folded together at some point.
  */
-static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
-			  bool is_null)
+static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno,
+				  bool is_null)
 {
 	struct bpf_func_state *state = vstate->frame[vstate->curframe];
 	struct bpf_reg_state *reg, *regs = state->regs;
@@ -3611,14 +3616,14 @@ static void mark_map_regs(struct bpf_verifier_state *vstate, u32 regno,
 	int i, j;
 
 	for (i = 0; i < MAX_BPF_REG; i++)
-		mark_map_reg(regs, i, id, is_null);
+		mark_ptr_or_null_reg(&regs[i], id, is_null);
 
 	for (j = 0; j <= vstate->curframe; j++) {
 		state = vstate->frame[j];
 		for_each_spilled_reg(i, state, reg) {
 			if (!reg)
 				continue;
-			mark_map_reg(&state->stack[i].spilled_ptr, 0, id, is_null);
+			mark_ptr_or_null_reg(reg, id, is_null);
 		}
 	}
 }
@@ -3820,12 +3825,14 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 	/* detect if R == 0 where R is returned from bpf_map_lookup_elem() */
 	if (BPF_SRC(insn->code) == BPF_K &&
 	    insn->imm == 0 && (opcode == BPF_JEQ || opcode == BPF_JNE) &&
-	    dst_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
-		/* Mark all identical map registers in each branch as either
+	    reg_type_may_be_null(dst_reg->type)) {
+		/* Mark all identical registers in each branch as either
 		 * safe or unknown depending R == 0 or R != 0 conditional.
 		 */
-		mark_map_regs(this_branch, insn->dst_reg, opcode == BPF_JNE);
-		mark_map_regs(other_branch, insn->dst_reg, opcode == BPF_JEQ);
+		mark_ptr_or_null_regs(this_branch, insn->dst_reg,
+				      opcode == BPF_JNE);
+		mark_ptr_or_null_regs(other_branch, insn->dst_reg,
+				      opcode == BPF_JEQ);
 	} else if (!try_match_pkt_pointers(insn, dst_reg, &regs[insn->src_reg],
 					   this_branch, other_branch) &&
 		   is_pointer_value(env, insn->dst_reg)) {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (2 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 03/11] bpf: Generalize ptr_or_null regs check Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12 22:50   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 05/11] bpf: Macrofy stack state copy Joe Stringer
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Teach the verifier a little bit about a new type of pointer, a
PTR_TO_SOCKET. This pointer type is accessed from BPF through the
'struct bpf_sock' structure.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 include/linux/bpf.h          |  17 +++++
 include/linux/bpf_verifier.h |   2 +
 kernel/bpf/verifier.c        | 125 ++++++++++++++++++++++++++++++-----
 net/core/filter.c            |  30 +++++----
 4 files changed, 147 insertions(+), 27 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 523481a3471b..6ec93f3d66dd 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -154,6 +154,7 @@ enum bpf_arg_type {
 
 	ARG_PTR_TO_CTX,		/* pointer to context */
 	ARG_ANYTHING,		/* any (initialized) argument is ok */
+	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock */
 };
 
 /* type of values returned from helper functions */
@@ -162,6 +163,7 @@ enum bpf_return_type {
 	RET_VOID,			/* function doesn't return anything */
 	RET_PTR_TO_MAP_VALUE,		/* returns a pointer to map elem value */
 	RET_PTR_TO_MAP_VALUE_OR_NULL,	/* returns a pointer to map elem value or NULL */
+	RET_PTR_TO_SOCKET_OR_NULL,	/* returns a pointer to a socket or NULL */
 };
 
 /* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
@@ -212,6 +214,8 @@ enum bpf_reg_type {
 	PTR_TO_PACKET_META,	 /* skb->data - meta_len */
 	PTR_TO_PACKET,		 /* reg points to skb->data */
 	PTR_TO_PACKET_END,	 /* skb->data + headlen */
+	PTR_TO_SOCKET,		 /* reg points to struct bpf_sock */
+	PTR_TO_SOCKET_OR_NULL,	 /* reg points to struct bpf_sock or NULL */
 };
 
 /* The information passed from prog-specific *_is_valid_access
@@ -334,6 +338,11 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
 
 typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src,
 					unsigned long off, unsigned long len);
+typedef u32 (*bpf_convert_ctx_access_t)(enum bpf_access_type type,
+					const struct bpf_insn *src,
+					struct bpf_insn *dst,
+					struct bpf_prog *prog,
+					u32 *target_size);
 
 u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy);
@@ -827,4 +836,12 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
 void bpf_user_rnd_init_once(void);
 u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 
+bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
+			      struct bpf_insn_access_aux *info);
+u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
+			        const struct bpf_insn *si,
+			        struct bpf_insn *insn_buf,
+			        struct bpf_prog *prog,
+			        u32 *target_size);
+
 #endif /* _LINUX_BPF_H */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index af262b97f586..23a2b17bfd75 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -58,6 +58,8 @@ struct bpf_reg_state {
 	 * offset, so they can share range knowledge.
 	 * For PTR_TO_MAP_VALUE_OR_NULL this is used to share which map value we
 	 * came from, when one is tested for != NULL.
+	 * For PTR_TO_SOCKET this is used to share which pointers retain the
+	 * same reference to the socket, to determine proper reference freeing.
 	 */
 	u32 id;
 	/* For scalar types (SCALAR_VALUE), this represents our knowledge of
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f2357c8c90de..111e031cf65d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -80,8 +80,8 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
  * (like pointer plus pointer becomes SCALAR_VALUE type)
  *
  * When verifier sees load or store instructions the type of base register
- * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK. These are three pointer
- * types recognized by check_mem_access() function.
+ * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK, PTR_TO_SOCKET. These are
+ * four pointer types recognized by check_mem_access() function.
  *
  * PTR_TO_MAP_VALUE means that this register is pointing to 'map element value'
  * and the range of [ptr, ptr + map's value_size) is accessible.
@@ -266,6 +266,8 @@ static const char * const reg_type_str[] = {
 	[PTR_TO_PACKET]		= "pkt",
 	[PTR_TO_PACKET_META]	= "pkt_meta",
 	[PTR_TO_PACKET_END]	= "pkt_end",
+	[PTR_TO_SOCKET]		= "sock",
+	[PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
 };
 
 static char slot_type_char[] = {
@@ -971,6 +973,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
 	case PTR_TO_PACKET_META:
 	case PTR_TO_PACKET_END:
 	case CONST_PTR_TO_MAP:
+	case PTR_TO_SOCKET:
+	case PTR_TO_SOCKET_OR_NULL:
 		return true;
 	default:
 		return false;
@@ -1326,6 +1330,28 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
 	return -EACCES;
 }
 
+static int check_sock_access(struct bpf_verifier_env *env, u32 regno, int off,
+			     int size, enum bpf_access_type t)
+{
+	struct bpf_reg_state *regs = cur_regs(env);
+	struct bpf_reg_state *reg = &regs[regno];
+	struct bpf_insn_access_aux info;
+
+	if (reg->smin_value < 0) {
+		verbose(env, "R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
+			regno);
+		return -EACCES;
+	}
+
+	if (!bpf_sock_is_valid_access(off, size, t, &info)) {
+		verbose(env, "invalid bpf_sock_ops access off=%d size=%d\n",
+			off, size);
+		return -EACCES;
+	}
+
+	return 0;
+}
+
 static bool __is_pointer_value(bool allow_ptr_leaks,
 			       const struct bpf_reg_state *reg)
 {
@@ -1441,6 +1467,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
 		 */
 		strict = true;
 		break;
+	case PTR_TO_SOCKET:
+		pointer_desc = "sock ";
+		break;
 	default:
 		break;
 	}
@@ -1697,6 +1726,16 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 		err = check_packet_access(env, regno, off, size, false);
 		if (!err && t == BPF_READ && value_regno >= 0)
 			mark_reg_unknown(env, regs, value_regno);
+
+	} else if (reg->type == PTR_TO_SOCKET) {
+		if (t == BPF_WRITE) {
+			verbose(env, "cannot write into socket\n");
+			return -EACCES;
+		}
+		err = check_sock_access(env, regno, off, size, t);
+		if (!err && value_regno >= 0)
+			mark_reg_unknown(env, regs, value_regno);
+
 	} else {
 		verbose(env, "R%d invalid mem access '%s'\n", regno,
 			reg_type_str[reg->type]);
@@ -1918,6 +1957,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 		err = check_ctx_reg(env, reg, regno);
 		if (err < 0)
 			return err;
+	} else if (arg_type == ARG_PTR_TO_SOCKET) {
+		expected_type = PTR_TO_SOCKET;
+		if (type != expected_type)
+			goto err_type;
 	} else if (arg_type_is_mem_ptr(arg_type)) {
 		expected_type = PTR_TO_STACK;
 		/* One exception here. In case function allows for NULL to be
@@ -2511,6 +2554,10 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		}
 		regs[BPF_REG_0].map_ptr = meta.map_ptr;
 		regs[BPF_REG_0].id = ++env->id_gen;
+	} else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
+		mark_reg_known_zero(env, regs, BPF_REG_0);
+		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
+		regs[BPF_REG_0].id = ++env->id_gen;
 	} else {
 		verbose(env, "unknown return type %d of func %s#%d\n",
 			fn->ret_type, func_id_name(func_id), func_id);
@@ -2648,6 +2695,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		return -EACCES;
 	case CONST_PTR_TO_MAP:
 	case PTR_TO_PACKET_END:
+	case PTR_TO_SOCKET:
+	case PTR_TO_SOCKET_OR_NULL:
 		verbose(env, "R%d pointer arithmetic on %s prohibited\n",
 			dst, reg_type_str[ptr_reg->type]);
 		return -EACCES;
@@ -3595,6 +3644,8 @@ static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
 			} else {
 				reg->type = PTR_TO_MAP_VALUE;
 			}
+		} else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
+			reg->type = PTR_TO_SOCKET;
 		}
 		/* We don't need id from this point onwards anymore, thus we
 		 * should better reset it, so that state pruning has chances
@@ -4369,6 +4420,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
 	case PTR_TO_CTX:
 	case CONST_PTR_TO_MAP:
 	case PTR_TO_PACKET_END:
+	case PTR_TO_SOCKET:
+	case PTR_TO_SOCKET_OR_NULL:
 		/* Only valid matches are exact, which memcmp() above
 		 * would have accepted
 		 */
@@ -4646,6 +4699,37 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
 	return 0;
 }
 
+/* Return true if it's OK to have the same insn return a different type. */
+static bool reg_type_mismatch_ok(enum bpf_reg_type type)
+{
+	switch (type) {
+	case PTR_TO_CTX:
+	case PTR_TO_SOCKET:
+	case PTR_TO_SOCKET_OR_NULL:
+		return false;
+	default:
+		return true;
+	}
+}
+
+/* If an instruction was previously used with particular pointer types, then we
+ * need to be careful to avoid cases such as the below, where it may be ok
+ * for one branch accessing the pointer, but not ok for the other branch:
+ *
+ * R1 = sock_ptr
+ * goto X;
+ * ...
+ * R1 = some_other_valid_ptr;
+ * goto X;
+ * ...
+ * R2 = *(u32 *)(R1 + 0);
+ */
+static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
+{
+	return src != prev && (!reg_type_mismatch_ok(src) ||
+			       !reg_type_mismatch_ok(prev));
+}
+
 static int do_check(struct bpf_verifier_env *env)
 {
 	struct bpf_verifier_state *state;
@@ -4778,9 +4862,7 @@ static int do_check(struct bpf_verifier_env *env)
 				 */
 				*prev_src_type = src_reg_type;
 
-			} else if (src_reg_type != *prev_src_type &&
-				   (src_reg_type == PTR_TO_CTX ||
-				    *prev_src_type == PTR_TO_CTX)) {
+			} else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
 				/* ABuser program is trying to use the same insn
 				 * dst_reg = *(u32*) (src_reg + off)
 				 * with different pointer types:
@@ -4826,8 +4908,8 @@ static int do_check(struct bpf_verifier_env *env)
 			if (*prev_dst_type == NOT_INIT) {
 				*prev_dst_type = dst_reg_type;
 			} else if (dst_reg_type != *prev_dst_type &&
-				   (dst_reg_type == PTR_TO_CTX ||
-				    *prev_dst_type == PTR_TO_CTX)) {
+				   (!reg_type_mismatch_ok(dst_reg_type) ||
+				    !reg_type_mismatch_ok(*prev_dst_type))) {
 				verbose(env, "same insn cannot be used with different pointers\n");
 				return -EINVAL;
 			}
@@ -5244,10 +5326,14 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
 	}
 }
 
-/* convert load instructions that access fields of 'struct __sk_buff'
- * into sequence of instructions that access fields of 'struct sk_buff'
+/* convert load instructions that access fields of a context type into a
+ * sequence of instructions that access fields of the underlying structure:
+ *     struct __sk_buff    -> struct sk_buff
+ *     struct bpf_sock_ops -> struct sock
  */
-static int convert_ctx_accesses(struct bpf_verifier_env *env)
+static int convert_ctx_accesses(struct bpf_verifier_env *env,
+				bpf_convert_ctx_access_t convert_ctx_access,
+				enum bpf_reg_type ctx_type)
 {
 	const struct bpf_verifier_ops *ops = env->ops;
 	int i, cnt, size, ctx_field_size, delta = 0;
@@ -5274,12 +5360,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 		}
 	}
 
-	if (!ops->convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
+	if (!convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
 		return 0;
 
 	insn = env->prog->insnsi + delta;
 
 	for (i = 0; i < insn_cnt; i++, insn++) {
+		enum bpf_reg_type ptr_type;
+
 		if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
 		    insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
 		    insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
@@ -5321,7 +5409,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 			continue;
 		}
 
-		if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
+		ptr_type = env->insn_aux_data[i + delta].ptr_type;
+		if (ptr_type != ctx_type)
 			continue;
 
 		ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
@@ -5354,8 +5443,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 		}
 
 		target_size = 0;
-		cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog,
-					      &target_size);
+		cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
+					 &target_size);
 		if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
 		    (ctx_field_size && !target_size)) {
 			verbose(env, "bpf verifier is misconfigured\n");
@@ -5899,7 +5988,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
 
 	if (ret == 0)
 		/* program is valid, convert *(u32*)(ctx + off) accesses */
-		ret = convert_ctx_accesses(env);
+		ret = convert_ctx_accesses(env, env->ops->convert_ctx_access,
+					   PTR_TO_CTX);
+
+	if (ret == 0)
+		/* Convert *(u32*)(sock_ops + off) accesses */
+		ret = convert_ctx_accesses(env, bpf_sock_convert_ctx_access,
+					   PTR_TO_SOCKET);
 
 	if (ret == 0)
 		ret = fixup_bpf_calls(env);
diff --git a/net/core/filter.c b/net/core/filter.c
index 8cb242b4400f..23e6e5202138 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5375,23 +5375,29 @@ static bool __sock_filter_check_size(int off, int size,
 	return size == size_default;
 }
 
-static bool sock_filter_is_valid_access(int off, int size,
-					enum bpf_access_type type,
-					const struct bpf_prog *prog,
-					struct bpf_insn_access_aux *info)
+bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
+			      struct bpf_insn_access_aux *info)
 {
 	if (off < 0 || off >= sizeof(struct bpf_sock))
 		return false;
 	if (off % size != 0)
 		return false;
-	if (!__sock_filter_check_attach_type(off, type,
-					     prog->expected_attach_type))
-		return false;
 	if (!__sock_filter_check_size(off, size, info))
 		return false;
 	return true;
 }
 
+static bool sock_filter_is_valid_access(int off, int size,
+					enum bpf_access_type type,
+					const struct bpf_prog *prog,
+					struct bpf_insn_access_aux *info)
+{
+	if (!__sock_filter_check_attach_type(off, type,
+					     prog->expected_attach_type))
+		return false;
+	return bpf_sock_is_valid_access(off, size, type, info);
+}
+
 static int bpf_unclone_prologue(struct bpf_insn *insn_buf, bool direct_write,
 				const struct bpf_prog *prog, int drop_verdict)
 {
@@ -6059,10 +6065,10 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
 	return insn - insn_buf;
 }
 
-static u32 sock_filter_convert_ctx_access(enum bpf_access_type type,
-					  const struct bpf_insn *si,
-					  struct bpf_insn *insn_buf,
-					  struct bpf_prog *prog, u32 *target_size)
+u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
+				const struct bpf_insn *si,
+				struct bpf_insn *insn_buf,
+				struct bpf_prog *prog, u32 *target_size)
 {
 	struct bpf_insn *insn = insn_buf;
 	int off;
@@ -6974,7 +6980,7 @@ const struct bpf_prog_ops lwt_seg6local_prog_ops = {
 const struct bpf_verifier_ops cg_sock_verifier_ops = {
 	.get_func_proto		= sock_filter_func_proto,
 	.is_valid_access	= sock_filter_is_valid_access,
-	.convert_ctx_access	= sock_filter_convert_ctx_access,
+	.convert_ctx_access	= bpf_sock_convert_ctx_access,
 };
 
 const struct bpf_prog_ops cg_sock_prog_ops = {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
  2018-09-12  0:36 ` [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type Joe Stringer
@ 2018-09-12 22:50   ` Alexei Starovoitov
  2018-09-13 18:59     ` Joe Stringer
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-12 22:50 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:33PM -0700, Joe Stringer wrote:
> Teach the verifier a little bit about a new type of pointer, a
> PTR_TO_SOCKET. This pointer type is accessed from BPF through the
> 'struct bpf_sock' structure.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ---
>  include/linux/bpf.h          |  17 +++++
>  include/linux/bpf_verifier.h |   2 +
>  kernel/bpf/verifier.c        | 125 ++++++++++++++++++++++++++++++-----
>  net/core/filter.c            |  30 +++++----
>  4 files changed, 147 insertions(+), 27 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 523481a3471b..6ec93f3d66dd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -154,6 +154,7 @@ enum bpf_arg_type {
>  
>  	ARG_PTR_TO_CTX,		/* pointer to context */
>  	ARG_ANYTHING,		/* any (initialized) argument is ok */
> +	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock */
>  };
>  
>  /* type of values returned from helper functions */
> @@ -162,6 +163,7 @@ enum bpf_return_type {
>  	RET_VOID,			/* function doesn't return anything */
>  	RET_PTR_TO_MAP_VALUE,		/* returns a pointer to map elem value */
>  	RET_PTR_TO_MAP_VALUE_OR_NULL,	/* returns a pointer to map elem value or NULL */
> +	RET_PTR_TO_SOCKET_OR_NULL,	/* returns a pointer to a socket or NULL */
>  };
>  
>  /* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
> @@ -212,6 +214,8 @@ enum bpf_reg_type {
>  	PTR_TO_PACKET_META,	 /* skb->data - meta_len */
>  	PTR_TO_PACKET,		 /* reg points to skb->data */
>  	PTR_TO_PACKET_END,	 /* skb->data + headlen */
> +	PTR_TO_SOCKET,		 /* reg points to struct bpf_sock */
> +	PTR_TO_SOCKET_OR_NULL,	 /* reg points to struct bpf_sock or NULL */
>  };
>  
>  /* The information passed from prog-specific *_is_valid_access
> @@ -334,6 +338,11 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
>  
>  typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src,
>  					unsigned long off, unsigned long len);
> +typedef u32 (*bpf_convert_ctx_access_t)(enum bpf_access_type type,
> +					const struct bpf_insn *src,
> +					struct bpf_insn *dst,
> +					struct bpf_prog *prog,
> +					u32 *target_size);
>  
>  u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>  		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy);
> @@ -827,4 +836,12 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
>  void bpf_user_rnd_init_once(void);
>  u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>  
> +bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
> +			      struct bpf_insn_access_aux *info);
> +u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
> +			        const struct bpf_insn *si,
> +			        struct bpf_insn *insn_buf,
> +			        struct bpf_prog *prog,
> +			        u32 *target_size);
> +
>  #endif /* _LINUX_BPF_H */
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index af262b97f586..23a2b17bfd75 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -58,6 +58,8 @@ struct bpf_reg_state {
>  	 * offset, so they can share range knowledge.
>  	 * For PTR_TO_MAP_VALUE_OR_NULL this is used to share which map value we
>  	 * came from, when one is tested for != NULL.
> +	 * For PTR_TO_SOCKET this is used to share which pointers retain the
> +	 * same reference to the socket, to determine proper reference freeing.
>  	 */
>  	u32 id;
>  	/* For scalar types (SCALAR_VALUE), this represents our knowledge of
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f2357c8c90de..111e031cf65d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -80,8 +80,8 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
>   * (like pointer plus pointer becomes SCALAR_VALUE type)
>   *
>   * When verifier sees load or store instructions the type of base register
> - * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK. These are three pointer
> - * types recognized by check_mem_access() function.
> + * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK, PTR_TO_SOCKET. These are
> + * four pointer types recognized by check_mem_access() function.
>   *
>   * PTR_TO_MAP_VALUE means that this register is pointing to 'map element value'
>   * and the range of [ptr, ptr + map's value_size) is accessible.
> @@ -266,6 +266,8 @@ static const char * const reg_type_str[] = {
>  	[PTR_TO_PACKET]		= "pkt",
>  	[PTR_TO_PACKET_META]	= "pkt_meta",
>  	[PTR_TO_PACKET_END]	= "pkt_end",
> +	[PTR_TO_SOCKET]		= "sock",
> +	[PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
>  };
>  
>  static char slot_type_char[] = {
> @@ -971,6 +973,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
>  	case PTR_TO_PACKET_META:
>  	case PTR_TO_PACKET_END:
>  	case CONST_PTR_TO_MAP:
> +	case PTR_TO_SOCKET:
> +	case PTR_TO_SOCKET_OR_NULL:
>  		return true;
>  	default:
>  		return false;
> @@ -1326,6 +1330,28 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
>  	return -EACCES;
>  }
>  
> +static int check_sock_access(struct bpf_verifier_env *env, u32 regno, int off,
> +			     int size, enum bpf_access_type t)
> +{
> +	struct bpf_reg_state *regs = cur_regs(env);
> +	struct bpf_reg_state *reg = &regs[regno];
> +	struct bpf_insn_access_aux info;
> +
> +	if (reg->smin_value < 0) {
> +		verbose(env, "R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
> +			regno);
> +		return -EACCES;
> +	}
> +
> +	if (!bpf_sock_is_valid_access(off, size, t, &info)) {
> +		verbose(env, "invalid bpf_sock_ops access off=%d size=%d\n",
> +			off, size);
> +		return -EACCES;
> +	}
> +
> +	return 0;
> +}
> +
>  static bool __is_pointer_value(bool allow_ptr_leaks,
>  			       const struct bpf_reg_state *reg)
>  {
> @@ -1441,6 +1467,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
>  		 */
>  		strict = true;
>  		break;
> +	case PTR_TO_SOCKET:
> +		pointer_desc = "sock ";
> +		break;
>  	default:
>  		break;
>  	}
> @@ -1697,6 +1726,16 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>  		err = check_packet_access(env, regno, off, size, false);
>  		if (!err && t == BPF_READ && value_regno >= 0)
>  			mark_reg_unknown(env, regs, value_regno);
> +
> +	} else if (reg->type == PTR_TO_SOCKET) {
> +		if (t == BPF_WRITE) {
> +			verbose(env, "cannot write into socket\n");
> +			return -EACCES;
> +		}
> +		err = check_sock_access(env, regno, off, size, t);
> +		if (!err && value_regno >= 0)
> +			mark_reg_unknown(env, regs, value_regno);
> +
>  	} else {
>  		verbose(env, "R%d invalid mem access '%s'\n", regno,
>  			reg_type_str[reg->type]);
> @@ -1918,6 +1957,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
>  		err = check_ctx_reg(env, reg, regno);
>  		if (err < 0)
>  			return err;
> +	} else if (arg_type == ARG_PTR_TO_SOCKET) {
> +		expected_type = PTR_TO_SOCKET;
> +		if (type != expected_type)
> +			goto err_type;
>  	} else if (arg_type_is_mem_ptr(arg_type)) {
>  		expected_type = PTR_TO_STACK;
>  		/* One exception here. In case function allows for NULL to be
> @@ -2511,6 +2554,10 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>  		}
>  		regs[BPF_REG_0].map_ptr = meta.map_ptr;
>  		regs[BPF_REG_0].id = ++env->id_gen;
> +	} else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
> +		mark_reg_known_zero(env, regs, BPF_REG_0);
> +		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
> +		regs[BPF_REG_0].id = ++env->id_gen;
>  	} else {
>  		verbose(env, "unknown return type %d of func %s#%d\n",
>  			fn->ret_type, func_id_name(func_id), func_id);
> @@ -2648,6 +2695,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	case CONST_PTR_TO_MAP:
>  	case PTR_TO_PACKET_END:
> +	case PTR_TO_SOCKET:
> +	case PTR_TO_SOCKET_OR_NULL:
>  		verbose(env, "R%d pointer arithmetic on %s prohibited\n",
>  			dst, reg_type_str[ptr_reg->type]);
>  		return -EACCES;
> @@ -3595,6 +3644,8 @@ static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
>  			} else {
>  				reg->type = PTR_TO_MAP_VALUE;
>  			}
> +		} else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
> +			reg->type = PTR_TO_SOCKET;
>  		}
>  		/* We don't need id from this point onwards anymore, thus we
>  		 * should better reset it, so that state pruning has chances
> @@ -4369,6 +4420,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
>  	case PTR_TO_CTX:
>  	case CONST_PTR_TO_MAP:
>  	case PTR_TO_PACKET_END:
> +	case PTR_TO_SOCKET:
> +	case PTR_TO_SOCKET_OR_NULL:
>  		/* Only valid matches are exact, which memcmp() above
>  		 * would have accepted
>  		 */
> @@ -4646,6 +4699,37 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
>  	return 0;
>  }
>  
> +/* Return true if it's OK to have the same insn return a different type. */
> +static bool reg_type_mismatch_ok(enum bpf_reg_type type)
> +{
> +	switch (type) {
> +	case PTR_TO_CTX:
> +	case PTR_TO_SOCKET:
> +	case PTR_TO_SOCKET_OR_NULL:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +/* If an instruction was previously used with particular pointer types, then we
> + * need to be careful to avoid cases such as the below, where it may be ok
> + * for one branch accessing the pointer, but not ok for the other branch:
> + *
> + * R1 = sock_ptr
> + * goto X;
> + * ...
> + * R1 = some_other_valid_ptr;
> + * goto X;
> + * ...
> + * R2 = *(u32 *)(R1 + 0);
> + */
> +static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
> +{
> +	return src != prev && (!reg_type_mismatch_ok(src) ||
> +			       !reg_type_mismatch_ok(prev));
> +}
> +
>  static int do_check(struct bpf_verifier_env *env)
>  {
>  	struct bpf_verifier_state *state;
> @@ -4778,9 +4862,7 @@ static int do_check(struct bpf_verifier_env *env)
>  				 */
>  				*prev_src_type = src_reg_type;
>  
> -			} else if (src_reg_type != *prev_src_type &&
> -				   (src_reg_type == PTR_TO_CTX ||
> -				    *prev_src_type == PTR_TO_CTX)) {
> +			} else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
>  				/* ABuser program is trying to use the same insn
>  				 * dst_reg = *(u32*) (src_reg + off)
>  				 * with different pointer types:
> @@ -4826,8 +4908,8 @@ static int do_check(struct bpf_verifier_env *env)
>  			if (*prev_dst_type == NOT_INIT) {
>  				*prev_dst_type = dst_reg_type;
>  			} else if (dst_reg_type != *prev_dst_type &&
> -				   (dst_reg_type == PTR_TO_CTX ||
> -				    *prev_dst_type == PTR_TO_CTX)) {
> +				   (!reg_type_mismatch_ok(dst_reg_type) ||
> +				    !reg_type_mismatch_ok(*prev_dst_type))) {

reg_type_mismatch() could have been used here as well ?

>  				verbose(env, "same insn cannot be used with different pointers\n");
>  				return -EINVAL;
>  			}
> @@ -5244,10 +5326,14 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
>  	}
>  }
>  
> -/* convert load instructions that access fields of 'struct __sk_buff'
> - * into sequence of instructions that access fields of 'struct sk_buff'
> +/* convert load instructions that access fields of a context type into a
> + * sequence of instructions that access fields of the underlying structure:
> + *     struct __sk_buff    -> struct sk_buff
> + *     struct bpf_sock_ops -> struct sock
>   */
> -static int convert_ctx_accesses(struct bpf_verifier_env *env)
> +static int convert_ctx_accesses(struct bpf_verifier_env *env,
> +				bpf_convert_ctx_access_t convert_ctx_access,
> +				enum bpf_reg_type ctx_type)
>  {
>  	const struct bpf_verifier_ops *ops = env->ops;
>  	int i, cnt, size, ctx_field_size, delta = 0;
> @@ -5274,12 +5360,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>  		}
>  	}
>  
> -	if (!ops->convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
> +	if (!convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
>  		return 0;
>  
>  	insn = env->prog->insnsi + delta;
>  
>  	for (i = 0; i < insn_cnt; i++, insn++) {
> +		enum bpf_reg_type ptr_type;
> +
>  		if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
>  		    insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
>  		    insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
> @@ -5321,7 +5409,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>  			continue;
>  		}
>  
> -		if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
> +		ptr_type = env->insn_aux_data[i + delta].ptr_type;
> +		if (ptr_type != ctx_type)
>  			continue;
>  
>  		ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
> @@ -5354,8 +5443,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>  		}
>  
>  		target_size = 0;
> -		cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog,
> -					      &target_size);
> +		cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
> +					 &target_size);
>  		if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
>  		    (ctx_field_size && !target_size)) {
>  			verbose(env, "bpf verifier is misconfigured\n");
> @@ -5899,7 +5988,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
>  
>  	if (ret == 0)
>  		/* program is valid, convert *(u32*)(ctx + off) accesses */
> -		ret = convert_ctx_accesses(env);
> +		ret = convert_ctx_accesses(env, env->ops->convert_ctx_access,
> +					   PTR_TO_CTX);
> +
> +	if (ret == 0)
> +		/* Convert *(u32*)(sock_ops + off) accesses */
> +		ret = convert_ctx_accesses(env, bpf_sock_convert_ctx_access,
> +					   PTR_TO_SOCKET);

can it be done in single pass instead?
env->insn_aux_data tells us what kind of conversion needs to happen.
while progs are small, I guess, it's ok, but would like to see a follow up
patch to this.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
  2018-09-12 22:50   ` Alexei Starovoitov
@ 2018-09-13 18:59     ` Joe Stringer
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-13 18:59 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Stringer, daniel, netdev, ast, john fastabend, tgraf,
	Martin KaFai Lau, Nitin Hande, mauricio.vasquez

On Wed, 12 Sep 2018 at 15:50, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 11, 2018 at 05:36:33PM -0700, Joe Stringer wrote:
> > ...
> > +static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
> > +{
> > +     return src != prev && (!reg_type_mismatch_ok(src) ||
> > +                            !reg_type_mismatch_ok(prev));
> > +}
> > +
> >  static int do_check(struct bpf_verifier_env *env)
> >  {
> >       struct bpf_verifier_state *state;
> > @@ -4778,9 +4862,7 @@ static int do_check(struct bpf_verifier_env *env)
> >                                */
> >                               *prev_src_type = src_reg_type;
> >
> > -                     } else if (src_reg_type != *prev_src_type &&
> > -                                (src_reg_type == PTR_TO_CTX ||
> > -                                 *prev_src_type == PTR_TO_CTX)) {
> > +                     } else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
> >                               /* ABuser program is trying to use the same insn
> >                                * dst_reg = *(u32*) (src_reg + off)
> >                                * with different pointer types:
> > @@ -4826,8 +4908,8 @@ static int do_check(struct bpf_verifier_env *env)
> >                       if (*prev_dst_type == NOT_INIT) {
> >                               *prev_dst_type = dst_reg_type;
> >                       } else if (dst_reg_type != *prev_dst_type &&
> > -                                (dst_reg_type == PTR_TO_CTX ||
> > -                                 *prev_dst_type == PTR_TO_CTX)) {
> > +                                (!reg_type_mismatch_ok(dst_reg_type) ||
> > +                                 !reg_type_mismatch_ok(*prev_dst_type))) {
>
> reg_type_mismatch() could have been used here as well ?

Missed that before, will fix.

> >                               verbose(env, "same insn cannot be used with different pointers\n");
> >                               return -EINVAL;
> >                       }
> > @@ -5244,10 +5326,14 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
> >       }
> >  }
> >
> > -/* convert load instructions that access fields of 'struct __sk_buff'
> > - * into sequence of instructions that access fields of 'struct sk_buff'
> > +/* convert load instructions that access fields of a context type into a
> > + * sequence of instructions that access fields of the underlying structure:
> > + *     struct __sk_buff    -> struct sk_buff
> > + *     struct bpf_sock_ops -> struct sock
> >   */
> > -static int convert_ctx_accesses(struct bpf_verifier_env *env)
> > +static int convert_ctx_accesses(struct bpf_verifier_env *env,
> > +                             bpf_convert_ctx_access_t convert_ctx_access,
> > +                             enum bpf_reg_type ctx_type)
> >  {
> >       const struct bpf_verifier_ops *ops = env->ops;
> >       int i, cnt, size, ctx_field_size, delta = 0;
> > @@ -5274,12 +5360,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> >               }
> >       }
> >
> > -     if (!ops->convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
> > +     if (!convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
> >               return 0;
> >
> >       insn = env->prog->insnsi + delta;
> >
> >       for (i = 0; i < insn_cnt; i++, insn++) {
> > +             enum bpf_reg_type ptr_type;
> > +
> >               if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
> >                   insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
> >                   insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
> > @@ -5321,7 +5409,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> >                       continue;
> >               }
> >
> > -             if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
> > +             ptr_type = env->insn_aux_data[i + delta].ptr_type;
> > +             if (ptr_type != ctx_type)
> >                       continue;
> >
> >               ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
> > @@ -5354,8 +5443,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> >               }
> >
> >               target_size = 0;
> > -             cnt = ops->convert_ctx_access(type, insn, insn_buf, env->prog,
> > -                                           &target_size);
> > +             cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
> > +                                      &target_size);
> >               if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
> >                   (ctx_field_size && !target_size)) {
> >                       verbose(env, "bpf verifier is misconfigured\n");
> > @@ -5899,7 +5988,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
> >
> >       if (ret == 0)
> >               /* program is valid, convert *(u32*)(ctx + off) accesses */
> > -             ret = convert_ctx_accesses(env);
> > +             ret = convert_ctx_accesses(env, env->ops->convert_ctx_access,
> > +                                        PTR_TO_CTX);
> > +
> > +     if (ret == 0)
> > +             /* Convert *(u32*)(sock_ops + off) accesses */
> > +             ret = convert_ctx_accesses(env, bpf_sock_convert_ctx_access,
> > +                                        PTR_TO_SOCKET);
>
> can it be done in single pass instead?
> env->insn_aux_data tells us what kind of conversion needs to happen.
> while progs are small, I guess, it's ok, but would like to see a follow up
> patch to this.

Yeah, good point - upon review it seems like this would be a simple
change (incremental):

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index db05f78bfc6e..7fa1b695a2a1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5329,9 +5329,7 @@ static void sanitize_dead_code(struct
bpf_verifier_env *env)
 *     struct __sk_buff    -> struct sk_buff
 *     struct bpf_sock_ops -> struct sock
 */
-static int convert_ctx_accesses(struct bpf_verifier_env *env,
-                               bpf_convert_ctx_access_t convert_ctx_access,
-                               enum bpf_reg_type ctx_type)
+static int convert_ctx_accesses(struct bpf_verifier_env *env)
{
       const struct bpf_verifier_ops *ops = env->ops;
       int i, cnt, size, ctx_field_size, delta = 0;
@@ -5358,13 +5356,13 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env,
               }
       }

-       if (!convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
+       if (bpf_prog_is_dev_bound(env->prog->aux))
               return 0;

       insn = env->prog->insnsi + delta;

       for (i = 0; i < insn_cnt; i++, insn++) {
-               enum bpf_reg_type ptr_type;
+               bpf_convert_ctx_access_t convert_ctx_access;

               if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
                   insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
@@ -5407,9 +5405,18 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env,
                       continue;
               }

-               ptr_type = env->insn_aux_data[i + delta].ptr_type;
-               if (ptr_type != ctx_type)
+               switch (env->insn_aux_data[i + delta].ptr_type) {
+               case PTR_TO_CTX:
+                       if (!ops->convert_ctx_access)
+                               continue;
+                       convert_ctx_access = ops->convert_ctx_access;
+                       break;
+               case PTR_TO_SOCKET:
+                       convert_ctx_access = bpf_sock_convert_ctx_access;
+                       break;
+               default:
                       continue;
+               }

               ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
               size = BPF_LDST_BYTES(insn);
@@ -5986,13 +5993,7 @@ int bpf_check(struct bpf_prog **prog, union
bpf_attr *attr)

       if (ret == 0)
               /* program is valid, convert *(u32*)(ctx + off) accesses */
-               ret = convert_ctx_accesses(env, env->ops->convert_ctx_access,
-                                          PTR_TO_CTX);
-
-       if (ret == 0)
-               /* Convert *(u32*)(sock_ops + off) accesses */
-               ret = convert_ctx_accesses(env, bpf_sock_convert_ctx_access,
-                                          PTR_TO_SOCKET);
+               ret = convert_ctx_accesses(env);

       if (ret == 0)
               ret = fixup_bpf_calls(env);

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 05/11] bpf: Macrofy stack state copy
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (3 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12 22:51   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier Joe Stringer
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

An upcoming commit will need very similar copy/realloc boilerplate, so
refactor the existing stack copy/realloc functions into macros to
simplify it.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 kernel/bpf/verifier.c | 106 ++++++++++++++++++++++++------------------
 1 file changed, 60 insertions(+), 46 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 111e031cf65d..faa83b3d7011 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -387,60 +387,74 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 	verbose(env, "\n");
 }
 
-static int copy_stack_state(struct bpf_func_state *dst,
-			    const struct bpf_func_state *src)
-{
-	if (!src->stack)
-		return 0;
-	if (WARN_ON_ONCE(dst->allocated_stack < src->allocated_stack)) {
-		/* internal bug, make state invalid to reject the program */
-		memset(dst, 0, sizeof(*dst));
-		return -EFAULT;
-	}
-	memcpy(dst->stack, src->stack,
-	       sizeof(*src->stack) * (src->allocated_stack / BPF_REG_SIZE));
-	return 0;
-}
+#define COPY_STATE_FN(NAME, COUNT, FIELD, SIZE)				\
+static int copy_##NAME##_state(struct bpf_func_state *dst,		\
+			       const struct bpf_func_state *src)	\
+{									\
+	if (!src->FIELD)						\
+		return 0;						\
+	if (WARN_ON_ONCE(dst->COUNT < src->COUNT)) {			\
+		/* internal bug, make state invalid to reject the program */ \
+		memset(dst, 0, sizeof(*dst));				\
+		return -EFAULT;						\
+	}								\
+	memcpy(dst->FIELD, src->FIELD,					\
+	       sizeof(*src->FIELD) * (src->COUNT / SIZE));		\
+	return 0;							\
+}
+/* copy_stack_state() */
+COPY_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
+#undef COPY_STATE_FN
+
+#define REALLOC_STATE_FN(NAME, COUNT, FIELD, SIZE)			\
+static int realloc_##NAME##_state(struct bpf_func_state *state, int size, \
+				  bool copy_old)			\
+{									\
+	u32 old_size = state->COUNT;					\
+	struct bpf_##NAME##_state *new_##FIELD;				\
+	int slot = size / SIZE;						\
+									\
+	if (size <= old_size || !size) {				\
+		if (copy_old)						\
+			return 0;					\
+		state->COUNT = slot * SIZE;				\
+		if (!size && old_size) {				\
+			kfree(state->FIELD);				\
+			state->FIELD = NULL;				\
+		}							\
+		return 0;						\
+	}								\
+	new_##FIELD = kmalloc_array(slot, sizeof(struct bpf_##NAME##_state), \
+				    GFP_KERNEL);			\
+	if (!new_##FIELD)						\
+		return -ENOMEM;						\
+	if (copy_old) {							\
+		if (state->FIELD)					\
+			memcpy(new_##FIELD, state->FIELD,		\
+			       sizeof(*new_##FIELD) * (old_size / SIZE)); \
+		memset(new_##FIELD + old_size / SIZE, 0,		\
+		       sizeof(*new_##FIELD) * (size - old_size) / SIZE); \
+	}								\
+	state->COUNT = slot * SIZE;					\
+	kfree(state->FIELD);						\
+	state->FIELD = new_##FIELD;					\
+	return 0;							\
+}
+/* realloc_stack_state() */
+REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
+#undef REALLOC_STATE_FN
 
 /* do_check() starts with zero-sized stack in struct bpf_verifier_state to
  * make it consume minimal amount of memory. check_stack_write() access from
  * the program calls into realloc_func_state() to grow the stack size.
- * Note there is a non-zero parent pointer inside each reg of bpf_verifier_state
- * which this function copies over. It points to corresponding reg in previous
- * bpf_verifier_state which is never reallocated
+ * Note there is a non-zero 'parent' pointer inside bpf_verifier_state
+ * which realloc_stack_state() copies over. It points to previous
+ * bpf_verifier_state which is never reallocated.
  */
 static int realloc_func_state(struct bpf_func_state *state, int size,
 			      bool copy_old)
 {
-	u32 old_size = state->allocated_stack;
-	struct bpf_stack_state *new_stack;
-	int slot = size / BPF_REG_SIZE;
-
-	if (size <= old_size || !size) {
-		if (copy_old)
-			return 0;
-		state->allocated_stack = slot * BPF_REG_SIZE;
-		if (!size && old_size) {
-			kfree(state->stack);
-			state->stack = NULL;
-		}
-		return 0;
-	}
-	new_stack = kmalloc_array(slot, sizeof(struct bpf_stack_state),
-				  GFP_KERNEL);
-	if (!new_stack)
-		return -ENOMEM;
-	if (copy_old) {
-		if (state->stack)
-			memcpy(new_stack, state->stack,
-			       sizeof(*new_stack) * (old_size / BPF_REG_SIZE));
-		memset(new_stack + old_size / BPF_REG_SIZE, 0,
-		       sizeof(*new_stack) * (size - old_size) / BPF_REG_SIZE);
-	}
-	state->allocated_stack = slot * BPF_REG_SIZE;
-	kfree(state->stack);
-	state->stack = new_stack;
-	return 0;
+	return realloc_stack_state(state, size, copy_old);
 }
 
 static void free_func_state(struct bpf_func_state *state)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 05/11] bpf: Macrofy stack state copy
  2018-09-12  0:36 ` [PATCH bpf-next 05/11] bpf: Macrofy stack state copy Joe Stringer
@ 2018-09-12 22:51   ` Alexei Starovoitov
  0 siblings, 0 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-12 22:51 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:34PM -0700, Joe Stringer wrote:
> An upcoming commit will need very similar copy/realloc boilerplate, so
> refactor the existing stack copy/realloc functions into macros to
> simplify it.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (4 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 05/11] bpf: Macrofy stack state copy Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-12 23:17   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Allow helper functions to acquire a reference and return it into a
register. Specific pointer types such as the PTR_TO_SOCKET will
implicitly represent such a reference. The verifier must ensure that
these references are released exactly once in each path through the
program.

To achieve this, this commit assigns an id to the pointer and tracks it
in the 'bpf_func_state', then when the function or program exits,
verifies that all of the acquired references have been freed. When the
pointer is passed to a function that frees the reference, it is removed
from the 'bpf_func_state` and all existing copies of the pointer in
registers are marked invalid.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 include/linux/bpf_verifier.h |  24 ++-
 kernel/bpf/verifier.c        | 303 ++++++++++++++++++++++++++++++++---
 2 files changed, 306 insertions(+), 21 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 23a2b17bfd75..23f222e0cb0b 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -104,6 +104,17 @@ struct bpf_stack_state {
 	u8 slot_type[BPF_REG_SIZE];
 };
 
+struct bpf_reference_state {
+	/* Track each reference created with a unique id, even if the same
+	 * instruction creates the reference multiple times (eg, via CALL).
+	 */
+	int id;
+	/* Instruction where the allocation of this reference occurred. This
+	 * is used purely to inform the user of a reference leak.
+	 */
+	int insn_idx;
+};
+
 /* state of the program:
  * type of all registers and stack info
  */
@@ -121,7 +132,9 @@ struct bpf_func_state {
 	 */
 	u32 subprogno;
 
-	/* should be second to last. See copy_func_state() */
+	/* The following fields should be last. See copy_func_state() */
+	int acquired_refs;
+	struct bpf_reference_state *refs;
 	int allocated_stack;
 	struct bpf_stack_state *stack;
 };
@@ -217,11 +230,16 @@ __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
 __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
 					   const char *fmt, ...);
 
-static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
+static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
 {
 	struct bpf_verifier_state *cur = env->cur_state;
 
-	return cur->frame[cur->curframe]->regs;
+	return cur->frame[cur->curframe];
+}
+
+static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
+{
+	return cur_func(env)->regs;
 }
 
 int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index faa83b3d7011..67c62ef67d37 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1,5 +1,6 @@
 /* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
  * Copyright (c) 2016 Facebook
+ * Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of version 2 of the GNU General Public
@@ -140,6 +141,18 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
  *
  * After the call R0 is set to return type of the function and registers R1-R5
  * are set to NOT_INIT to indicate that they are no longer readable.
+ *
+ * The following reference types represent a potential reference to a kernel
+ * resource which, after first being allocated, must be checked and freed by
+ * the BPF program:
+ * - PTR_TO_SOCKET_OR_NULL, PTR_TO_SOCKET
+ *
+ * When the verifier sees a helper call return a reference type, it allocates a
+ * pointer id for the reference and stores it in the current function state.
+ * Similar to the way that PTR_TO_MAP_VALUE_OR_NULL is converted into
+ * PTR_TO_MAP_VALUE, PTR_TO_SOCKET_OR_NULL becomes PTR_TO_SOCKET when the type
+ * passes through a NULL-check conditional. For the branch wherein the state is
+ * changed to CONST_IMM, the verifier releases the reference.
  */
 
 /* verifier_state + insn_idx are pushed to stack when branch is encountered */
@@ -189,6 +202,7 @@ struct bpf_call_arg_meta {
 	int access_size;
 	s64 msize_smax_value;
 	u64 msize_umax_value;
+	int ptr_id;
 };
 
 static DEFINE_MUTEX(bpf_verifier_lock);
@@ -251,7 +265,42 @@ static bool type_is_pkt_pointer(enum bpf_reg_type type)
 
 static bool reg_type_may_be_null(enum bpf_reg_type type)
 {
-	return type == PTR_TO_MAP_VALUE_OR_NULL;
+	return type == PTR_TO_MAP_VALUE_OR_NULL ||
+	       type == PTR_TO_SOCKET_OR_NULL;
+}
+
+static bool type_is_refcounted(enum bpf_reg_type type)
+{
+	return type == PTR_TO_SOCKET;
+}
+
+static bool type_is_refcounted_or_null(enum bpf_reg_type type)
+{
+	return type == PTR_TO_SOCKET || type == PTR_TO_SOCKET_OR_NULL;
+}
+
+static bool reg_is_refcounted(const struct bpf_reg_state *reg)
+{
+	return type_is_refcounted(reg->type);
+}
+
+static bool reg_is_refcounted_or_null(const struct bpf_reg_state *reg)
+{
+	return type_is_refcounted_or_null(reg->type);
+}
+
+static bool arg_type_is_refcounted(enum bpf_arg_type type)
+{
+	return type == ARG_PTR_TO_SOCKET;
+}
+
+/* Determine whether the function releases some resources allocated by another
+ * function call. The first reference type argument will be assumed to be
+ * released by release_reference().
+ */
+static bool is_release_function(enum bpf_func_id func_id)
+{
+	return false;
 }
 
 /* string representation of 'enum bpf_reg_type' */
@@ -384,6 +433,12 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 		else
 			verbose(env, "=%s", types_buf);
 	}
+	if (state->acquired_refs && state->refs[0].id) {
+		verbose(env, " refs=%d", state->refs[0].id);
+		for (i = 1; i < state->acquired_refs; i++)
+			if (state->refs[i].id)
+				verbose(env, ",%d", state->refs[i].id);
+	}
 	verbose(env, "\n");
 }
 
@@ -402,6 +457,8 @@ static int copy_##NAME##_state(struct bpf_func_state *dst,		\
 	       sizeof(*src->FIELD) * (src->COUNT / SIZE));		\
 	return 0;							\
 }
+/* copy_reference_state() */
+COPY_STATE_FN(reference, acquired_refs, refs, 1)
 /* copy_stack_state() */
 COPY_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
 #undef COPY_STATE_FN
@@ -440,6 +497,8 @@ static int realloc_##NAME##_state(struct bpf_func_state *state, int size, \
 	state->FIELD = new_##FIELD;					\
 	return 0;							\
 }
+/* realloc_reference_state() */
+REALLOC_STATE_FN(reference, acquired_refs, refs, 1)
 /* realloc_stack_state() */
 REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
 #undef REALLOC_STATE_FN
@@ -451,16 +510,89 @@ REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
  * which realloc_stack_state() copies over. It points to previous
  * bpf_verifier_state which is never reallocated.
  */
-static int realloc_func_state(struct bpf_func_state *state, int size,
-			      bool copy_old)
+static int realloc_func_state(struct bpf_func_state *state, int stack_size,
+			      int refs_size, bool copy_old)
 {
-	return realloc_stack_state(state, size, copy_old);
+	int err = realloc_reference_state(state, refs_size, copy_old);
+	if (err)
+		return err;
+	return realloc_stack_state(state, stack_size, copy_old);
+}
+
+/* Acquire a pointer id from the env and update the state->refs to include
+ * this new pointer reference.
+ * On success, returns a valid pointer id to associate with the register
+ * On failure, returns a negative errno.
+ */
+static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
+{
+	struct bpf_func_state *state = cur_func(env);
+	int new_ofs = state->acquired_refs;
+	int id, err;
+
+	err = realloc_reference_state(state, state->acquired_refs + 1, true);
+	if (err)
+		return err;
+	id = ++env->id_gen;
+	state->refs[new_ofs].id = id;
+	state->refs[new_ofs].insn_idx = insn_idx;
+
+	return id;
+}
+
+/* release function corresponding to acquire_reference_state(). Idempotent. */
+static int __release_reference_state(struct bpf_func_state *state, int ptr_id)
+{
+	int i, last_idx;
+
+	if (!ptr_id)
+		return 0;
+
+	last_idx = state->acquired_refs - 1;
+	for (i = 0; i < state->acquired_refs; i++) {
+		if (state->refs[i].id == ptr_id) {
+			if (last_idx && i != last_idx)
+				memcpy(&state->refs[i], &state->refs[last_idx],
+				       sizeof(*state->refs));
+			memset(&state->refs[last_idx], 0, sizeof(*state->refs));
+			state->acquired_refs--;
+			return 0;
+		}
+	}
+	return -EFAULT;
+}
+
+/* variation on the above for cases where we expect that there must be an
+ * outstanding reference for the specified ptr_id.
+ */
+static int release_reference_state(struct bpf_verifier_env *env, int ptr_id)
+{
+	struct bpf_func_state *state = cur_func(env);
+	int err;
+
+	err = __release_reference_state(state, ptr_id);
+	if (WARN_ON_ONCE(err != 0))
+		verbose(env, "verifier internal error: can't release reference\n");
+	return err;
+}
+
+static int transfer_reference_state(struct bpf_func_state *dst,
+				    struct bpf_func_state *src)
+{
+	int err = realloc_reference_state(dst, src->acquired_refs, false);
+	if (err)
+		return err;
+	err = copy_reference_state(dst, src);
+	if (err)
+		return err;
+	return 0;
 }
 
 static void free_func_state(struct bpf_func_state *state)
 {
 	if (!state)
 		return;
+	kfree(state->refs);
 	kfree(state->stack);
 	kfree(state);
 }
@@ -486,10 +618,14 @@ static int copy_func_state(struct bpf_func_state *dst,
 {
 	int err;
 
-	err = realloc_func_state(dst, src->allocated_stack, false);
+	err = realloc_func_state(dst, src->allocated_stack, src->acquired_refs,
+				 false);
+	if (err)
+		return err;
+	memcpy(dst, src, offsetof(struct bpf_func_state, acquired_refs));
+	err = copy_reference_state(dst, src);
 	if (err)
 		return err;
-	memcpy(dst, src, offsetof(struct bpf_func_state, allocated_stack));
 	return copy_stack_state(dst, src);
 }
 
@@ -1013,7 +1149,7 @@ static int check_stack_write(struct bpf_verifier_env *env,
 	enum bpf_reg_type type;
 
 	err = realloc_func_state(state, round_up(slot + 1, BPF_REG_SIZE),
-				 true);
+				 state->acquired_refs, true);
 	if (err)
 		return err;
 	/* caller checked that off % size == 0 and -MAX_BPF_STACK <= off < 0,
@@ -1975,6 +2111,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 		expected_type = PTR_TO_SOCKET;
 		if (type != expected_type)
 			goto err_type;
+		if (meta->ptr_id || !reg->id) {
+			verbose(env, "verifier internal error: mismatched references meta=%d, reg=%d\n",
+				meta->ptr_id, reg->id);
+			return -EFAULT;
+		}
+		meta->ptr_id = reg->id;
 	} else if (arg_type_is_mem_ptr(arg_type)) {
 		expected_type = PTR_TO_STACK;
 		/* One exception here. In case function allows for NULL to be
@@ -2262,10 +2404,32 @@ static bool check_arg_pair_ok(const struct bpf_func_proto *fn)
 	return true;
 }
 
+static bool check_refcount_ok(const struct bpf_func_proto *fn)
+{
+	int count = 0;
+
+	if (arg_type_is_refcounted(fn->arg1_type))
+		count++;
+	if (arg_type_is_refcounted(fn->arg2_type))
+		count++;
+	if (arg_type_is_refcounted(fn->arg3_type))
+		count++;
+	if (arg_type_is_refcounted(fn->arg4_type))
+		count++;
+	if (arg_type_is_refcounted(fn->arg5_type))
+		count++;
+
+	/* We only support one arg being unreferenced at the moment,
+	 * which is sufficient for the helper functions we have right now.
+	 */
+	return count <= 1;
+}
+
 static int check_func_proto(const struct bpf_func_proto *fn)
 {
 	return check_raw_mode_ok(fn) &&
-	       check_arg_pair_ok(fn) ? 0 : -EINVAL;
+	       check_arg_pair_ok(fn) &&
+	       check_refcount_ok(fn) ? 0 : -EINVAL;
 }
 
 /* Packet data might have moved, any old PTR_TO_PACKET[_META,_END]
@@ -2298,12 +2462,45 @@ static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
 		__clear_all_pkt_pointers(env, vstate->frame[i]);
 }
 
+static void release_reg_references(struct bpf_verifier_env *env,
+				   struct bpf_func_state *state, int id)
+{
+	struct bpf_reg_state *regs = state->regs, *reg;
+	int i;
+
+	for (i = 0; i < MAX_BPF_REG; i++)
+		if (regs[i].id == id)
+			mark_reg_unknown(env, regs, i);
+
+	for_each_spilled_reg(i, state, reg) {
+		if (!reg)
+			continue;
+		if (reg_is_refcounted(reg) && reg->id == id)
+			__mark_reg_unknown(reg);
+	}
+}
+
+/* The pointer with the specified id has released its reference to kernel
+ * resources. Identify all copies of the same pointer and clear the reference.
+ */
+static int release_reference(struct bpf_verifier_env *env,
+			     struct bpf_call_arg_meta *meta)
+{
+	struct bpf_verifier_state *vstate = env->cur_state;
+	int i;
+
+	for (i = 0; i <= vstate->curframe; i++)
+		release_reg_references(env, vstate->frame[i], meta->ptr_id);
+
+	return release_reference_state(env, meta->ptr_id);
+}
+
 static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			   int *insn_idx)
 {
 	struct bpf_verifier_state *state = env->cur_state;
 	struct bpf_func_state *caller, *callee;
-	int i, subprog, target_insn;
+	int i, err, subprog, target_insn;
 
 	if (state->curframe + 1 >= MAX_CALL_FRAMES) {
 		verbose(env, "the call stack of %d frames is too deep\n",
@@ -2341,6 +2538,11 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			state->curframe + 1 /* frameno within this callchain */,
 			subprog /* subprog number within this prog */);
 
+	/* Transfer references to the callee */
+	err = transfer_reference_state(callee, caller);
+	if (err)
+		return err;
+
 	/* copy r1 - r5 args that callee can access.  The copy includes parent
 	 * pointers, which connects us up to the liveness chain
 	 */
@@ -2373,6 +2575,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 	struct bpf_verifier_state *state = env->cur_state;
 	struct bpf_func_state *caller, *callee;
 	struct bpf_reg_state *r0;
+	int err;
 
 	callee = state->frame[state->curframe];
 	r0 = &callee->regs[BPF_REG_0];
@@ -2392,6 +2595,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 	/* return to the caller whatever r0 had in the callee */
 	caller->regs[BPF_REG_0] = *r0;
 
+	/* Transfer references to the caller */
+	err = transfer_reference_state(caller, callee);
+	if (err)
+		return err;
+
 	*insn_idx = callee->callsite + 1;
 	if (env->log.level) {
 		verbose(env, "returning from callee:\n");
@@ -2448,6 +2656,18 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 	return 0;
 }
 
+static int check_reference_leak(struct bpf_verifier_env *env)
+{
+	struct bpf_func_state *state = cur_func(env);
+	int i;
+
+	for (i = 0; i < state->acquired_refs; i++) {
+		verbose(env, "Unreleased reference id=%d alloc_insn=%d\n",
+			state->refs[i].id, state->refs[i].insn_idx);
+	}
+	return state->acquired_refs ? -EINVAL : 0;
+}
+
 static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
 {
 	const struct bpf_func_proto *fn = NULL;
@@ -2526,6 +2746,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 			return err;
 	}
 
+	if (func_id == BPF_FUNC_tail_call) {
+		err = check_reference_leak(env);
+		if (err) {
+			verbose(env, "tail_call would lead to reference leak\n");
+			return err;
+		}
+	} else if (is_release_function(func_id)) {
+		err = release_reference(env, &meta);
+		if (err)
+			return err;
+	}
+
 	regs = cur_regs(env);
 
 	/* check that flags argument in get_local_storage(map, flags) is 0,
@@ -2569,9 +2801,12 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		regs[BPF_REG_0].map_ptr = meta.map_ptr;
 		regs[BPF_REG_0].id = ++env->id_gen;
 	} else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
+		int id = acquire_reference_state(env, insn_idx);
+		if (id < 0)
+			return id;
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
-		regs[BPF_REG_0].id = ++env->id_gen;
+		regs[BPF_REG_0].id = id;
 	} else {
 		verbose(env, "unknown return type %d of func %s#%d\n",
 			fn->ret_type, func_id_name(func_id), func_id);
@@ -3635,7 +3870,8 @@ static void reg_combine_min_max(struct bpf_reg_state *true_src,
 	}
 }
 
-static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
+static void mark_ptr_or_null_reg(struct bpf_func_state *state,
+				 struct bpf_reg_state *reg, u32 id,
 				 bool is_null)
 {
 	if (reg_type_may_be_null(reg->type) && reg->id == id) {
@@ -3661,11 +3897,13 @@ static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
 		} else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
 			reg->type = PTR_TO_SOCKET;
 		}
-		/* We don't need id from this point onwards anymore, thus we
-		 * should better reset it, so that state pruning has chances
-		 * to take effect.
-		 */
-		reg->id = 0;
+		if (is_null || !reg_is_refcounted(reg)) {
+			/* We don't need id from this point onwards anymore,
+			 * thus we should better reset it, so that state
+			 * pruning has chances to take effect.
+			 */
+			reg->id = 0;
+		}
 	}
 }
 
@@ -3680,15 +3918,18 @@ static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno,
 	u32 id = regs[regno].id;
 	int i, j;
 
+	if (reg_is_refcounted_or_null(&regs[regno]) && is_null)
+		__release_reference_state(state, id);
+
 	for (i = 0; i < MAX_BPF_REG; i++)
-		mark_ptr_or_null_reg(&regs[i], id, is_null);
+		mark_ptr_or_null_reg(state, &regs[i], id, is_null);
 
 	for (j = 0; j <= vstate->curframe; j++) {
 		state = vstate->frame[j];
 		for_each_spilled_reg(i, state, reg) {
 			if (!reg)
 				continue;
-			mark_ptr_or_null_reg(reg, id, is_null);
+			mark_ptr_or_null_reg(state, reg, id, is_null);
 		}
 	}
 }
@@ -4020,6 +4261,16 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	if (err)
 		return err;
 
+	/* Disallow usage of BPF_LD_[ABS|IND] with reference tracking, as
+	 * gen_ld_abs() may terminate the program at runtime, leading to
+	 * reference leak.
+	 */
+	err = check_reference_leak(env);
+	if (err) {
+		verbose(env, "BPF_LD_[ABS|IND] cannot be mixed with socket references\n");
+		return err;
+	}
+
 	if (regs[BPF_REG_6].type != PTR_TO_CTX) {
 		verbose(env,
 			"at the time of BPF_LD_ABS|IND R6 != pointer to skb\n");
@@ -4511,6 +4762,14 @@ static bool stacksafe(struct bpf_func_state *old,
 	return true;
 }
 
+static bool refsafe(struct bpf_func_state *old, struct bpf_func_state *cur)
+{
+	if (old->acquired_refs != cur->acquired_refs)
+		return false;
+	return !memcmp(old->refs, cur->refs,
+		       sizeof(*old->refs) * old->acquired_refs);
+}
+
 /* compare two verifier states
  *
  * all states stored in state_list are known to be valid, since
@@ -4556,6 +4815,9 @@ static bool func_states_equal(struct bpf_func_state *old,
 
 	if (!stacksafe(old, cur, idmap))
 		goto out_free;
+
+	if (!refsafe(old, cur))
+		goto out_free;
 	ret = true;
 out_free:
 	kfree(idmap);
@@ -4837,6 +5099,7 @@ static int do_check(struct bpf_verifier_env *env)
 
 		regs = cur_regs(env);
 		env->insn_aux_data[insn_idx].seen = true;
+
 		if (class == BPF_ALU || class == BPF_ALU64) {
 			err = check_alu_op(env, insn);
 			if (err)
@@ -5003,6 +5266,10 @@ static int do_check(struct bpf_verifier_env *env)
 					continue;
 				}
 
+				err = check_reference_leak(env);
+				if (err)
+					return err;
+
 				/* eBPF calling convetion is such that R0 is used
 				 * to return the value from eBPF program.
 				 * Make sure that it's readable at this time
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier
  2018-09-12  0:36 ` [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier Joe Stringer
@ 2018-09-12 23:17   ` Alexei Starovoitov
  2018-09-13 19:00     ` Joe Stringer
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-12 23:17 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:35PM -0700, Joe Stringer wrote:
> Allow helper functions to acquire a reference and return it into a
> register. Specific pointer types such as the PTR_TO_SOCKET will
> implicitly represent such a reference. The verifier must ensure that
> these references are released exactly once in each path through the
> program.
> 
> To achieve this, this commit assigns an id to the pointer and tracks it
> in the 'bpf_func_state', then when the function or program exits,
> verifies that all of the acquired references have been freed. When the
> pointer is passed to a function that frees the reference, it is removed
> from the 'bpf_func_state` and all existing copies of the pointer in
> registers are marked invalid.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ---
>  include/linux/bpf_verifier.h |  24 ++-
>  kernel/bpf/verifier.c        | 303 ++++++++++++++++++++++++++++++++---
>  2 files changed, 306 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 23a2b17bfd75..23f222e0cb0b 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -104,6 +104,17 @@ struct bpf_stack_state {
>  	u8 slot_type[BPF_REG_SIZE];
>  };
>  
> +struct bpf_reference_state {
> +	/* Track each reference created with a unique id, even if the same
> +	 * instruction creates the reference multiple times (eg, via CALL).
> +	 */
> +	int id;
> +	/* Instruction where the allocation of this reference occurred. This
> +	 * is used purely to inform the user of a reference leak.
> +	 */
> +	int insn_idx;
> +};
> +
>  /* state of the program:
>   * type of all registers and stack info
>   */
> @@ -121,7 +132,9 @@ struct bpf_func_state {
>  	 */
>  	u32 subprogno;
>  
> -	/* should be second to last. See copy_func_state() */
> +	/* The following fields should be last. See copy_func_state() */
> +	int acquired_refs;
> +	struct bpf_reference_state *refs;
>  	int allocated_stack;
>  	struct bpf_stack_state *stack;
>  };
> @@ -217,11 +230,16 @@ __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log,
>  __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
>  					   const char *fmt, ...);
>  
> -static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
> +static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
>  {
>  	struct bpf_verifier_state *cur = env->cur_state;
>  
> -	return cur->frame[cur->curframe]->regs;
> +	return cur->frame[cur->curframe];
> +}
> +
> +static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
> +{
> +	return cur_func(env)->regs;
>  }
>  
>  int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index faa83b3d7011..67c62ef67d37 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1,5 +1,6 @@
>  /* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
>   * Copyright (c) 2016 Facebook
> + * Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of version 2 of the GNU General Public
> @@ -140,6 +141,18 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
>   *
>   * After the call R0 is set to return type of the function and registers R1-R5
>   * are set to NOT_INIT to indicate that they are no longer readable.
> + *
> + * The following reference types represent a potential reference to a kernel
> + * resource which, after first being allocated, must be checked and freed by
> + * the BPF program:
> + * - PTR_TO_SOCKET_OR_NULL, PTR_TO_SOCKET
> + *
> + * When the verifier sees a helper call return a reference type, it allocates a
> + * pointer id for the reference and stores it in the current function state.
> + * Similar to the way that PTR_TO_MAP_VALUE_OR_NULL is converted into
> + * PTR_TO_MAP_VALUE, PTR_TO_SOCKET_OR_NULL becomes PTR_TO_SOCKET when the type
> + * passes through a NULL-check conditional. For the branch wherein the state is
> + * changed to CONST_IMM, the verifier releases the reference.
>   */
>  
>  /* verifier_state + insn_idx are pushed to stack when branch is encountered */
> @@ -189,6 +202,7 @@ struct bpf_call_arg_meta {
>  	int access_size;
>  	s64 msize_smax_value;
>  	u64 msize_umax_value;
> +	int ptr_id;
>  };
>  
>  static DEFINE_MUTEX(bpf_verifier_lock);
> @@ -251,7 +265,42 @@ static bool type_is_pkt_pointer(enum bpf_reg_type type)
>  
>  static bool reg_type_may_be_null(enum bpf_reg_type type)
>  {
> -	return type == PTR_TO_MAP_VALUE_OR_NULL;
> +	return type == PTR_TO_MAP_VALUE_OR_NULL ||
> +	       type == PTR_TO_SOCKET_OR_NULL;
> +}
> +
> +static bool type_is_refcounted(enum bpf_reg_type type)
> +{
> +	return type == PTR_TO_SOCKET;
> +}
> +
> +static bool type_is_refcounted_or_null(enum bpf_reg_type type)
> +{
> +	return type == PTR_TO_SOCKET || type == PTR_TO_SOCKET_OR_NULL;
> +}
> +
> +static bool reg_is_refcounted(const struct bpf_reg_state *reg)
> +{
> +	return type_is_refcounted(reg->type);
> +}
> +
> +static bool reg_is_refcounted_or_null(const struct bpf_reg_state *reg)
> +{
> +	return type_is_refcounted_or_null(reg->type);
> +}
> +
> +static bool arg_type_is_refcounted(enum bpf_arg_type type)
> +{
> +	return type == ARG_PTR_TO_SOCKET;
> +}
> +
> +/* Determine whether the function releases some resources allocated by another
> + * function call. The first reference type argument will be assumed to be
> + * released by release_reference().
> + */
> +static bool is_release_function(enum bpf_func_id func_id)
> +{
> +	return false;
>  }
>  
>  /* string representation of 'enum bpf_reg_type' */
> @@ -384,6 +433,12 @@ static void print_verifier_state(struct bpf_verifier_env *env,
>  		else
>  			verbose(env, "=%s", types_buf);
>  	}
> +	if (state->acquired_refs && state->refs[0].id) {
> +		verbose(env, " refs=%d", state->refs[0].id);
> +		for (i = 1; i < state->acquired_refs; i++)
> +			if (state->refs[i].id)
> +				verbose(env, ",%d", state->refs[i].id);
> +	}
>  	verbose(env, "\n");
>  }
>  
> @@ -402,6 +457,8 @@ static int copy_##NAME##_state(struct bpf_func_state *dst,		\
>  	       sizeof(*src->FIELD) * (src->COUNT / SIZE));		\
>  	return 0;							\
>  }
> +/* copy_reference_state() */
> +COPY_STATE_FN(reference, acquired_refs, refs, 1)
>  /* copy_stack_state() */
>  COPY_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
>  #undef COPY_STATE_FN
> @@ -440,6 +497,8 @@ static int realloc_##NAME##_state(struct bpf_func_state *state, int size, \
>  	state->FIELD = new_##FIELD;					\
>  	return 0;							\
>  }
> +/* realloc_reference_state() */
> +REALLOC_STATE_FN(reference, acquired_refs, refs, 1)
>  /* realloc_stack_state() */
>  REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
>  #undef REALLOC_STATE_FN
> @@ -451,16 +510,89 @@ REALLOC_STATE_FN(stack, allocated_stack, stack, BPF_REG_SIZE)
>   * which realloc_stack_state() copies over. It points to previous
>   * bpf_verifier_state which is never reallocated.
>   */
> -static int realloc_func_state(struct bpf_func_state *state, int size,
> -			      bool copy_old)
> +static int realloc_func_state(struct bpf_func_state *state, int stack_size,
> +			      int refs_size, bool copy_old)
>  {
> -	return realloc_stack_state(state, size, copy_old);
> +	int err = realloc_reference_state(state, refs_size, copy_old);
> +	if (err)
> +		return err;
> +	return realloc_stack_state(state, stack_size, copy_old);
> +}
> +
> +/* Acquire a pointer id from the env and update the state->refs to include
> + * this new pointer reference.
> + * On success, returns a valid pointer id to associate with the register
> + * On failure, returns a negative errno.
> + */
> +static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> +{
> +	struct bpf_func_state *state = cur_func(env);
> +	int new_ofs = state->acquired_refs;
> +	int id, err;
> +
> +	err = realloc_reference_state(state, state->acquired_refs + 1, true);
> +	if (err)
> +		return err;
> +	id = ++env->id_gen;
> +	state->refs[new_ofs].id = id;
> +	state->refs[new_ofs].insn_idx = insn_idx;
> +
> +	return id;
> +}
> +
> +/* release function corresponding to acquire_reference_state(). Idempotent. */
> +static int __release_reference_state(struct bpf_func_state *state, int ptr_id)
> +{
> +	int i, last_idx;
> +
> +	if (!ptr_id)
> +		return 0;

Is this defensive programming or this condition can actually happen?
As far as I can see all callers suppose to pass valid ptr_id into it.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier
  2018-09-12 23:17   ` Alexei Starovoitov
@ 2018-09-13 19:00     ` Joe Stringer
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-13 19:00 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Stringer, daniel, netdev, ast, john fastabend, tgraf,
	Martin KaFai Lau, Nitin Hande, mauricio.vasquez

On Wed, 12 Sep 2018 at 16:17, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 11, 2018 at 05:36:35PM -0700, Joe Stringer wrote:
> > ...
> > +
> > +/* release function corresponding to acquire_reference_state(). Idempotent. */
> > +static int __release_reference_state(struct bpf_func_state *state, int ptr_id)
> > +{
> > +     int i, last_idx;
> > +
> > +     if (!ptr_id)
> > +             return 0;
>
> Is this defensive programming or this condition can actually happen?
> As far as I can see all callers suppose to pass valid ptr_id into it.
>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
>

Looks like defensive programming to me. That said, if it's being
defensive, why not return `-EFAULT`? I'll try this out locally.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (5 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-13  0:06   ` Alexei Starovoitov
                     ` (2 more replies)
  2018-09-12  0:36 ` [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking Joe Stringer
                   ` (3 subsequent siblings)
  10 siblings, 3 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

This patch adds new BPF helper functions, bpf_sk_lookup_tcp() and
bpf_sk_lookup_udp() which allows BPF programs to find out if there is a
socket listening on this host, and returns a socket pointer which the
BPF program can then access to determine, for instance, whether to
forward or drop traffic. bpf_sk_lookup_xxx() may take a reference on the
socket, so when a BPF program makes use of this function, it must
subsequently pass the returned pointer into the newly added sk_release()
to return the reference.

By way of example, the following pseudocode would filter inbound
connections at XDP if there is no corresponding service listening for
the traffic:

  struct bpf_sock_tuple tuple;
  struct bpf_sock_ops *sk;

  populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
  sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof tuple, netns, 0);
  if (!sk) {
    // Couldn't find a socket listening for this traffic. Drop.
    return TC_ACT_SHOT;
  }
  bpf_sk_release(sk, 0);
  return TC_ACT_OK;

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 include/uapi/linux/bpf.h                  |  54 +++++++-
 kernel/bpf/verifier.c                     |   8 +-
 net/core/filter.c                         | 145 ++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h            |  54 +++++++-
 tools/testing/selftests/bpf/bpf_helpers.h |  12 ++
 5 files changed, 270 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 66917a4eba27..8ed6e293113f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2141,6 +2141,41 @@ union bpf_attr {
  *		request in the skb.
  *	Return
  *		0 on success, or a negative error in case of failure.
+ *
+ * struct bpf_sock_ops *bpf_sk_lookup_tcp(ctx, tuple, tuple_size, netns, flags)
+ * 	Decription
+ * 		Look for TCP socket matching 'tuple'. The return value must
+ * 		be checked, and if non-NULL, released via bpf_sk_release().
+ * 		@ctx: pointer to ctx
+ * 		@tuple: pointer to struct bpf_sock_tuple
+ * 		@tuple_size: size of the tuple
+ * 		@netns: network namespace id
+ * 		@flags: flags value
+ * 	Return
+ * 		pointer to socket ops on success, or
+ * 		NULL in case of failure
+ *
+ * struct bpf_sock_ops *bpf_sk_lookup_udp(ctx, tuple, tuple_size, netns, flags)
+ * 	Decription
+ * 		Look for UDP socket matching 'tuple'. The return value must
+ * 		be checked, and if non-NULL, released via bpf_sk_release().
+ * 		@ctx: pointer to ctx
+ * 		@tuple: pointer to struct bpf_sock_tuple
+ * 		@tuple_size: size of the tuple
+ * 		@netns: network namespace id
+ * 		@flags: flags value
+ * 	Return
+ * 		pointer to socket ops on success, or
+ * 		NULL in case of failure
+ *
+ *  int bpf_sk_release(sock, flags)
+ * 	Description
+ * 		Release the reference held by 'sock'.
+ * 		@sock: Pointer reference to release. Must be found via
+ * 		       bpf_sk_lookup_xxx().
+ * 		@flags: flags value
+ * 	Return
+ * 		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2226,7 +2261,10 @@ union bpf_attr {
 	FN(get_current_cgroup_id),	\
 	FN(get_local_storage),		\
 	FN(sk_select_reuseport),	\
-	FN(skb_ancestor_cgroup_id),
+	FN(skb_ancestor_cgroup_id),	\
+	FN(sk_lookup_tcp),		\
+	FN(sk_lookup_udp),		\
+	FN(sk_release),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -2395,6 +2433,20 @@ struct bpf_sock {
 				 */
 };
 
+struct bpf_sock_tuple {
+	union {
+		__be32 ipv6[4];
+		__be32 ipv4;
+	} saddr;
+	union {
+		__be32 ipv6[4];
+		__be32 ipv4;
+	} daddr;
+	__be16 sport;
+	__be16 dport;
+	__u8 family;
+};
+
 #define XDP_PACKET_HEADROOM 256
 
 /* User return codes for XDP prog type.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 67c62ef67d37..37feedaaa1c3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -153,6 +153,12 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
  * PTR_TO_MAP_VALUE, PTR_TO_SOCKET_OR_NULL becomes PTR_TO_SOCKET when the type
  * passes through a NULL-check conditional. For the branch wherein the state is
  * changed to CONST_IMM, the verifier releases the reference.
+ *
+ * For each helper function that allocates a reference, such as
+ * bpf_sk_lookup_tcp(), there is a corresponding release function, such as
+ * bpf_sk_release(). When a reference type passes into the release function,
+ * the verifier also releases the reference. If any unchecked or unreleased
+ * reference remains at the end of the program, the verifier rejects it.
  */
 
 /* verifier_state + insn_idx are pushed to stack when branch is encountered */
@@ -300,7 +306,7 @@ static bool arg_type_is_refcounted(enum bpf_arg_type type)
  */
 static bool is_release_function(enum bpf_func_id func_id)
 {
-	return false;
+	return func_id == BPF_FUNC_sk_release;
 }
 
 /* string representation of 'enum bpf_reg_type' */
diff --git a/net/core/filter.c b/net/core/filter.c
index 23e6e5202138..b092cae8efd5 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -58,13 +58,17 @@
 #include <net/busy_poll.h>
 #include <net/tcp.h>
 #include <net/xfrm.h>
+#include <net/udp.h>
 #include <linux/bpf_trace.h>
 #include <net/xdp_sock.h>
 #include <linux/inetdevice.h>
+#include <net/inet_hashtables.h>
+#include <net/inet6_hashtables.h>
 #include <net/ip_fib.h>
 #include <net/flow.h>
 #include <net/arp.h>
 #include <net/ipv6.h>
+#include <net/net_namespace.h>
 #include <linux/seg6_local.h>
 #include <net/seg6.h>
 #include <net/seg6_local.h>
@@ -4811,6 +4815,135 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
 };
 #endif /* CONFIG_IPV6_SEG6_BPF */
 
+struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
+		       struct sk_buff *skb, u8 proto)
+{
+	int dif = skb->dev->ifindex;
+	bool refcounted = false;
+	struct sock *sk = NULL;
+
+	if (tuple->family == AF_INET) {
+		int sdif = inet_sdif(skb);
+
+		if (proto == IPPROTO_TCP)
+			sk = __inet_lookup(net, &tcp_hashinfo, skb, 0,
+					   tuple->saddr.ipv4, tuple->sport,
+					   tuple->daddr.ipv4, tuple->dport,
+					   dif, sdif, &refcounted);
+		else
+			sk = __udp4_lib_lookup(net,
+					       tuple->saddr.ipv4, tuple->sport,
+					       tuple->daddr.ipv4, tuple->dport,
+					       dif, sdif, &udp_table, skb);
+#if IS_ENABLED(CONFIG_IPV6)
+	} else {
+		struct in6_addr *src6 = (struct in6_addr *)&tuple->saddr.ipv6;
+		struct in6_addr *dst6 = (struct in6_addr *)&tuple->daddr.ipv6;
+		int sdif = inet6_sdif(skb);
+
+		if (proto == IPPROTO_TCP)
+			sk = __inet6_lookup(net, &tcp_hashinfo, skb, 0,
+					    src6, tuple->sport,
+					    dst6, tuple->dport,
+					    dif, sdif, &refcounted);
+		else
+			sk = __udp6_lib_lookup(net, src6, tuple->sport,
+					       dst6, tuple->dport,
+					       dif, sdif, &udp_table, skb);
+	}
+#endif
+
+	if (unlikely(sk && !refcounted && !sock_flag(sk, SOCK_RCU_FREE))) {
+		WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
+		sk = NULL;
+	}
+	return sk;
+}
+
+/* bpf_sk_lookup performs the core lookup for different types of sockets,
+ * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
+ * Returns the socket as an 'unsigned long' to simplify the casting in the
+ * callers to satisfy BPF_CALL declarations.
+ */
+static unsigned long
+bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
+	      u8 proto, u32 netns_id, u64 flags)
+{
+	struct net *caller_net = dev_net(skb->dev);
+	struct sock *sk = NULL;
+	struct net *net;
+
+	if (unlikely(len != sizeof(struct bpf_sock_tuple) || flags ||
+		     (tuple->family != AF_INET && tuple->family != AF_INET6)))
+		goto out;
+
+	if (netns_id)
+		net = get_net_ns_by_id(caller_net, netns_id);
+	else
+		net = caller_net;
+	if (unlikely(!net))
+		goto out;
+	sk = sk_lookup(net, tuple, skb, proto);
+	put_net(net);
+
+	if (sk)
+		sk = sk_to_full_sk(sk);
+out:
+	return (unsigned long) sk;
+}
+
+BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
+	   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
+{
+	return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
+	.func		= bpf_sk_lookup_tcp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
+BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
+	   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
+{
+	return bpf_sk_lookup(skb, tuple, len, IPPROTO_UDP, netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
+	.func		= bpf_sk_lookup_udp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
+BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
+{
+	if (!sock_flag(sk, SOCK_RCU_FREE))
+		sock_gen_put(sk);
+
+	if (unlikely(flags))
+		return -EINVAL;
+	return 0;
+}
+
+static const struct bpf_func_proto bpf_sk_release_proto = {
+	.func		= bpf_sk_release,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_SOCKET,
+	.arg2_type	= ARG_ANYTHING,
+};
+
 bool bpf_helper_changes_pkt_data(void *func)
 {
 	if (func == bpf_skb_vlan_push ||
@@ -5017,6 +5150,12 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_skb_ancestor_cgroup_id:
 		return &bpf_skb_ancestor_cgroup_id_proto;
 #endif
+	case BPF_FUNC_sk_lookup_tcp:
+		return &bpf_sk_lookup_tcp_proto;
+	case BPF_FUNC_sk_lookup_udp:
+		return &bpf_sk_lookup_udp_proto;
+	case BPF_FUNC_sk_release:
+		return &bpf_sk_release_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
@@ -5117,6 +5256,12 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sk_redirect_hash_proto;
 	case BPF_FUNC_get_local_storage:
 		return &bpf_get_local_storage_proto;
+	case BPF_FUNC_sk_lookup_tcp:
+		return &bpf_sk_lookup_tcp_proto;
+	case BPF_FUNC_sk_lookup_udp:
+		return &bpf_sk_lookup_udp_proto;
+	case BPF_FUNC_sk_release:
+		return &bpf_sk_release_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 66917a4eba27..8ed6e293113f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2141,6 +2141,41 @@ union bpf_attr {
  *		request in the skb.
  *	Return
  *		0 on success, or a negative error in case of failure.
+ *
+ * struct bpf_sock_ops *bpf_sk_lookup_tcp(ctx, tuple, tuple_size, netns, flags)
+ * 	Decription
+ * 		Look for TCP socket matching 'tuple'. The return value must
+ * 		be checked, and if non-NULL, released via bpf_sk_release().
+ * 		@ctx: pointer to ctx
+ * 		@tuple: pointer to struct bpf_sock_tuple
+ * 		@tuple_size: size of the tuple
+ * 		@netns: network namespace id
+ * 		@flags: flags value
+ * 	Return
+ * 		pointer to socket ops on success, or
+ * 		NULL in case of failure
+ *
+ * struct bpf_sock_ops *bpf_sk_lookup_udp(ctx, tuple, tuple_size, netns, flags)
+ * 	Decription
+ * 		Look for UDP socket matching 'tuple'. The return value must
+ * 		be checked, and if non-NULL, released via bpf_sk_release().
+ * 		@ctx: pointer to ctx
+ * 		@tuple: pointer to struct bpf_sock_tuple
+ * 		@tuple_size: size of the tuple
+ * 		@netns: network namespace id
+ * 		@flags: flags value
+ * 	Return
+ * 		pointer to socket ops on success, or
+ * 		NULL in case of failure
+ *
+ *  int bpf_sk_release(sock, flags)
+ * 	Description
+ * 		Release the reference held by 'sock'.
+ * 		@sock: Pointer reference to release. Must be found via
+ * 		       bpf_sk_lookup_xxx().
+ * 		@flags: flags value
+ * 	Return
+ * 		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2226,7 +2261,10 @@ union bpf_attr {
 	FN(get_current_cgroup_id),	\
 	FN(get_local_storage),		\
 	FN(sk_select_reuseport),	\
-	FN(skb_ancestor_cgroup_id),
+	FN(skb_ancestor_cgroup_id),	\
+	FN(sk_lookup_tcp),		\
+	FN(sk_lookup_udp),		\
+	FN(sk_release),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -2395,6 +2433,20 @@ struct bpf_sock {
 				 */
 };
 
+struct bpf_sock_tuple {
+	union {
+		__be32 ipv6[4];
+		__be32 ipv4;
+	} saddr;
+	union {
+		__be32 ipv6[4];
+		__be32 ipv4;
+	} daddr;
+	__be16 sport;
+	__be16 dport;
+	__u8 family;
+};
+
 #define XDP_PACKET_HEADROOM 256
 
 /* User return codes for XDP prog type.
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index e4be7730222d..88ce00c3aa0f 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -143,6 +143,18 @@ static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
 	(void *) BPF_FUNC_skb_cgroup_id;
 static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
 	(void *) BPF_FUNC_skb_ancestor_cgroup_id;
+static struct bpf_sock *(*bpf_sk_lookup_tcp)(void *ctx,
+					     struct bpf_sock_tuple *tuple,
+					     int size, unsigned int netns_id,
+					     unsigned long long flags) =
+	(void *) BPF_FUNC_sk_lookup_tcp;
+static struct bpf_sock *(*bpf_sk_lookup_udp)(void *ctx,
+					     struct bpf_sock_tuple *tuple,
+					     int size, unsigned int netns_id,
+					     unsigned long long flags) =
+	(void *) BPF_FUNC_sk_lookup_udp;
+static int (*bpf_sk_release)(struct bpf_sock *sk, unsigned long long flags) =
+	(void *) BPF_FUNC_sk_release;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
  2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
@ 2018-09-13  0:06   ` Alexei Starovoitov
  2018-09-14  6:57   ` kbuild test robot
  2018-09-14  7:11   ` kbuild test robot
  2 siblings, 0 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-13  0:06 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:36PM -0700, Joe Stringer wrote:
> This patch adds new BPF helper functions, bpf_sk_lookup_tcp() and
> bpf_sk_lookup_udp() which allows BPF programs to find out if there is a
> socket listening on this host, and returns a socket pointer which the
> BPF program can then access to determine, for instance, whether to
> forward or drop traffic. bpf_sk_lookup_xxx() may take a reference on the
> socket, so when a BPF program makes use of this function, it must
> subsequently pass the returned pointer into the newly added sk_release()
> to return the reference.
> 
> By way of example, the following pseudocode would filter inbound
> connections at XDP if there is no corresponding service listening for
> the traffic:
> 
>   struct bpf_sock_tuple tuple;
>   struct bpf_sock_ops *sk;
> 
>   populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
>   sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof tuple, netns, 0);
...
> +struct bpf_sock_tuple {
> +	union {
> +		__be32 ipv6[4];
> +		__be32 ipv4;
> +	} saddr;
> +	union {
> +		__be32 ipv6[4];
> +		__be32 ipv4;
> +	} daddr;
> +	__be16 sport;
> +	__be16 dport;
> +	__u8 family;
> +};

since we can pass ptr_to_packet into map lookup and other helpers now,
can you move 'family' out of bpf_sock_tuple and combine with netns_id arg?
then progs wouldn't need to copy bytes from the packet into tuple
to do a lookup.
The rest looks great to me.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
  2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
  2018-09-13  0:06   ` Alexei Starovoitov
@ 2018-09-14  6:57   ` kbuild test robot
  2018-09-14  7:11   ` kbuild test robot
  2 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-09-14  6:57 UTC (permalink / raw)
  To: Joe Stringer
  Cc: kbuild-all, daniel, netdev, ast, john.fastabend, tgraf, kafai,
	nitin.hande, mauricio.vasquez

[-- Attachment #1: Type: text/plain, Size: 19595 bytes --]

Hi Joe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Joe-Stringer/Add-socket-lookup-support/20180914-134632
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: xtensa-common_defconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   net/core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_release'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^~~~
   include/linux/filter.h:443:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_2(name, ...) BPF_CALL_x(2, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net/core/filter.c:4941:11: error: initializer element is not constant
     .func  = bpf_sk_release,
              ^~~~~~~~~~~~~~
   net/core/filter.c:4941:11: note: (near initialization for 'bpf_sk_release_proto.func')
   net/core/filter.c:4980:1: error: invalid storage class for function 'bpf_base_func_proto'
    bpf_base_func_proto(enum bpf_func_id func_id)
    ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5009:1: error: invalid storage class for function 'sock_filter_func_proto'
    sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5025:1: error: invalid storage class for function 'sock_addr_func_proto'
    sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5051:1: error: invalid storage class for function 'sk_filter_func_proto'
    sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5068:1: error: invalid storage class for function 'cg_skb_func_proto'
    cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:5079:1: error: invalid storage class for function 'tc_cls_act_func_proto'
    tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5166:1: error: invalid storage class for function 'xdp_func_proto'
    xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~
   net/core/filter.c:5193:1: error: invalid storage class for function 'sock_ops_func_proto'
    sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5216:1: error: invalid storage class for function 'sk_msg_func_proto'
    sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:5237:1: error: invalid storage class for function 'sk_skb_func_proto'
    sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:5272:1: error: invalid storage class for function 'lwt_out_func_proto'
    lwt_out_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~
   net/core/filter.c:5299:1: error: invalid storage class for function 'lwt_in_func_proto'
    lwt_in_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:5310:1: error: invalid storage class for function 'lwt_xmit_func_proto'
    lwt_xmit_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5345:1: error: invalid storage class for function 'lwt_seg6local_func_proto'
    lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5361:13: error: invalid storage class for function 'bpf_skb_is_valid_access'
    static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type,
                ^~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5404:13: error: invalid storage class for function 'sk_filter_is_valid_access'
    static bool sk_filter_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5430:13: error: invalid storage class for function 'lwt_is_valid_access'
    static bool lwt_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5466:13: error: invalid storage class for function '__sock_filter_check_attach_type'
    static bool __sock_filter_check_attach_type(int off,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5509:13: error: invalid storage class for function '__sock_filter_check_size'
    static bool __sock_filter_check_size(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5536:13: error: invalid storage class for function 'sock_filter_is_valid_access'
    static bool sock_filter_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5547:12: error: invalid storage class for function 'bpf_unclone_prologue'
    static int bpf_unclone_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5586:12: error: invalid storage class for function 'bpf_gen_ld_abs'
    static int bpf_gen_ld_abs(const struct bpf_insn *orig,
               ^~~~~~~~~~~~~~
   net/core/filter.c:5621:12: error: invalid storage class for function 'tc_cls_act_prologue'
    static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5627:13: error: invalid storage class for function 'tc_cls_act_is_valid_access'
    static bool tc_cls_act_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5662:13: error: invalid storage class for function '__is_valid_xdp_access'
    static bool __is_valid_xdp_access(int off, int size)
                ^~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5674:13: error: invalid storage class for function 'xdp_is_valid_access'
    static bool xdp_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/core/filter.c:24:
>> net/core/filter.c:5712:19: error: non-static declaration of 'bpf_warn_invalid_xdp_action' follows static declaration
    EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/export.h:79:21: note: in definition of macro '___EXPORT_SYMBOL'
     extern typeof(sym) sym;      \
                        ^~~
   net/core/filter.c:5712:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
    EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:5704:6: note: previous definition of 'bpf_warn_invalid_xdp_action' was here
    void bpf_warn_invalid_xdp_action(u32 act)
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5714:13: error: invalid storage class for function 'sock_addr_is_valid_access'
    static bool sock_addr_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5801:13: error: invalid storage class for function 'sock_ops_is_valid_access'
    static bool sock_ops_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5842:12: error: invalid storage class for function 'sk_skb_prologue'
    static int sk_skb_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~
   net/core/filter.c:5848:13: error: invalid storage class for function 'sk_skb_is_valid_access'
    static bool sk_skb_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5883:13: error: invalid storage class for function 'sk_msg_is_valid_access'
    static bool sk_msg_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:5915:12: error: invalid storage class for function 'bpf_convert_ctx_access'
    static u32 bpf_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6318:12: error: invalid storage class for function 'tc_cls_act_convert_ctx_access'
    static u32 tc_cls_act_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6342:12: error: invalid storage class for function 'xdp_convert_ctx_access'
    static u32 xdp_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6462:12: error: invalid storage class for function 'sock_addr_convert_ctx_access'
    static u32 sock_addr_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6551:12: error: invalid storage class for function 'sock_ops_convert_ctx_access'
    static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6890:12: error: invalid storage class for function 'sk_skb_convert_ctx_access'
    static u32 sk_skb_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:6915:12: error: invalid storage class for function 'sk_msg_convert_ctx_access'
    static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/kernel.h:7,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/core/filter.c:24:
>> net/core/filter.c:7190:19: error: non-static declaration of 'sk_detach_filter' follows static declaration
    EXPORT_SYMBOL_GPL(sk_detach_filter);
                      ^~~~~~~~~~~~~~~~
   include/linux/export.h:79:21: note: in definition of macro '___EXPORT_SYMBOL'
     extern typeof(sym) sym;      \
                        ^~~
   net/core/filter.c:7190:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
    EXPORT_SYMBOL_GPL(sk_detach_filter);
    ^~~~~~~~~~~~~~~~~
   net/core/filter.c:7172:5: note: previous definition of 'sk_detach_filter' was here
    int sk_detach_filter(struct sock *sk)
        ^~~~~~~~~~~~~~~~
   net/core/filter.c:7247:13: error: invalid storage class for function 'bpf_init_reuseport_kern'
    static void bpf_init_reuseport_kern(struct sk_reuseport_kern *reuse_kern,
                ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/net/sock.h:64,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
   include/linux/filter.h:432:6: error: invalid storage class for function '____sk_select_reuseport'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^~~~
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net/core/filter.c:7277:12: error: static declaration of 'sk_select_reuseport' follows non-static declaration
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
               ^~~~~~~~~~~~~~~~~~~
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net/core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net/core/filter.c:7277:12: note: previous declaration of 'sk_select_reuseport' was here
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
               ^~~~~~~~~~~~~~~~~~~
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net/core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_select_reuseport':
   include/linux/filter.h:436:10: error: implicit declaration of function '____sk_select_reuseport'; did you mean 'sk_select_reuseport'? [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^~~~
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____sk_select_reuseport'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^~~~
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net/core/filter.c:7323:20: error: initializer element is not constant
     .func           = sk_select_reuseport,
                       ^~~~~~~~~~~~~~~~~~~
   net/core/filter.c:7323:20: note: (near initialization for 'sk_select_reuseport_proto.func')
   In file included from include/net/sock.h:64,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
   include/linux/filter.h:432:6: error: invalid storage class for function '____sk_reuseport_load_bytes'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^~~~
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net/core/filter.c:7332:12: error: static declaration of 'sk_reuseport_load_bytes' follows non-static declaration
    BPF_CALL_4(sk_reuseport_load_bytes,
               ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net/core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net/core/filter.c:7332:12: note: previous declaration of 'sk_reuseport_load_bytes' was here
    BPF_CALL_4(sk_reuseport_load_bytes,
               ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net/core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_reuseport_load_bytes':
   include/linux/filter.h:436:10: error: implicit declaration of function '____sk_reuseport_load_bytes'; did you mean 'sk_reuseport_load_bytes'? [-Werror=implicit-function-declaration]

vim +/bpf_warn_invalid_xdp_action +5712 net/core/filter.c

6a773a15a Brenden Blanco  2016-07-19  5673  
6a773a15a Brenden Blanco  2016-07-19 @5674  static bool xdp_is_valid_access(int off, int size,
6a773a15a Brenden Blanco  2016-07-19  5675  				enum bpf_access_type type,
5e43f899b Andrey Ignatov  2018-03-30  5676  				const struct bpf_prog *prog,
239946314 Yonghong Song   2017-06-22  5677  				struct bpf_insn_access_aux *info)
6a773a15a Brenden Blanco  2016-07-19  5678  {
0d8300325 Jakub Kicinski  2018-05-08  5679  	if (type == BPF_WRITE) {
0d8300325 Jakub Kicinski  2018-05-08  5680  		if (bpf_prog_is_dev_bound(prog->aux)) {
0d8300325 Jakub Kicinski  2018-05-08  5681  			switch (off) {
0d8300325 Jakub Kicinski  2018-05-08  5682  			case offsetof(struct xdp_md, rx_queue_index):
0d8300325 Jakub Kicinski  2018-05-08  5683  				return __is_valid_xdp_access(off, size);
0d8300325 Jakub Kicinski  2018-05-08  5684  			}
0d8300325 Jakub Kicinski  2018-05-08  5685  		}
6a773a15a Brenden Blanco  2016-07-19  5686  		return false;
0d8300325 Jakub Kicinski  2018-05-08  5687  	}
6a773a15a Brenden Blanco  2016-07-19  5688  
6a773a15a Brenden Blanco  2016-07-19  5689  	switch (off) {
6a773a15a Brenden Blanco  2016-07-19  5690  	case offsetof(struct xdp_md, data):
239946314 Yonghong Song   2017-06-22  5691  		info->reg_type = PTR_TO_PACKET;
6a773a15a Brenden Blanco  2016-07-19  5692  		break;
de8f3a83b Daniel Borkmann 2017-09-25  5693  	case offsetof(struct xdp_md, data_meta):
de8f3a83b Daniel Borkmann 2017-09-25  5694  		info->reg_type = PTR_TO_PACKET_META;
de8f3a83b Daniel Borkmann 2017-09-25  5695  		break;
6a773a15a Brenden Blanco  2016-07-19  5696  	case offsetof(struct xdp_md, data_end):
239946314 Yonghong Song   2017-06-22  5697  		info->reg_type = PTR_TO_PACKET_END;
6a773a15a Brenden Blanco  2016-07-19  5698  		break;
6a773a15a Brenden Blanco  2016-07-19  5699  	}
6a773a15a Brenden Blanco  2016-07-19  5700  
1afaf661b Daniel Borkmann 2016-12-04  5701  	return __is_valid_xdp_access(off, size);
6a773a15a Brenden Blanco  2016-07-19  5702  }
6a773a15a Brenden Blanco  2016-07-19  5703  
6a773a15a Brenden Blanco  2016-07-19  5704  void bpf_warn_invalid_xdp_action(u32 act)
6a773a15a Brenden Blanco  2016-07-19  5705  {
9beb8bedb Daniel Borkmann 2017-09-09  5706  	const u32 act_max = XDP_REDIRECT;
9beb8bedb Daniel Borkmann 2017-09-09  5707  
9beb8bedb Daniel Borkmann 2017-09-09  5708  	WARN_ONCE(1, "%s XDP return value %u, expect packet loss!\n",
9beb8bedb Daniel Borkmann 2017-09-09  5709  		  act > act_max ? "Illegal" : "Driver unsupported",
9beb8bedb Daniel Borkmann 2017-09-09  5710  		  act);
6a773a15a Brenden Blanco  2016-07-19  5711  }
6a773a15a Brenden Blanco  2016-07-19 @5712  EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
6a773a15a Brenden Blanco  2016-07-19  5713  

:::::: The code at line 5712 was first introduced by commit
:::::: 6a773a15a1e8874e5eccd2f29190c31085912c95 bpf: add XDP prog type for early driver filter

:::::: TO: Brenden Blanco <bblanco@plumgrid.com>
:::::: CC: David S. Miller <davem@davemloft.net>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10358 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
  2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
  2018-09-13  0:06   ` Alexei Starovoitov
  2018-09-14  6:57   ` kbuild test robot
@ 2018-09-14  7:11   ` kbuild test robot
  2 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-09-14  7:11 UTC (permalink / raw)
  To: Joe Stringer
  Cc: kbuild-all, daniel, netdev, ast, john.fastabend, tgraf, kafai,
	nitin.hande, mauricio.vasquez

[-- Attachment #1: Type: text/plain, Size: 41175 bytes --]

Hi Joe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Joe-Stringer/Add-socket-lookup-support/20180914-134632
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-s0-09141346 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   net/core/filter.c: In function 'sk_lookup':
>> net/core/filter.c:4870:1: error: invalid storage class for function 'bpf_sk_lookup'
    bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
    ^~~~~~~~~~~~~
>> net/core/filter.c:4869:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static unsigned long
    ^~~~~~
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____bpf_sk_lookup_tcp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
>> net/core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
>> net/core/filter.c:4896:12: error: static declaration of 'bpf_sk_lookup_tcp' follows non-static declaration
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
>> net/core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c:4896:12: note: previous declaration of 'bpf_sk_lookup_tcp' was here
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
>> net/core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c: In function 'bpf_sk_lookup_tcp':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____bpf_sk_lookup_tcp' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
>> net/core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_lookup_tcp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
>> net/core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
>> net/core/filter.c:4903:11: error: initializer element is not constant
     .func  = bpf_sk_lookup_tcp,
              ^~~~~~~~~~~~~~~~~
   net/core/filter.c:4903:11: note: (near initialization for 'bpf_sk_lookup_tcp_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____bpf_sk_lookup_udp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
>> net/core/filter.c:4913:12: error: static declaration of 'bpf_sk_lookup_udp' follows non-static declaration
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net/core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c:4913:12: note: previous declaration of 'bpf_sk_lookup_udp' was here
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net/core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c: In function 'bpf_sk_lookup_udp':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____bpf_sk_lookup_udp' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_lookup_udp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net/core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net/core/filter.c:4920:11: error: initializer element is not constant
     .func  = bpf_sk_lookup_udp,
              ^~~~~~~~~~~~~~~~~
   net/core/filter.c:4920:11: note: (near initialization for 'bpf_sk_lookup_udp_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
--
   net//core/filter.c: In function 'sk_lookup':
   net//core/filter.c:4870:1: error: invalid storage class for function 'bpf_sk_lookup'
    bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
    ^~~~~~~~~~~~~
   net//core/filter.c:4869:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static unsigned long
    ^~~~~~
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____bpf_sk_lookup_tcp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4896:12: error: static declaration of 'bpf_sk_lookup_tcp' follows non-static declaration
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4896:12: note: previous declaration of 'bpf_sk_lookup_tcp' was here
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c: In function 'bpf_sk_lookup_tcp':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____bpf_sk_lookup_tcp' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_lookup_tcp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4896:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4903:11: error: initializer element is not constant
     .func  = bpf_sk_lookup_tcp,
              ^~~~~~~~~~~~~~~~~
   net//core/filter.c:4903:11: note: (near initialization for 'bpf_sk_lookup_tcp_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____bpf_sk_lookup_udp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4913:12: error: static declaration of 'bpf_sk_lookup_udp' follows non-static declaration
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4913:12: note: previous declaration of 'bpf_sk_lookup_udp' was here
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c: In function 'bpf_sk_lookup_udp':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____bpf_sk_lookup_udp' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_lookup_udp'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4913:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
    ^~~~~~~~~~
   net//core/filter.c:4920:11: error: initializer element is not constant
     .func  = bpf_sk_lookup_udp,
              ^~~~~~~~~~~~~~~~~
   net//core/filter.c:4920:11: note: (near initialization for 'bpf_sk_lookup_udp_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____bpf_sk_release'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
   include/linux/filter.h:443:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_2(name, ...) BPF_CALL_x(2, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net//core/filter.c:4930:12: error: static declaration of 'bpf_sk_release' follows non-static declaration
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net//core/filter.c:4930:12: note: previous declaration of 'bpf_sk_release' was here
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net//core/filter.c: In function 'bpf_sk_release':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____bpf_sk_release' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
   include/linux/filter.h:443:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_2(name, ...) BPF_CALL_x(2, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____bpf_sk_release'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
   include/linux/filter.h:443:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_2(name, ...) BPF_CALL_x(2, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:4930:1: note: in expansion of macro 'BPF_CALL_2'
    BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
    ^~~~~~~~~~
   net//core/filter.c:4941:11: error: initializer element is not constant
     .func  = bpf_sk_release,
              ^~~~~~~~~~~~~~
   net//core/filter.c:4941:11: note: (near initialization for 'bpf_sk_release_proto.func')
   net//core/filter.c:4980:1: error: invalid storage class for function 'bpf_base_func_proto'
    bpf_base_func_proto(enum bpf_func_id func_id)
    ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5009:1: error: invalid storage class for function 'sock_filter_func_proto'
    sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5025:1: error: invalid storage class for function 'sock_addr_func_proto'
    sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5051:1: error: invalid storage class for function 'sk_filter_func_proto'
    sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5068:1: error: invalid storage class for function 'cg_skb_func_proto'
    cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net//core/filter.c:5079:1: error: invalid storage class for function 'tc_cls_act_func_proto'
    tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5166:1: error: invalid storage class for function 'xdp_func_proto'
    xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~
   net//core/filter.c:5193:1: error: invalid storage class for function 'sock_ops_func_proto'
    sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5216:1: error: invalid storage class for function 'sk_msg_func_proto'
    sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net//core/filter.c:5237:1: error: invalid storage class for function 'sk_skb_func_proto'
    sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net//core/filter.c:5272:1: error: invalid storage class for function 'lwt_out_func_proto'
    lwt_out_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~
   net//core/filter.c:5299:1: error: invalid storage class for function 'lwt_in_func_proto'
    lwt_in_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~
   net//core/filter.c:5310:1: error: invalid storage class for function 'lwt_xmit_func_proto'
    lwt_xmit_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5345:1: error: invalid storage class for function 'lwt_seg6local_func_proto'
    lwt_seg6local_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    ^~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5361:13: error: invalid storage class for function 'bpf_skb_is_valid_access'
    static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type,
                ^~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5404:13: error: invalid storage class for function 'sk_filter_is_valid_access'
    static bool sk_filter_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5430:13: error: invalid storage class for function 'lwt_is_valid_access'
    static bool lwt_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5466:13: error: invalid storage class for function '__sock_filter_check_attach_type'
    static bool __sock_filter_check_attach_type(int off,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5509:13: error: invalid storage class for function '__sock_filter_check_size'
    static bool __sock_filter_check_size(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5536:13: error: invalid storage class for function 'sock_filter_is_valid_access'
    static bool sock_filter_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5547:12: error: invalid storage class for function 'bpf_unclone_prologue'
    static int bpf_unclone_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5586:12: error: invalid storage class for function 'bpf_gen_ld_abs'
    static int bpf_gen_ld_abs(const struct bpf_insn *orig,
               ^~~~~~~~~~~~~~
   net//core/filter.c:5621:12: error: invalid storage class for function 'tc_cls_act_prologue'
    static int tc_cls_act_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5627:13: error: invalid storage class for function 'tc_cls_act_is_valid_access'
    static bool tc_cls_act_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5662:13: error: invalid storage class for function '__is_valid_xdp_access'
    static bool __is_valid_xdp_access(int off, int size)
                ^~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5674:13: error: invalid storage class for function 'xdp_is_valid_access'
    static bool xdp_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5714:13: error: invalid storage class for function 'sock_addr_is_valid_access'
    static bool sock_addr_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5714:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    static bool sock_addr_is_valid_access(int off, int size,
    ^~~~~~
   net//core/filter.c:5801:13: error: invalid storage class for function 'sock_ops_is_valid_access'
    static bool sock_ops_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5842:12: error: invalid storage class for function 'sk_skb_prologue'
    static int sk_skb_prologue(struct bpf_insn *insn_buf, bool direct_write,
               ^~~~~~~~~~~~~~~
   net//core/filter.c:5848:13: error: invalid storage class for function 'sk_skb_is_valid_access'
    static bool sk_skb_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5883:13: error: invalid storage class for function 'sk_msg_is_valid_access'
    static bool sk_msg_is_valid_access(int off, int size,
                ^~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:5915:12: error: invalid storage class for function 'bpf_convert_ctx_access'
    static u32 bpf_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6318:12: error: invalid storage class for function 'tc_cls_act_convert_ctx_access'
    static u32 tc_cls_act_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6342:12: error: invalid storage class for function 'xdp_convert_ctx_access'
    static u32 xdp_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6462:12: error: invalid storage class for function 'sock_addr_convert_ctx_access'
    static u32 sock_addr_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6551:12: error: invalid storage class for function 'sock_ops_convert_ctx_access'
    static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6890:12: error: invalid storage class for function 'sk_skb_convert_ctx_access'
    static u32 sk_skb_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:6915:12: error: invalid storage class for function 'sk_msg_convert_ctx_access'
    static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:7192:1: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    int sk_get_filter(struct sock *sk, struct sock_filter __user *ubuf,
    ^~~
   net//core/filter.c:7247:13: error: invalid storage class for function 'bpf_init_reuseport_kern'
    static void bpf_init_reuseport_kern(struct sk_reuseport_kern *reuse_kern,
                ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____sk_select_reuseport'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net//core/filter.c:7277:12: error: static declaration of 'sk_select_reuseport' follows non-static declaration
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net//core/filter.c:7277:12: note: previous declaration of 'sk_select_reuseport' was here
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_select_reuseport':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____sk_select_reuseport' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____sk_select_reuseport'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7277:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
    ^~~~~~~~~~
   net//core/filter.c:7323:20: error: initializer element is not constant
     .func           = sk_select_reuseport,
                       ^~~~~~~~~~~~~~~~~~~
   net//core/filter.c:7323:20: note: (near initialization for 'sk_select_reuseport_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____sk_reuseport_load_bytes'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net//core/filter.c:7332:12: error: static declaration of 'sk_reuseport_load_bytes' follows non-static declaration
    BPF_CALL_4(sk_reuseport_load_bytes,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net//core/filter.c:7332:12: note: previous declaration of 'sk_reuseport_load_bytes' was here
    BPF_CALL_4(sk_reuseport_load_bytes,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_reuseport_load_bytes':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____sk_reuseport_load_bytes' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____sk_reuseport_load_bytes'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^
   include/linux/filter.h:445:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_4(name, ...) BPF_CALL_x(4, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7332:1: note: in expansion of macro 'BPF_CALL_4'
    BPF_CALL_4(sk_reuseport_load_bytes,
    ^~~~~~~~~~
   net//core/filter.c:7340:11: error: initializer element is not constant
     .func  = sk_reuseport_load_bytes,
              ^~~~~~~~~~~~~~~~~~~~~~~
   net//core/filter.c:7340:11: note: (near initialization for 'sk_reuseport_load_bytes_proto.func')
   In file included from include/net/sock.h:64:0,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
>> include/linux/filter.h:432:6: error: invalid storage class for function '____sk_reuseport_load_bytes_relative'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
         ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7349:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
    ^~~~~~~~~~
   net//core/filter.c:7349:12: error: static declaration of 'sk_reuseport_load_bytes_relative' follows non-static declaration
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
               ^
   include/linux/filter.h:434:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))        \
         ^~~~
   net//core/filter.c:7349:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
    ^~~~~~~~~~
   net//core/filter.c:7349:12: note: previous declaration of 'sk_reuseport_load_bytes_relative' was here
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
               ^
   include/linux/filter.h:433:6: note: in definition of macro 'BPF_CALL_x'
     u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));        \
         ^~~~
   net//core/filter.c:7349:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_reuseport_load_bytes_relative':
>> include/linux/filter.h:436:10: error: implicit declaration of function '____sk_reuseport_load_bytes_relative' [-Werror=implicit-function-declaration]
      return ____##name(__BPF_MAP(x,__BPF_CAST,__BPF_N,__VA_ARGS__));\
             ^
>> include/linux/filter.h:446:31: note: in expansion of macro 'BPF_CALL_x'
    #define BPF_CALL_5(name, ...) BPF_CALL_x(5, name, __VA_ARGS__)
                                  ^~~~~~~~~~
   net//core/filter.c:7349:1: note: in expansion of macro 'BPF_CALL_5'
    BPF_CALL_5(sk_reuseport_load_bytes_relative,
    ^~~~~~~~~~
   net//core/filter.c: In function 'sk_lookup':
   include/linux/filter.h:439:6: error: invalid storage class for function '____sk_reuseport_load_bytes_relative'
     u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
         ^

vim +/bpf_sk_lookup +4870 net/core/filter.c

  4863	
  4864	/* bpf_sk_lookup performs the core lookup for different types of sockets,
  4865	 * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
  4866	 * Returns the socket as an 'unsigned long' to simplify the casting in the
  4867	 * callers to satisfy BPF_CALL declarations.
  4868	 */
> 4869	static unsigned long
> 4870	bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
  4871		      u8 proto, u32 netns_id, u64 flags)
  4872	{
  4873		struct net *caller_net = dev_net(skb->dev);
  4874		struct sock *sk = NULL;
  4875		struct net *net;
  4876	
  4877		if (unlikely(len != sizeof(struct bpf_sock_tuple) || flags ||
  4878			     (tuple->family != AF_INET && tuple->family != AF_INET6)))
  4879			goto out;
  4880	
  4881		if (netns_id)
  4882			net = get_net_ns_by_id(caller_net, netns_id);
  4883		else
  4884			net = caller_net;
  4885		if (unlikely(!net))
  4886			goto out;
  4887		sk = sk_lookup(net, tuple, skb, proto);
  4888		put_net(net);
  4889	
  4890		if (sk)
  4891			sk = sk_to_full_sk(sk);
  4892	out:
  4893		return (unsigned long) sk;
  4894	}
  4895	
> 4896	BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
  4897		   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
  4898	{
  4899		return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
  4900	}
  4901	
  4902	static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
> 4903		.func		= bpf_sk_lookup_tcp,
  4904		.gpl_only	= false,
  4905		.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
  4906		.arg1_type	= ARG_PTR_TO_CTX,
  4907		.arg2_type	= ARG_PTR_TO_MEM,
  4908		.arg3_type	= ARG_CONST_SIZE,
  4909		.arg4_type	= ARG_ANYTHING,
  4910		.arg5_type	= ARG_ANYTHING,
  4911	};
  4912	
> 4913	BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
  4914		   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
  4915	{
  4916		return bpf_sk_lookup(skb, tuple, len, IPPROTO_UDP, netns_id, flags);
  4917	}
  4918	
  4919	static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
  4920		.func		= bpf_sk_lookup_udp,
  4921		.gpl_only	= false,
  4922		.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
  4923		.arg1_type	= ARG_PTR_TO_CTX,
  4924		.arg2_type	= ARG_PTR_TO_MEM,
  4925		.arg3_type	= ARG_CONST_SIZE,
  4926		.arg4_type	= ARG_ANYTHING,
  4927		.arg5_type	= ARG_ANYTHING,
  4928	};
  4929	
> 4930	BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
  4931	{
  4932		if (!sock_flag(sk, SOCK_RCU_FREE))
  4933			sock_gen_put(sk);
  4934	
  4935		if (unlikely(flags))
  4936			return -EINVAL;
  4937		return 0;
  4938	}
  4939	
  4940	static const struct bpf_func_proto bpf_sk_release_proto = {
> 4941		.func		= bpf_sk_release,
  4942		.gpl_only	= false,
  4943		.ret_type	= RET_INTEGER,
  4944		.arg1_type	= ARG_PTR_TO_SOCKET,
  4945		.arg2_type	= ARG_ANYTHING,
  4946	};
  4947	
  4948	bool bpf_helper_changes_pkt_data(void *func)
  4949	{
  4950		if (func == bpf_skb_vlan_push ||
  4951		    func == bpf_skb_vlan_pop ||
  4952		    func == bpf_skb_store_bytes ||
  4953		    func == bpf_skb_change_proto ||
  4954		    func == bpf_skb_change_head ||
  4955		    func == sk_skb_change_head ||
  4956		    func == bpf_skb_change_tail ||
  4957		    func == sk_skb_change_tail ||
  4958		    func == bpf_skb_adjust_room ||
  4959		    func == bpf_skb_pull_data ||
  4960		    func == sk_skb_pull_data ||
  4961		    func == bpf_clone_redirect ||
  4962		    func == bpf_l3_csum_replace ||
  4963		    func == bpf_l4_csum_replace ||
  4964		    func == bpf_xdp_adjust_head ||
  4965		    func == bpf_xdp_adjust_meta ||
  4966		    func == bpf_msg_pull_data ||
  4967		    func == bpf_xdp_adjust_tail ||
  4968	#if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
  4969		    func == bpf_lwt_seg6_store_bytes ||
  4970		    func == bpf_lwt_seg6_adjust_srh ||
  4971		    func == bpf_lwt_seg6_action ||
  4972	#endif
  4973		    func == bpf_lwt_push_encap)
  4974			return true;
  4975	
  4976		return false;
  4977	}
  4978	
  4979	static const struct bpf_func_proto *
> 4980	bpf_base_func_proto(enum bpf_func_id func_id)
  4981	{
  4982		switch (func_id) {
  4983		case BPF_FUNC_map_lookup_elem:
  4984			return &bpf_map_lookup_elem_proto;
  4985		case BPF_FUNC_map_update_elem:
  4986			return &bpf_map_update_elem_proto;
  4987		case BPF_FUNC_map_delete_elem:
  4988			return &bpf_map_delete_elem_proto;
  4989		case BPF_FUNC_get_prandom_u32:
  4990			return &bpf_get_prandom_u32_proto;
  4991		case BPF_FUNC_get_smp_processor_id:
  4992			return &bpf_get_raw_smp_processor_id_proto;
  4993		case BPF_FUNC_get_numa_node_id:
  4994			return &bpf_get_numa_node_id_proto;
  4995		case BPF_FUNC_tail_call:
  4996			return &bpf_tail_call_proto;
  4997		case BPF_FUNC_ktime_get_ns:
  4998			return &bpf_ktime_get_ns_proto;
  4999		case BPF_FUNC_trace_printk:
  5000			if (capable(CAP_SYS_ADMIN))
  5001				return bpf_get_trace_printk_proto();
  5002			/* else: fall through */
  5003		default:
  5004			return NULL;
  5005		}
  5006	}
  5007	
  5008	static const struct bpf_func_proto *
> 5009	sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  5010	{
  5011		switch (func_id) {
  5012		/* inet and inet6 sockets are created in a process
  5013		 * context so there is always a valid uid/gid
  5014		 */
  5015		case BPF_FUNC_get_current_uid_gid:
  5016			return &bpf_get_current_uid_gid_proto;
  5017		case BPF_FUNC_get_local_storage:
  5018			return &bpf_get_local_storage_proto;
  5019		default:
  5020			return bpf_base_func_proto(func_id);
  5021		}
  5022	}
  5023	
  5024	static const struct bpf_func_proto *
> 5025	sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  5026	{
  5027		switch (func_id) {
  5028		/* inet and inet6 sockets are created in a process
  5029		 * context so there is always a valid uid/gid
  5030		 */
  5031		case BPF_FUNC_get_current_uid_gid:
  5032			return &bpf_get_current_uid_gid_proto;
  5033		case BPF_FUNC_bind:
  5034			switch (prog->expected_attach_type) {
  5035			case BPF_CGROUP_INET4_CONNECT:
  5036			case BPF_CGROUP_INET6_CONNECT:
  5037				return &bpf_bind_proto;
  5038			default:
  5039				return NULL;
  5040			}
  5041		case BPF_FUNC_get_socket_cookie:
  5042			return &bpf_get_socket_cookie_sock_addr_proto;
  5043		case BPF_FUNC_get_local_storage:
  5044			return &bpf_get_local_storage_proto;
  5045		default:
  5046			return bpf_base_func_proto(func_id);
  5047		}
  5048	}
  5049	
  5050	static const struct bpf_func_proto *
> 5051	sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  5052	{
  5053		switch (func_id) {
  5054		case BPF_FUNC_skb_load_bytes:
  5055			return &bpf_skb_load_bytes_proto;
  5056		case BPF_FUNC_skb_load_bytes_relative:
  5057			return &bpf_skb_load_bytes_relative_proto;
  5058		case BPF_FUNC_get_socket_cookie:
  5059			return &bpf_get_socket_cookie_proto;
  5060		case BPF_FUNC_get_socket_uid:
  5061			return &bpf_get_socket_uid_proto;
  5062		default:
  5063			return bpf_base_func_proto(func_id);
  5064		}
  5065	}
  5066	
  5067	static const struct bpf_func_proto *
> 5068	cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  5069	{
  5070		switch (func_id) {
  5071		case BPF_FUNC_get_local_storage:
  5072			return &bpf_get_local_storage_proto;
  5073		default:
  5074			return sk_filter_func_proto(func_id, prog);
  5075		}
  5076	}
  5077	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26893 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (6 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-13  0:09   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 09/11] libbpf: Support loading individual progs Joe Stringer
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

reference tracking: leak potential reference
reference tracking: leak potential reference on stack
reference tracking: leak potential reference on stack 2
reference tracking: zero potential reference
reference tracking: copy and zero potential references
reference tracking: release reference without check
reference tracking: release reference
reference tracking: release reference twice
reference tracking: release reference twice inside branch
reference tracking: alloc, check, free in one subbranch
reference tracking: alloc, check, free in both subbranches
reference tracking in call: free reference in subprog
reference tracking in call: free reference in subprog and outside
reference tracking in call: alloc & leak reference in subprog
reference tracking in call: alloc in subprog, release outside
reference tracking in call: sk_ptr leak into caller stack
reference tracking in call: sk_ptr spill into caller stack

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 tools/testing/selftests/bpf/test_verifier.c | 359 ++++++++++++++++++++
 1 file changed, 359 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index ceb55a9f3da9..eb760ead257a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2014 PLUMgrid, http://plumgrid.com
  * Copyright (c) 2017 Facebook
+ * Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of version 2 of the GNU General Public
@@ -177,6 +178,23 @@ static void bpf_fill_rand_ld_dw(struct bpf_test *self)
 	self->retval = (uint32_t)res;
 }
 
+#define BPF_SK_LOOKUP							\
+	/* struct bpf_sock_tuple tuple = {} */				\
+	BPF_MOV64_IMM(BPF_REG_2, 0),					\
+	BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -8),			\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -16),		\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -24),		\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -32),		\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -40),		\
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -48),		\
+	/* sk = sk_lookup_tcp(ctx, &tuple, sizeof tuple, 0, 0) */	\
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),				\
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -48),				\
+	BPF_MOV64_IMM(BPF_REG_3, 44),					\
+	BPF_MOV64_IMM(BPF_REG_4, 0),					\
+	BPF_MOV64_IMM(BPF_REG_5, 0),					\
+	BPF_EMIT_CALL(BPF_FUNC_sk_lookup_tcp)
+
 static struct bpf_test tests[] = {
 	{
 		"add+sub+mul",
@@ -12441,6 +12459,222 @@ static struct bpf_test tests[] = {
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 		.result = ACCEPT,
 	},
+	{
+		"reference tracking: leak potential reference",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_0), /* leak reference */
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: leak potential reference on stack",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),
+			BPF_STX_MEM(BPF_DW, BPF_REG_4, BPF_REG_0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: leak potential reference on stack 2",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),
+			BPF_STX_MEM(BPF_DW, BPF_REG_4, BPF_REG_0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_ST_MEM(BPF_DW, BPF_REG_4, 0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: zero potential reference",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_IMM(BPF_REG_0, 0), /* leak reference */
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: copy and zero potential references",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_MOV64_IMM(BPF_REG_7, 0), /* leak reference */
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: release reference without check",
+		.insns = {
+			BPF_SK_LOOKUP,
+			/* reference in r0 may be NULL */
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "type=sock_or_null expected=sock",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: release reference",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.result = ACCEPT,
+	},
+	{
+		"reference tracking: release reference 2",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+			BPF_EXIT_INSN(),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.result = ACCEPT,
+	},
+	{
+		"reference tracking: release reference twice",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "type=inv expected=sock",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: release reference twice inside branch",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4), /* goto end */
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "type=inv expected=sock",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: alloc, check, free in one subbranch",
+		.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_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 16),
+			/* if (offsetof(skb, mark) > data_len) exit; */
+			BPF_JMP_REG(BPF_JLE, BPF_REG_0, BPF_REG_3, 1),
+			BPF_EXIT_INSN(),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_2,
+				    offsetof(struct __sk_buff, mark)),
+			BPF_SK_LOOKUP,
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 1), /* mark == 0? */
+			/* Leak reference in R0 */
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 3), /* sk NULL? */
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking: alloc, check, free in both subbranches",
+		.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_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 16),
+			/* if (offsetof(skb, mark) > data_len) exit; */
+			BPF_JMP_REG(BPF_JLE, BPF_REG_0, BPF_REG_3, 1),
+			BPF_EXIT_INSN(),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_2,
+				    offsetof(struct __sk_buff, mark)),
+			BPF_SK_LOOKUP,
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 5), /* mark == 0? */
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 3), /* sk NULL? */
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 3), /* sk NULL? */
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.result = ACCEPT,
+	},
+	{
+		"reference tracking in call: free reference in subprog",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), /* unchecked reference */
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_1),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.result = ACCEPT,
+	},
 	{
 		"pass modified ctx pointer to helper, 1",
 		.insns = {
@@ -12511,6 +12745,131 @@ static struct bpf_test tests[] = {
 		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 		.result = ACCEPT,
 	},
+	{
+		"reference tracking in call: free reference in subprog and outside",
+		.insns = {
+			BPF_SK_LOOKUP,
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), /* unchecked reference */
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 3),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_1),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "type=inv expected=sock",
+		.result = REJECT,
+	},
+	{
+		"reference tracking in call: alloc & leak reference in subprog",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 3),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_4),
+			BPF_SK_LOOKUP,
+			/* spill unchecked sk_ptr into stack of caller */
+			BPF_STX_MEM(BPF_DW, BPF_REG_6, BPF_REG_0, 0),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking in call: alloc in subprog, release outside",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 5),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_SK_LOOKUP,
+			BPF_EXIT_INSN(), /* return sk */
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.retval = POINTER_VALUE,
+		.result = ACCEPT,
+	},
+	{
+		"reference tracking in call: sk_ptr leak into caller stack",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_MOV64_REG(BPF_REG_5, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, -8),
+			BPF_STX_MEM(BPF_DW, BPF_REG_5, BPF_REG_4, 0),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 5),
+			/* spill unchecked sk_ptr into stack of caller */
+			BPF_MOV64_REG(BPF_REG_5, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_4, BPF_REG_5, 0),
+			BPF_STX_MEM(BPF_DW, BPF_REG_4, BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+
+			/* subprog 2 */
+			BPF_SK_LOOKUP,
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.errstr = "Unreleased reference",
+		.result = REJECT,
+	},
+	{
+		"reference tracking in call: sk_ptr spill into caller stack",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+
+			/* subprog 1 */
+			BPF_MOV64_REG(BPF_REG_5, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, -8),
+			BPF_STX_MEM(BPF_DW, BPF_REG_5, BPF_REG_4, 0),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 9),
+			/* spill unchecked sk_ptr into stack of caller */
+			BPF_MOV64_REG(BPF_REG_5, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_4, BPF_REG_5, 0),
+			BPF_STX_MEM(BPF_DW, BPF_REG_4, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 3),
+			/* now the sk_ptr is verified, free the reference */
+			BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_4, 0),
+			BPF_MOV64_IMM(BPF_REG_2, 0),
+			BPF_EMIT_CALL(BPF_FUNC_sk_release),
+			BPF_EXIT_INSN(),
+
+			/* subprog 2 */
+			BPF_SK_LOOKUP,
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+		.result = ACCEPT,
+	},
 };
 
 static int probe_filter_length(const struct bpf_insn *fp)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking
  2018-09-12  0:36 ` [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking Joe Stringer
@ 2018-09-13  0:09   ` Alexei Starovoitov
  0 siblings, 0 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-13  0:09 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:37PM -0700, Joe Stringer wrote:
> reference tracking: leak potential reference
> reference tracking: leak potential reference on stack
> reference tracking: leak potential reference on stack 2
> reference tracking: zero potential reference
> reference tracking: copy and zero potential references
> reference tracking: release reference without check
> reference tracking: release reference
> reference tracking: release reference twice
> reference tracking: release reference twice inside branch
> reference tracking: alloc, check, free in one subbranch
> reference tracking: alloc, check, free in both subbranches
> reference tracking in call: free reference in subprog
> reference tracking in call: free reference in subprog and outside
> reference tracking in call: alloc & leak reference in subprog
> reference tracking in call: alloc in subprog, release outside
> reference tracking in call: sk_ptr leak into caller stack
> reference tracking in call: sk_ptr spill into caller stack
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
...
> +		"reference tracking in call: alloc in subprog, release outside",
> +		.insns = {
> +			BPF_MOV64_REG(BPF_REG_4, BPF_REG_10),
> +			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 5),
> +			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> +			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +			BPF_MOV64_IMM(BPF_REG_2, 0),
> +			BPF_EMIT_CALL(BPF_FUNC_sk_release),
> +			BPF_EXIT_INSN(),
> +
> +			/* subprog 1 */
> +			BPF_SK_LOOKUP,
> +			BPF_EXIT_INSN(), /* return sk */
> +		},

Thanks a lot for adding subprog tests and checking that return to the caller
and spill works too.
Awesome stuff!

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 09/11] libbpf: Support loading individual progs
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (7 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-13  0:10   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking Joe Stringer
  2018-09-12  0:36 ` [PATCH bpf-next 11/11] Documentation: Describe bpf " Joe Stringer
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Allow the individual program load to be invoked. This will help with
testing, where a single ELF may contain several sections, some of which
denote subprograms that are expected to fail verification, along with
some which are expected to pass verification. By allowing programs to be
iterated and individually loaded, each program can be independently
checked against its expected verification result.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 tools/lib/bpf/libbpf.c | 4 ++--
 tools/lib/bpf/libbpf.h | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8476da7f2720..aadf05f6bfa0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -227,7 +227,7 @@ struct bpf_object {
 };
 #define obj_elf_valid(o)	((o)->efile.elf)
 
-static void bpf_program__unload(struct bpf_program *prog)
+void bpf_program__unload(struct bpf_program *prog)
 {
 	int i;
 
@@ -1375,7 +1375,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
 	return ret;
 }
 
-static int
+int
 bpf_program__load(struct bpf_program *prog,
 		  char *license, u32 kern_version)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e3b00e23e181..40e4395f1c07 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -126,10 +126,13 @@ void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex);
 
 const char *bpf_program__title(struct bpf_program *prog, bool needs_copy);
 
+int bpf_program__load(struct bpf_program *prog, char *license,
+		      u32 kern_version);
 int bpf_program__fd(struct bpf_program *prog);
 int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
 			      int instance);
 int bpf_program__pin(struct bpf_program *prog, const char *path);
+void bpf_program__unload(struct bpf_program *prog);
 
 struct bpf_insn;
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 09/11] libbpf: Support loading individual progs
  2018-09-12  0:36 ` [PATCH bpf-next 09/11] libbpf: Support loading individual progs Joe Stringer
@ 2018-09-13  0:10   ` Alexei Starovoitov
  0 siblings, 0 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-13  0:10 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:38PM -0700, Joe Stringer wrote:
> Allow the individual program load to be invoked. This will help with
> testing, where a single ELF may contain several sections, some of which
> denote subprograms that are expected to fail verification, along with
> some which are expected to pass verification. By allowing programs to be
> iterated and individually loaded, each program can be independently
> checked against its expected verification result.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (8 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 09/11] libbpf: Support loading individual progs Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-13  0:11   ` Alexei Starovoitov
  2018-09-12  0:36 ` [PATCH bpf-next 11/11] Documentation: Describe bpf " Joe Stringer
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/test_progs.c      |  38 ++++++
 .../selftests/bpf/test_sk_lookup_kern.c       | 128 ++++++++++++++++++
 3 files changed, 167 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/test_sk_lookup_kern.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index fff7fb1285fc..311b7bc9e37a 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -35,7 +35,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
 	test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
 	test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
 	get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
-	test_skb_cgroup_id_kern.o
+	test_skb_cgroup_id_kern.o test_sk_lookup_kern.o
 
 # Order correspond to 'make run_tests' order
 TEST_PROGS := test_kmod.sh \
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 63a671803ed6..e8becca9c521 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1698,6 +1698,43 @@ static void test_task_fd_query_tp(void)
 				   "sys_enter_read");
 }
 
+static void test_reference_tracking()
+{
+	const char *file = "./test_sk_lookup_kern.o";
+	struct bpf_object *obj;
+	struct bpf_program *prog;
+	__u32 duration;
+	int err = 0;
+
+	obj = bpf_object__open(file);
+	if (IS_ERR(obj)) {
+		error_cnt++;
+		return;
+	}
+
+	bpf_object__for_each_program(prog, obj) {
+		const char *title;
+
+		/* Ignore .text sections */
+		title = bpf_program__title(prog, false);
+		if (strstr(title, ".text") != NULL)
+			continue;
+
+		bpf_program__set_type(prog, BPF_PROG_TYPE_SCHED_CLS);
+
+		/* Expect verifier failure if test name has 'fail' */
+		if (strstr(title, "fail") != NULL) {
+			libbpf_set_print(NULL, NULL, NULL);
+			err = !bpf_program__load(prog, "GPL", 0);
+			libbpf_set_print(printf, printf, NULL);
+		} else {
+			err = bpf_program__load(prog, "GPL", 0);
+		}
+		CHECK(err, title, "\n");
+	}
+	bpf_object__close(obj);
+}
+
 int main(void)
 {
 	jit_enabled = is_jit_enabled();
@@ -1719,6 +1756,7 @@ int main(void)
 	test_get_stack_raw_tp();
 	test_task_fd_query_rawtp();
 	test_task_fd_query_tp();
+	test_reference_tracking();
 
 	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
 	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/tools/testing/selftests/bpf/test_sk_lookup_kern.c b/tools/testing/selftests/bpf/test_sk_lookup_kern.c
new file mode 100644
index 000000000000..321a2299a3ac
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sk_lookup_kern.c
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
+
+#include <stddef.h>
+#include <string.h>
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/pkt_cls.h>
+#include <linux/tcp.h>
+#include <sys/socket.h>
+#include "bpf_helpers.h"
+#include "bpf_endian.h"
+
+int _version SEC("version") = 1;
+char _license[] SEC("license") = "GPL";
+
+/* Fill 'tuple' with L3 info, and attempt to find L4. On fail, return NULL. */
+static void *fill_ip(struct bpf_sock_tuple *tuple, void *data, __u64 nh_off,
+		     void *data_end, __u16 eth_proto)
+{
+	__u64 ihl_len;
+	__u8 proto;
+
+	if (eth_proto == bpf_htons(ETH_P_IP)) {
+		struct iphdr *iph = (struct iphdr *)(data + nh_off);
+
+		if (iph + 1 > data_end)
+			return NULL;
+		ihl_len = iph->ihl * 4;
+		proto = iph->protocol;
+
+		tuple->family = AF_INET;
+		tuple->saddr.ipv4 = iph->saddr;
+		tuple->daddr.ipv4 = iph->daddr;
+	} else if (eth_proto == bpf_htons(ETH_P_IPV6)) {
+		struct ipv6hdr *ip6h = (struct ipv6hdr *)(data + nh_off);
+
+		if (ip6h + 1 > data_end)
+			return NULL;
+		ihl_len = sizeof(*ip6h);
+		proto = ip6h->nexthdr;
+
+		tuple->family = AF_INET6;
+		*((struct in6_addr *)&tuple->saddr.ipv6) = ip6h->saddr;
+		*((struct in6_addr *)&tuple->daddr.ipv6) = ip6h->daddr;
+	}
+
+	if (proto != IPPROTO_TCP)
+		return NULL;
+
+	return data + nh_off + ihl_len;
+}
+
+SEC("sk_lookup_success")
+int bpf_sk_lookup_test0(struct __sk_buff *skb)
+{
+	void *data_end = (void *)(long)skb->data_end;
+	void *data = (void *)(long)skb->data;
+	struct ethhdr *eth = (struct ethhdr *)(data);
+	struct bpf_sock_tuple tuple = {};
+	struct tcphdr *tcp;
+	struct bpf_sock *sk;
+	void *l4;
+
+	if (eth + 1 > data_end)
+		return TC_ACT_SHOT;
+
+	l4 = fill_ip(&tuple, data, sizeof(*eth), data_end, eth->h_proto);
+	if (!l4 || l4 + sizeof *tcp > data_end)
+		return TC_ACT_SHOT;
+
+	tcp = l4;
+	tuple.sport = tcp->source;
+	tuple.dport = tcp->dest;
+
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	if (sk)
+		bpf_sk_release(sk, 0);
+	return sk ? TC_ACT_OK : TC_ACT_UNSPEC;
+}
+
+SEC("fail_no_release")
+int bpf_sk_lookup_test1(struct __sk_buff *skb)
+{
+	struct bpf_sock_tuple tuple = {};
+
+	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	return 0;
+}
+
+SEC("fail_release_twice")
+int bpf_sk_lookup_test2(struct __sk_buff *skb)
+{
+	struct bpf_sock_tuple tuple = {};
+	struct bpf_sock *sk;
+
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	bpf_sk_release(sk, 0);
+	bpf_sk_release(sk, 0);
+	return 0;
+}
+
+SEC("fail_release_unchecked")
+int bpf_sk_lookup_test3(struct __sk_buff *skb)
+{
+	struct bpf_sock_tuple tuple = {};
+	struct bpf_sock *sk;
+
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	bpf_sk_release(sk, 0);
+	return 0;
+}
+
+void lookup_no_release(struct __sk_buff *skb)
+{
+	struct bpf_sock_tuple tuple = {};
+	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+}
+
+SEC("fail_no_release_subcall")
+int bpf_sk_lookup_test4(struct __sk_buff *skb)
+{
+	lookup_no_release(skb);
+	return 0;
+}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking
  2018-09-12  0:36 ` [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking Joe Stringer
@ 2018-09-13  0:11   ` Alexei Starovoitov
  2018-09-13 20:56     ` Joe Stringer
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-13  0:11 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:39PM -0700, Joe Stringer wrote:
> Signed-off-by: Joe Stringer <joe@wand.net.nz>

really nice set of tests.
please describe them briefly in commit log.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking
  2018-09-13  0:11   ` Alexei Starovoitov
@ 2018-09-13 20:56     ` Joe Stringer
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-13 20:56 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Stringer, daniel, netdev, ast, john fastabend, tgraf,
	Martin KaFai Lau, Nitin Hande, mauricio.vasquez

On Wed, 12 Sep 2018 at 17:11, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 11, 2018 at 05:36:39PM -0700, Joe Stringer wrote:
> > Signed-off-by: Joe Stringer <joe@wand.net.nz>
>
> really nice set of tests.
> please describe them briefly in commit log.
>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Ack, will do.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH bpf-next 11/11] Documentation: Describe bpf reference tracking
  2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
                   ` (9 preceding siblings ...)
  2018-09-12  0:36 ` [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking Joe Stringer
@ 2018-09-12  0:36 ` Joe Stringer
  2018-09-13  0:12   ` Alexei Starovoitov
  10 siblings, 1 reply; 26+ messages in thread
From: Joe Stringer @ 2018-09-12  0:36 UTC (permalink / raw)
  To: daniel
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 Documentation/networking/filter.txt | 64 +++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index e6b4ebb2b243..4443ce958862 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -1125,6 +1125,14 @@ pointer type.  The types of pointers describe their base, as follows:
     PTR_TO_STACK        Frame pointer.
     PTR_TO_PACKET       skb->data.
     PTR_TO_PACKET_END   skb->data + headlen; arithmetic forbidden.
+    PTR_TO_SOCKET       Pointer to struct bpf_sock_ops, implicitly refcounted.
+    PTR_TO_SOCKET_OR_NULL
+                        Either a pointer to a socket, or NULL; socket lookup
+                        returns this type, which becomes a PTR_TO_SOCKET when
+                        checked != NULL. PTR_TO_SOCKET is reference-counted,
+                        so programs must release the reference through the
+                        socket release function before the end of the program.
+                        Arithmetic on these pointers is forbidden.
 However, a pointer may be offset from this base (as a result of pointer
 arithmetic), and this is tracked in two parts: the 'fixed offset' and 'variable
 offset'.  The former is used when an exactly-known value (e.g. an immediate
@@ -1171,6 +1179,13 @@ over the Ethernet header, then reads IHL and addes (IHL * 4), the resulting
 pointer will have a variable offset known to be 4n+2 for some n, so adding the 2
 bytes (NET_IP_ALIGN) gives a 4-byte alignment and so word-sized accesses through
 that pointer are safe.
+The 'id' field is also used on PTR_TO_SOCKET and PTR_TO_SOCKET_OR_NULL, common
+to all copies of the pointer returned from a socket lookup. This has similar
+behaviour to the handling for PTR_TO_MAP_VALUE_OR_NULL->PTR_TO_MAP_VALUE, but
+it also handles reference tracking for the pointer. PTR_TO_SOCKET implicitly
+represents a reference to the corresponding 'struct sock'. To ensure that the
+reference is not leaked, it is imperative to NULL-check the reference and in
+the non-NULL case, and pass the valid reference to the socket release function.
 
 Direct packet access
 --------------------
@@ -1444,6 +1459,55 @@ Error:
   8: (7a) *(u64 *)(r0 +0) = 1
   R0 invalid mem access 'imm'
 
+Program that performs a socket lookup then sets the pointer to NULL without
+checking it:
+value:
+  BPF_MOV64_IMM(BPF_REG_2, 0),
+  BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -8),
+  BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+  BPF_MOV64_IMM(BPF_REG_3, 4),
+  BPF_MOV64_IMM(BPF_REG_4, 0),
+  BPF_MOV64_IMM(BPF_REG_5, 0),
+  BPF_EMIT_CALL(BPF_FUNC_sk_lookup_tcp),
+  BPF_MOV64_IMM(BPF_REG_0, 0),
+  BPF_EXIT_INSN(),
+Error:
+  0: (b7) r2 = 0
+  1: (63) *(u32 *)(r10 -8) = r2
+  2: (bf) r2 = r10
+  3: (07) r2 += -8
+  4: (b7) r3 = 4
+  5: (b7) r4 = 0
+  6: (b7) r5 = 0
+  7: (85) call bpf_sk_lookup_tcp#65
+  8: (b7) r0 = 0
+  9: (95) exit
+  Unreleased reference id=1, alloc_insn=7
+
+Program that performs a socket lookup but does not NULL-check the returned
+value:
+  BPF_MOV64_IMM(BPF_REG_2, 0),
+  BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -8),
+  BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+  BPF_MOV64_IMM(BPF_REG_3, 4),
+  BPF_MOV64_IMM(BPF_REG_4, 0),
+  BPF_MOV64_IMM(BPF_REG_5, 0),
+  BPF_EMIT_CALL(BPF_FUNC_sk_lookup_tcp),
+  BPF_EXIT_INSN(),
+Error:
+  0: (b7) r2 = 0
+  1: (63) *(u32 *)(r10 -8) = r2
+  2: (bf) r2 = r10
+  3: (07) r2 += -8
+  4: (b7) r3 = 4
+  5: (b7) r4 = 0
+  6: (b7) r5 = 0
+  7: (85) call bpf_sk_lookup_tcp#65
+  8: (95) exit
+  Unreleased reference id=1, alloc_insn=7
+
 Testing
 -------
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 11/11] Documentation: Describe bpf reference tracking
  2018-09-12  0:36 ` [PATCH bpf-next 11/11] Documentation: Describe bpf " Joe Stringer
@ 2018-09-13  0:12   ` Alexei Starovoitov
  2018-09-13 20:56     ` Joe Stringer
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2018-09-13  0:12 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez

On Tue, Sep 11, 2018 at 05:36:40PM -0700, Joe Stringer wrote:
> Signed-off-by: Joe Stringer <joe@wand.net.nz>

just few words in commit log would be better than nothing.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH bpf-next 11/11] Documentation: Describe bpf reference tracking
  2018-09-13  0:12   ` Alexei Starovoitov
@ 2018-09-13 20:56     ` Joe Stringer
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Stringer @ 2018-09-13 20:56 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Stringer, daniel, netdev, ast, john fastabend, tgraf,
	Martin KaFai Lau, Nitin Hande, mauricio.vasquez

On Wed, 12 Sep 2018 at 17:13, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 11, 2018 at 05:36:40PM -0700, Joe Stringer wrote:
> > Signed-off-by: Joe Stringer <joe@wand.net.nz>
>
> just few words in commit log would be better than nothing.
>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Ack, thanks for the review!

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-09-14 12:25 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-12  0:36 [PATCH bpf-next 00/11] Add socket lookup support Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 01/11] bpf: Add iterator for spilled registers Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 02/11] bpf: Simplify ptr_min_max_vals adjustment Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 03/11] bpf: Generalize ptr_or_null regs check Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type Joe Stringer
2018-09-12 22:50   ` Alexei Starovoitov
2018-09-13 18:59     ` Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 05/11] bpf: Macrofy stack state copy Joe Stringer
2018-09-12 22:51   ` Alexei Starovoitov
2018-09-12  0:36 ` [PATCH bpf-next 06/11] bpf: Add reference tracking to verifier Joe Stringer
2018-09-12 23:17   ` Alexei Starovoitov
2018-09-13 19:00     ` Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 07/11] bpf: Add helper to retrieve socket in BPF Joe Stringer
2018-09-13  0:06   ` Alexei Starovoitov
2018-09-14  6:57   ` kbuild test robot
2018-09-14  7:11   ` kbuild test robot
2018-09-12  0:36 ` [PATCH bpf-next 08/11] selftests/bpf: Add tests for reference tracking Joe Stringer
2018-09-13  0:09   ` Alexei Starovoitov
2018-09-12  0:36 ` [PATCH bpf-next 09/11] libbpf: Support loading individual progs Joe Stringer
2018-09-13  0:10   ` Alexei Starovoitov
2018-09-12  0:36 ` [PATCH bpf-next 10/11] selftests/bpf: Add C tests for reference tracking Joe Stringer
2018-09-13  0:11   ` Alexei Starovoitov
2018-09-13 20:56     ` Joe Stringer
2018-09-12  0:36 ` [PATCH bpf-next 11/11] Documentation: Describe bpf " Joe Stringer
2018-09-13  0:12   ` Alexei Starovoitov
2018-09-13 20:56     ` Joe Stringer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).