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
next prev 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 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.