All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: [PATCH bpf-next v2 09/13] bpf: Mark OBJ_RELEASE argument as MEM_RCU when possible
Date: Fri, 25 Aug 2023 12:54:14 -0700	[thread overview]
Message-ID: <20230825195414.96952-1-yonghong.song@linux.dev> (raw)
In-Reply-To: <20230825195328.92126-1-yonghong.song@linux.dev>

In previous selftests/bpf patch, we have
  p = bpf_percpu_obj_new(struct val_t);
  if (!p)
          goto out;

  p1 = bpf_kptr_xchg(&e->pc, p);
  if (p1) {
          /* race condition */
          bpf_percpu_obj_drop(p1);
  }

  p = e->pc;
  if (!p)
          goto out;

After bpf_kptr_xchg(), we need to re-read e->pc into 'p'.
This is due to that the second argument of bpf_kptr_xchg() is marked
OBJ_RELEASE and it will be marked as invalid after the call.
So after bpf_kptr_xchg(), 'p' is an unknown scalar,
and the bpf program needs to reread from the map value.

This patch checks if the 'p' has type MEM_ALLOC and MEM_PERCPU,
and if 'p' is RCU protected. If this is the case, 'p' can be marked
as MEM_RCU. MEM_ALLOC needs to be removed since 'p' is not
an owning reference any more. Such a change makes re-read
from the map value unnecessary.

Note that re-reading 'e->pc' after bpf_kptr_xchg() might get
a different value from 'p' if immediately before 'p = e->pc',
another cpu may do another bpf_kptr_xchg() and swap in another value
into 'e->pc'. If this is the case, then 'p = e->pc' may
get either 'p' or another value, and race condition already exists.
So removing direct re-reading seems fine too.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/verifier.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6b7e7ca611f3..dbba2b806017 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9660,6 +9660,26 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 				return -EFAULT;
 			}
 			err = unmark_stack_slots_dynptr(env, &regs[meta.release_regno]);
+		} else if (func_id == BPF_FUNC_kptr_xchg && meta.ref_obj_id) {
+			u32 ref_obj_id = meta.ref_obj_id;
+			bool in_rcu = in_rcu_cs(env);
+			struct bpf_func_state *state;
+			struct bpf_reg_state *reg;
+
+			err = release_reference_state(cur_func(env), ref_obj_id);
+			if (!err) {
+				bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
+					if (reg->ref_obj_id == ref_obj_id) {
+						if (in_rcu && (reg->type & MEM_ALLOC) && (reg->type & MEM_PERCPU)) {
+							reg->ref_obj_id = 0;
+							reg->type &= ~MEM_ALLOC;
+							reg->type |= MEM_RCU;
+						} else {
+							mark_reg_invalid(env, reg);
+						}
+					}
+				}));
+			}
 		} else if (meta.ref_obj_id) {
 			err = release_reference(env, meta.ref_obj_id);
 		} else if (register_is_null(&regs[meta.release_regno])) {
-- 
2.34.1


  parent reply	other threads:[~2023-08-25 19:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-25 19:53 [PATCH bpf-next v2 00/13] bpf: Add support for local percpu kptr Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 01/13] bpf: Add support for non-fix-size percpu mem allocation Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 02/13] bpf: Add BPF_KPTR_PERCPU as a field type Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 03/13] bpf: Add alloc/xchg/direct_access support for local percpu kptr Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 04/13] bpf: Add bpf_this_cpu_ptr/bpf_per_cpu_ptr support for allocated percpu obj Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 05/13] selftests/bpf: Update error message in negative linked_list test Yonghong Song
2023-08-25 19:53 ` [PATCH bpf-next v2 06/13] libbpf: Add __percpu_kptr macro definition Yonghong Song
2023-08-25 21:13   ` Andrii Nakryiko
2023-08-25 23:13     ` Yonghong Song
2023-08-25 19:54 ` [PATCH bpf-next v2 07/13] selftests/bpf: Add bpf_percpu_obj_{new,drop}() macro in bpf_experimental.h Yonghong Song
2023-08-25 19:54 ` [PATCH bpf-next v2 08/13] selftests/bpf: Add tests for array map with local percpu kptr Yonghong Song
2023-08-25 19:54 ` Yonghong Song [this message]
2023-08-25 19:54 ` [PATCH bpf-next v2 10/13] selftests/bpf: Remove unnecessary direct read of " Yonghong Song
2023-08-25 19:54 ` [PATCH bpf-next v2 11/13] selftests/bpf: Add tests for cgrp_local_storage with " Yonghong Song
2023-08-25 19:54 ` [PATCH bpf-next v2 12/13] selftests/bpf: Add some negative tests Yonghong Song
2023-08-25 19:54 ` [PATCH bpf-next v2 13/13] bpf: Mark BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE deprecated Yonghong Song
2023-08-26  5:58 ` [PATCH bpf-next v2 00/13] bpf: Add support for local percpu kptr Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230825195414.96952-1-yonghong.song@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.