bpf: put bpf_link's program when link is safe to be deallocated
Andrii Nakryiko
maintainerPATCH v2 1/3 bpf: put bpf_link's program when link is safe to be deallocated
In general, BPF link's underlying BPF program should be considered to be reachable through attach hook -> link -> prog chain, and, pessimistically, we have to assume that as long as link's memory is not safe to free, attach hook's code might hold a pointer to BPF program and use it.
As such, it's not (generally) correct to put link's program early before waiting for RCU GPs to go through. More eager bpf_prog_put() that we currently do is mostly correct due to BPF program's release code doing similar RCU GP waiting, but as will be shown in the following patches, BPF program can be non-sleepable (and, thus, reliant on only "classic" RCU GP), while BPF link's attach hook can have sleepable semantics and needs to be protected by RCU Tasks Trace, and for such cases BPF link has to go through RCU Tasks Trace + "classic" RCU GPs before being deallocated. And so, if we put BPF program early, we might free BPF program before we free BPF link, leading to use-after-free situation.
So, this patch defers bpf_prog_put() until we are ready to perform bpf_link's deallocation. At worst, this delays BPF program freeing by one extra RCU GP, but that seems completely acceptable. Alternatively, we'd need more elaborate ways to determine BPF hook, BPF link, and BPF program lifetimes, and how they relate to each other, which seems like an unnecessary complication.
Note, for most BPF links we still will perform eager bpf_prog_put() and link dealloc, so for those BPF links there are no observable changes whatsoever. Only BPF links that use deferred dealloc might notice slightly delayed freeing of BPF programs.
Also, to reduce code and logic duplication, extract program put + link dealloc logic into bpf_link_dealloc() helper.
--- kernel/bpf/syscall.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
kernel/bpf/syscall.c +17 -5
@@ -2976,12 +2976,24 @@ void bpf_link_inc(struct bpf_link *link)atomic64_inc (&link->refcnt); } +static void bpf_link_dealloc (struct bpf_link *link) +{ +/* now that we know that bpf_link itself can't be reached, put underlying BPF program */ +if (link->prog) +bpf_prog_put (link->prog); + +/* free bpf_link and its containing memory */ +if (link->ops->dealloc_deferred) + link->ops->dealloc_deferred (link); +else + link->ops->dealloc (link); +} +static void bpf_link_defer_dealloc_rcu_gp (struct rcu_head *rcu) { struct bpf_link *link = container_of(rcu, struct bpf_link, rcu); -/* free bpf_link and its containing memory */ - link->ops->dealloc_deferred (link); +bpf_link_dealloc (link); }static void bpf_link_defer_dealloc_mult_rcu_gp (struct rcu_head *rcu)
@@ -3003,7 +3015,6 @@ static void bpf_link_free(struct bpf_link *link) sleepable = link->prog->sleepable;/* detach BPF program, clean up used resources */ ops->release (link); -bpf_prog_put (link->prog); }if (ops->dealloc_deferred) {/* schedule BPF link deallocation; if underlying BPF program
@@ -3014,8 +3025,9 @@ static void bpf_link_free(struct bpf_link *link)call_rcu_tasks_trace (&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp); else call_rcu(&link->rcu, bpf_link_defer_dealloc_rcu_gp); - }else if (ops->dealloc) - ops->dealloc (link); + }else if (ops->dealloc) { +bpf_link_dealloc (link); + } }static void bpf_link_put_deferred (struct work_struct *work)
Signature · 1 line
2.43.5
Andrii Nakryiko
maintainerPATCH v2 2/3 bpf: decouple BPF link/attach hook and BPF program sleepable semantics
BPF link's lifecycle protection scheme depends on both BPF hook and BPF program. If *either* of those require RCU Tasks Trace GP, then we need to go through a chain of GPs before putting BPF program refcount and deallocating BPF link memory.
This patch adds bpf_link-specific sleepable flag, which can be set to true even if underlying BPF program is not sleepable itself. If either link->sleepable or link->prog->sleepable is true, we'll go through a chain of RCU Tasks Trace GP and RCU GP before putting BPF program and freeing memory.
This will be used to protect BPF link for sleepable (faultable) raw tracepoints in the next patch.
--- include/linux/bpf.h | 20 ++++++++++++++++++-- kernel/bpf/syscall.c | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 13 deletions(-)
include/linux/bpf.h +18 -2
@@ -1598,6 +1598,11 @@ struct bpf_link {enum bpf_link_type type;const struct bpf_link_ops *ops;struct bpf_prog *prog; +/* whether BPF link itself has "sleepable" semantics, which can differ +* from underlying BPF program having a "sleepable" semantics, as BPF +* link's semantics is determined by target attach hook +*/ +bool sleepable;/* rcu is used before freeing, work can be used to schedule that * RCU-based freeing before that, so they never overlap */
@@ -1614,8 +1619,10 @@ struct bpf_link_ops {*/ void (*dealloc)(struct bpf_link *link);/* deallocate link resources callback, called after RCU grace period; -* if underlying BPF program is sleepable we go through tasks trace -* RCU GP and then "classic" RCU GP +* if either the underlying BPF program is sleepable or BPF link's +* target hook is sleepable, we'll go through tasks trace RCU GP and +* then "classic" RCU GP; this need for chaining tasks trace and +* classic RCU GPs is designated by setting bpf_link->sleepable flag */ void (*dealloc_deferred)(struct bpf_link *link);int (*detach)(struct bpf_link *link);
@@ -2362,6 +2369,9 @@ int bpf_prog_new_fd(struct bpf_prog *prog);void bpf_link_init (struct bpf_link *link,enum bpf_link_type type,const struct bpf_link_ops *ops,struct bpf_prog *prog); +void bpf_link_init_sleepable (struct bpf_link *link,enum bpf_link_type type, +const struct bpf_link_ops *ops,struct bpf_prog *prog, +bool sleepable);int bpf_link_prime (struct bpf_link *link,struct bpf_link_primer *primer);int bpf_link_settle (struct bpf_link_primer *primer);void bpf_link_cleanup (struct bpf_link_primer *primer);
@@ -2717,6 +2727,12 @@ static inline void bpf_link_init(struct bpf_link *link, enum bpf_link_type type, { } +static inline void bpf_link_init_sleepable (struct bpf_link *link,enum bpf_link_type type, +const struct bpf_link_ops *ops,struct bpf_prog *prog, +bool sleepable) +{ +} +static inline int bpf_link_prime (struct bpf_link *link,struct bpf_link_primer *primer) {
kernel/bpf/syscall.c +28 -11
@@ -2933,17 +2933,33 @@ static int bpf_obj_get(const union bpf_attr *attr) attr->file_flags); } -void bpf_link_init (struct bpf_link *link,enum bpf_link_type type, -const struct bpf_link_ops *ops,struct bpf_prog *prog) +/* bpf_link_init_sleepable() allows to specify whether BPF link itself has +* "sleepable" semantics, which normally would mean that BPF link's attach +* hook can dereference link or link's underlying program for some time after +* detachment due to RCU Tasks Trace-based lifetime protection scheme. +* BPF program itself can be non-sleepable, yet, because it's transitively +* reachable through BPF link, its freeing has to be delayed until after RCU +* Tasks Trace GP. +*/ +void bpf_link_init_sleepable (struct bpf_link *link,enum bpf_link_type type, +const struct bpf_link_ops *ops,struct bpf_prog *prog, +bool sleepable) {WARN_ON (ops->dealloc && ops->dealloc_deferred);atomic64_set (&link->refcnt,1 ); link->type = type; + link->sleepable = sleepable; link->id =0 ; link->ops = ops; link->prog = prog; } +void bpf_link_init (struct bpf_link *link,enum bpf_link_type type, +const struct bpf_link_ops *ops,struct bpf_prog *prog) +{ +bpf_link_init_sleepable (link, type, ops, prog, false); +} +static void bpf_link_free_id (int id) {if (!id)
@@ -3008,20 +3024,21 @@ static void bpf_link_defer_dealloc_mult_rcu_gp(struct rcu_head *rcu)static void bpf_link_free (struct bpf_link *link) {const struct bpf_link_ops *ops = link->ops; -bool sleepable = false;bpf_link_free_id (link->id); -if (link->prog) { - sleepable = link->prog->sleepable; -/* detach BPF program, clean up used resources */ +/* detach BPF program, clean up used resources */ +if (link->prog) ops->release (link); - }if (ops->dealloc_deferred) { -/* schedule BPF link deallocation; if underlying BPF program -* is sleepable, we need to first wait for RCU tasks trace -* sync, then go through "classic" RCU grace period +/* Schedule BPF link deallocation, which will only then +* trigger putting BPF program refcount. +* If underlying BPF program is sleepable or BPF link's target +* attach hookpoint is sleepable or otherwise requires RCU GPs +* to ensure link and its underlying BPF program is not +* reachable anymore, we need to first wait for RCU tasks +* trace sync, and then go through "classic" RCU grace period */ -if (sleepable) +if (link->sleepable || (link->prog && link->prog->sleepable))call_rcu_tasks_trace (&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp);else call_rcu (&link->rcu, bpf_link_defer_dealloc_rcu_gp);
Signature · 1 line
2.43.5
Andrii Nakryiko
maintainerPATCH v2 3/3 bpf: ensure RCU Tasks Trace GP for sleepable raw tracepoint BPF links
In reply to Andrii NakryikoNow that kernel supports sleepable tracepoints, the fact that bpf_probe_unregister() is asynchronous, i.e., that it doesn't wait for any in-flight tracepoints to conclude before returning, we now need to delay BPF raw tp link's deallocation and bpf_prog_put() of its underlying BPF program (regardless of program's own sleepable semantics) until after full RCU Tasks Trace GP. With that GP over, we'll have a guarantee that no tracepoint can reach BPF link and thus its BPF program.
We use newly added tracepoint_is_faultable() check to know when this RCU Tasks Trace GP is necessary and utilize BPF link's own sleepable flag passed through bpf_link_init_sleepable() initializer.
--- kernel/bpf/syscall.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/bpf/syscall.c +4 -2
@@ -35,6 +35,7 @@#include <linux/rcupdate_trace.h> #include <linux/memcontrol.h> #include <linux/trace_events.h> +#include <linux/tracepoint.h> #include <net/netfilter/nf_bpf_link.h> #include <net/netkit.h>
@@ -3845,8 +3846,9 @@ static int bpf_raw_tp_link_attach(struct bpf_prog *prog, err = -ENOMEM ; goto out_put_btp; } -bpf_link_init (&link->link,BPF_LINK_TYPE_RAW_TRACEPOINT , - &bpf_raw_tp_link_lops, prog); +bpf_link_init_sleepable (&link->link,BPF_LINK_TYPE_RAW_TRACEPOINT , + &bpf_raw_tp_link_lops, prog, +tracepoint_is_faultable (btp->tp)); link->btp = btp; link->cookie = cookie;
Signature · 1 line
2.43.5
kernel test robot
build botRe: [PATCH v2 trace/for-next 3/3] bpf: ensure RCU Tasks Trace GP for sleepable raw tracepoint BPF links
kernel test robot noticed the following build errors:
Hi Andrii,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
url: https://github.com/intel-lab-lkp/linux/commits/Andrii-Nakryiko/bpf-decouple-BPF-link-attach-hook-and-BPF-program-sleepable-semantics/20241102-022026 base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next patch link: https://lore.kernel.org/r/20241101181754.782341-3-andrii%40kernel.org patch subject: [PATCH v2 trace/for-next 3/3] bpf: ensure RCU Tasks Trace GP for sleepable raw tracepoint BPF links config: arm-imx_v6_v7_defconfig (https://download.01.org/0day-ci/archive/20241102/[email protected]/config) compiler: arm-linux-gnueabi-gcc (GCC) 13.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
kernel/bpf/syscall.c: In function 'bpf_raw_tp_link_attach':
>> kernel/bpf/syscall.c:3866:33: error: implicit declaration of function 'tracepoint_is_faultable' [-Werror=implicit-function-declaration]
3866 | tracepoint_is_faultable(btp->tp));
| ^~~~~~~~~~~~~~~~~~~~~~~cc1: some warnings being treated as errors
vim +/tracepoint_is_faultable +3866 kernel/bpf/syscall.c
3818
3819 static int bpf_raw_tp_link_attach(struct bpf_prog *prog,
3820 const char __user *user_tp_name, u64 cookie)
3821 {
3822 struct bpf_link_primer link_primer;
3823 struct bpf_raw_tp_link *link;
3824 struct bpf_raw_event_map *btp;
3825 const char *tp_name;
3826 char buf[128];
3827 int err;
3828
3829 switch (prog->type) {
3830 case BPF_PROG_TYPE_TRACING:
3831 case BPF_PROG_TYPE_EXT:
3832 case BPF_PROG_TYPE_LSM:
3833 if (user_tp_name)
3834 /* The attach point for this category of programs
3835 * should be specified via btf_id during program load.
3836 */
3837 return -EINVAL;
3838 if (prog->type == BPF_PROG_TYPE_TRACING &&
3839 prog->expected_attach_type == BPF_TRACE_RAW_TP) {
3840 tp_name = prog->aux->attach_func_name;
3841 break;
3842 }
3843 return bpf_tracing_prog_attach(prog, 0, 0, 0);
3844 case BPF_PROG_TYPE_RAW_TRACEPOINT:
3845 case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
3846 if (strncpy_from_user(buf, user_tp_name, sizeof(buf) - 1) < 0)
3847 return -EFAULT;
3848 buf[sizeof(buf) - 1] = 0;
3849 tp_name = buf;
3850 break;
3851 default:
3852 return -EINVAL;
3853 }
3854
3855 btp = bpf_get_raw_tracepoint(tp_name);
3856 if (!btp)
3857 return -ENOENT;
3858
3859 link = kzalloc(sizeof(*link), GFP_USER);
3860 if (!link) {
3861 err = -ENOMEM;
3862 goto out_put_btp;
3863 }
3864 bpf_link_init_sleepable(&link->link, BPF_LINK_TYPE_RAW_TRACEPOINT,
3865 &bpf_raw_tp_link_lops, prog,
> 3866 tracepoint_is_faultable(btp->tp));
3867 link->btp = btp;
3868 link->cookie = cookie;
3869
3870 err = bpf_link_prime(&link->link, &link_primer);
3871 if (err) {
3872 kfree(link);
3873 goto out_put_btp;
3874 }
3875
3876 err = bpf_probe_register(link->btp, link);
3877 if (err) {
3878 bpf_link_cleanup(&link_primer);
3879 goto out_put_btp;
3880 }
3881
3882 return bpf_link_settle(&link_primer);
3883
3884 out_put_btp:
3885 bpf_put_raw_tracepoint(btp);
3886 return err;
3887 }
3888
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
kernel test robot
build botRe: [PATCH v2 trace/for-next 3/3] bpf: ensure RCU Tasks Trace GP for sleepable raw tracepoint BPF links
In reply to Andrii Nakryikokernel test robot noticed the following build errors:
Hi Andrii,
kernel test robot noticed the following build errors:
[auto build test ERROR on trace/for-next]
url: https://github.com/intel-lab-lkp/linux/commits/Andrii-Nakryiko/bpf-decouple-BPF-link-attach-hook-and-BPF-program-sleepable-semantics/20241102-022026 base: https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next patch link: https://lore.kernel.org/r/20241101181754.782341-3-andrii%40kernel.org patch subject: [PATCH v2 trace/for-next 3/3] bpf: ensure RCU Tasks Trace GP for sleepable raw tracepoint BPF links compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241104/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
make[1]: Circular tools/testing/selftests/alsa/global-timer <- tools/testing/selftests/alsa/global-timer dependency dropped. Makefile:60: warning: overriding recipe for target 'emit_tests'
../lib.mk:182: warning: ignoring old recipe for target 'emit_tests'
make[1]: *** No targets. Stop.
>> Makefile:47: *** Cannot find a vmlinux for VMLINUX_BTF at any of " ../../../../vmlinux /sys/kernel/btf/vmlinux /boot/vmlinux-5.9.0-2-amd64". Stop.
make[1]: *** No targets. Stop.
make[1]: *** No targets. Stop.
vim +47 Makefile
3812b8c5c5d527 Masahiro Yamada 2019-02-22 46
3812b8c5c5d527 Masahiro Yamada 2019-02-22 @47 # Do not use make's built-in rules and variables 3812b8c5c5d527 Masahiro Yamada 2019-02-22 48 # (this increases performance and avoids hard-to-debug behaviour) 3812b8c5c5d527 Masahiro Yamada 2019-02-22 49 MAKEFLAGS += -rR 3812b8c5c5d527 Masahiro Yamada 2019-02-22 50 3812b8c5c5d527 Masahiro Yamada 2019-02-22 51 # Avoid funny character set dependencies 3812b8c5c5d527 Masahiro Yamada 2019-02-22 52 unexport LC_ALL 3812b8c5c5d527 Masahiro Yamada 2019-02-22 53 LC_COLLATE=C 3812b8c5c5d527 Masahiro Yamada 2019-02-22 54 LC_NUMERIC=C 3812b8c5c5d527 Masahiro Yamada 2019-02-22 55 export LC_COLLATE LC_NUMERIC 3812b8c5c5d527 Masahiro Yamada 2019-02-22 56 3812b8c5c5d527 Masahiro Yamada 2019-02-22 57 # Avoid interference with shell env settings 3812b8c5c5d527 Masahiro Yamada 2019-02-22 58 unexport GREP_OPTIONS 3812b8c5c5d527 Masahiro Yamada 2019-02-22 59 066b7ed9558087 Michal Marek 2014-07-04 @60 # Beautify output 066b7ed9558087 Michal Marek 2014-07-04 61 # --------------------------------------------------------------------------- 066b7ed9558087 Michal Marek 2014-07-04 62 # 8962b6b475bddc Masahiro Yamada 2022-12-23 63 # Most of build commands in Kbuild start with "cmd_". You can optionally define 8962b6b475bddc Masahiro Yamada 2022-12-23 64 # "quiet_cmd_*". If defined, the short log is printed. Otherwise, no log from 8962b6b475bddc Masahiro Yamada 2022-12-23 65 # that command is printed by default. 066b7ed9558087 Michal Marek 2014-07-04 66 # 8962b6b475bddc Masahiro Yamada 2022-12-23 67 # e.g.) 8962b6b475bddc Masahiro Yamada 2022-12-23 68 # quiet_cmd_depmod = DEPMOD $(MODLIB) 8962b6b475bddc Masahiro Yamada 2022-12-23 69 # cmd_depmod = $(srctree)/scripts/depmod.sh $(DEPMOD) $(KERNELRELEASE) 066b7ed9558087 Michal Marek 2014-07-04 70 # 066b7ed9558087 Michal Marek 2014-07-04 71 # A simple variant is to prefix commands with $(Q) - that's useful 066b7ed9558087 Michal Marek 2014-07-04 72 # for commands that shall be hidden in non-verbose mode. 066b7ed9558087 Michal Marek 2014-07-04 73 # 8962b6b475bddc Masahiro Yamada 2022-12-23 74 # $(Q)$(MAKE) $(build)=scripts/basic 066b7ed9558087 Michal Marek 2014-07-04 75 # 6ae4b9868a8f72 Masahiro Yamada 2022-12-23 76 # If KBUILD_VERBOSE contains 1, the whole command is echoed. 6ae4b9868a8f72 Masahiro Yamada 2022-12-23 77 # If KBUILD_VERBOSE contains 2, the reason for rebuilding is printed. 066b7ed9558087 Michal Marek 2014-07-04 78 # ^1da177e4c3f41 Linus Torvalds 2005-04-16 79 # To put more focus on warnings, be less verbose as default ^1da177e4c3f41 Linus Torvalds 2005-04-16 80 # Use 'make V=1' to see the full commands
^1da177e4c3f41 Linus Torvalds 2005-04-16 81
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki