BPF List
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@google.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	 martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
	john.fastabend@gmail.com,  kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org
Subject: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr
Date: Mon, 13 Nov 2023 20:54:53 -0800	[thread overview]
Message-ID: <20231114045453.1816995-3-sdf@google.com> (raw)
In-Reply-To: <20231114045453.1816995-1-sdf@google.com>

Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
idr when the offloaded/bound netdev goes away. I was supposed to
take a look and check in [0], but apparently I did not.

The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
stale ids for the programs that have a dead netdev. This functionality
is verified by test_offload.py, but we don't run this test in the CI.

Introduce new bpf_prog_remove_from_idr which takes care of correctly
dealing with potential double idr_remove() via separate skip_idr_remove
flag in the aux.

Verified by running the test manually:
test_offload.py: OK

0: https://lore.kernel.org/all/CAKH8qBtyR20ZWAc11z1-6pGb3Hd47AQUTbE_cfoktG59TqaJ7Q@mail.gmail.com/

Fixes: ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 include/linux/bpf.h  |  2 ++
 kernel/bpf/offload.c |  3 +++
 kernel/bpf/syscall.c | 15 +++++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4001d11be151..d2aa4b59bf1e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
 	bool xdp_has_frags;
 	bool exception_cb;
 	bool exception_boundary;
+	bool skip_idr_remove;
 	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
 	const struct btf_type *attach_func_proto;
 	/* function name for valid attach_btf_id */
@@ -2049,6 +2050,7 @@ void bpf_prog_inc(struct bpf_prog *prog);
 struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog);
 void bpf_prog_put(struct bpf_prog *prog);
 
+void bpf_prog_remove_from_idr(struct bpf_prog *prog);
 void bpf_prog_free_id(struct bpf_prog *prog);
 void bpf_map_free_id(struct bpf_map *map);
 
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 1a4fec330eaa..6f4fe492ee2a 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -112,6 +112,9 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
 	if (offload->dev_state)
 		offload->offdev->ops->destroy(prog);
 
+	/* Make sure BPF_PROG_GET_NEXT_ID can't find this dead program */
+	bpf_prog_remove_from_idr(prog);
+
 	list_del_init(&offload->offloads);
 	kfree(offload);
 	prog->aux->offload = NULL;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0ed286b8a0f0..bc813e03e2cf 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2083,10 +2083,19 @@ static int bpf_prog_alloc_id(struct bpf_prog *prog)
 	return id > 0 ? 0 : id;
 }
 
-void bpf_prog_free_id(struct bpf_prog *prog)
+void bpf_prog_remove_from_idr(struct bpf_prog *prog)
 {
 	unsigned long flags;
 
+	spin_lock_irqsave(&prog_idr_lock, flags);
+	if (!prog->aux->skip_idr_remove)
+		idr_remove(&prog_idr, prog->aux->id);
+	prog->aux->skip_idr_remove = 1;
+	spin_unlock_irqrestore(&prog_idr_lock, flags);
+}
+
+void bpf_prog_free_id(struct bpf_prog *prog)
+{
 	/* cBPF to eBPF migrations are currently not in the idr store.
 	 * Offloaded programs are removed from the store when their device
 	 * disappears - even if someone grabs an fd to them they are unusable,
@@ -2095,10 +2104,8 @@ void bpf_prog_free_id(struct bpf_prog *prog)
 	if (!prog->aux->id)
 		return;
 
-	spin_lock_irqsave(&prog_idr_lock, flags);
-	idr_remove(&prog_idr, prog->aux->id);
+	bpf_prog_remove_from_idr(prog);
 	prog->aux->id = 0;
-	spin_unlock_irqrestore(&prog_idr_lock, flags);
 }
 
 static void __bpf_prog_put_rcu(struct rcu_head *rcu)
-- 
2.42.0.869.gea05f2083d-goog


  parent reply	other threads:[~2023-11-14  4:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-14  4:54 [PATCH bpf-next 0/2] bpf: fix couple of netdevsim issues Stanislav Fomichev
2023-11-14  4:54 ` [PATCH bpf-next 1/2] netdevsim: don't accept device bound programs Stanislav Fomichev
2023-11-20 21:38   ` Stanislav Fomichev
2023-11-20 21:58   ` Jakub Kicinski
2023-11-14  4:54 ` Stanislav Fomichev [this message]
2023-11-20 21:39   ` [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr Stanislav Fomichev
2023-11-21 21:03   ` Martin KaFai Lau
2023-11-22 15:07     ` Daniel Borkmann
2023-11-22 18:05       ` Stanislav Fomichev
2023-11-22 18:40         ` Martin KaFai Lau
2023-11-22 22:41           ` Stanislav Fomichev
2023-11-22 22:54             ` Daniel Borkmann

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=20231114045453.1816995-3-sdf@google.com \
    --to=sdf@google.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox