[PATCH bpf-next] bpf: Add kptr_xchg to may_be_acquire_function check

[Dave Marchevsky <davemarchevsky@fb.com>]

The may_be_acquire_function check is a weaker version of
is_acquire_function that only uses bpf_func_id to determine whether a
func may be acquiring a reference. Most funcs which acquire a reference
do so regardless of their input, so bpf_func_id is all that's necessary
to make an accurate determination. However, map_lookup_elem only
acquires when operating on certain MAP_TYPEs, so commit 64d85290d79c
("bpf: Allow bpf_map_lookup_elem for SOCKMAP and SOCKHASH") added the
may_be check.

Any helper which always acquires a reference should pass both
may_be_acquire_function and is_acquire_function checks. Recently-added
kptr_xchg passes the latter but not the former. This patch resolves this
discrepancy and does some refactoring such that the list of functions
which always acquire is in one place so future updates are in sync.

Fixes: c0a5a21c25f3 ("bpf: Allow storing referenced kptr in map")
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---

Sent to bpf-next instead of bpf as kptr_xchg not passing
may_be_acquire_function isn't currently breaking anything, just
logically inconsistent.

 kernel/bpf/verifier.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 26e7e787c20a..df4b923e77de 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -477,13 +477,30 @@ static bool type_may_be_null(u32 type)
 	return type & PTR_MAYBE_NULL;
 }
 
+/* These functions acquire a resource that must be later released
+ * regardless of their input
+ */
+static bool __check_function_always_acquires(enum bpf_func_id func_id)
+{
+	switch (func_id) {
+	case BPF_FUNC_sk_lookup_tcp:
+	case BPF_FUNC_sk_lookup_udp:
+	case BPF_FUNC_skc_lookup_tcp:
+	case BPF_FUNC_ringbuf_reserve:
+	case BPF_FUNC_kptr_xchg:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static bool may_be_acquire_function(enum bpf_func_id func_id)
 {
-	return func_id == BPF_FUNC_sk_lookup_tcp ||
-		func_id == BPF_FUNC_sk_lookup_udp ||
-		func_id == BPF_FUNC_skc_lookup_tcp ||
-		func_id == BPF_FUNC_map_lookup_elem ||
-	        func_id == BPF_FUNC_ringbuf_reserve;
+	/* See is_acquire_function for the conditions under which funcs
+	 * not in __check_function_always_acquires acquire a resource
+	 */
+	return __check_function_always_acquires(func_id) ||
+		func_id == BPF_FUNC_map_lookup_elem;
 }
 
 static bool is_acquire_function(enum bpf_func_id func_id,
@@ -491,11 +508,7 @@ static bool is_acquire_function(enum bpf_func_id func_id,
 {
 	enum bpf_map_type map_type = map ? map->map_type : BPF_MAP_TYPE_UNSPEC;
 
-	if (func_id == BPF_FUNC_sk_lookup_tcp ||
-	    func_id == BPF_FUNC_sk_lookup_udp ||
-	    func_id == BPF_FUNC_skc_lookup_tcp ||
-	    func_id == BPF_FUNC_ringbuf_reserve ||
-	    func_id == BPF_FUNC_kptr_xchg)
+	if (__check_function_always_acquires(func_id))
 		return true;
 
 	if (func_id == BPF_FUNC_map_lookup_elem &&
-- 
2.30.2