In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org --- Changes in v2: - Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call() - Extend xdp_metadata selftest - Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern... --- kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; }
-bool bpf_prog_map_compatible(struct bpf_map *map, - const struct bpf_prog *fp) +static bool __bpf_prog_map_compatible(struct bpf_map *map, + const struct bpf_prog *fp) { enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on - * a particular netdev (and can run outside driver context entirely - * in the case of devmap and cpumap). Until device checks - * are implemented, prohibit adding dev-bound programs to program maps. - */ - if (bpf_prog_is_dev_bound(aux)) - return false; - spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for @@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; }
+bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{ + /* XDP programs inserted into maps are not guaranteed to run on + * a particular netdev (and can run outside driver context entirely + * in the case of devmap and cpumap). Until device checks + * are implemented, prohibit adding dev-bound programs to program maps. + */ + if (bpf_prog_is_dev_bound(fp->aux)) + return false; + + return __bpf_prog_map_compatible(map, fp); +} + static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
- if (!bpf_prog_map_compatible(map, fp)) { + if (!__bpf_prog_map_compatible(map, fp)) { ret = -EINVAL; goto out; } diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c index 3d47878ef6bfb55c236dc9df2c100fcc449f8de3..19f92affc2daa23fdd869554e7a0475b86350a4f 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c @@ -351,9 +351,10 @@ void test_xdp_metadata(void) struct xdp_metadata2 *bpf_obj2 = NULL; struct xdp_metadata *bpf_obj = NULL; struct bpf_program *new_prog, *prog; + struct bpf_devmap_val devmap_e = {}; + struct bpf_map *prog_arr, *devmap; struct nstoken *tok = NULL; __u32 queue_id = QUEUE_ID; - struct bpf_map *prog_arr; struct xsk tx_xsk = {}; struct xsk rx_xsk = {}; __u32 val, key = 0; @@ -409,6 +410,13 @@ void test_xdp_metadata(void) bpf_program__set_ifindex(prog, rx_ifindex); bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
+ /* Make sure we can load a dev-bound program that performs + * XDP_REDIRECT into a devmap. + */ + new_prog = bpf_object__find_program_by_name(bpf_obj->obj, "redirect"); + bpf_program__set_ifindex(new_prog, rx_ifindex); + bpf_program__set_flags(new_prog, BPF_F_XDP_DEV_BOUND_ONLY); + if (!ASSERT_OK(xdp_metadata__load(bpf_obj), "load skeleton")) goto out;
@@ -423,6 +431,18 @@ void test_xdp_metadata(void) "update prog_arr")) goto out;
+ /* Make sure we can't add dev-bound programs to devmaps. */ + devmap = bpf_object__find_map_by_name(bpf_obj->obj, "dev_map"); + if (!ASSERT_OK_PTR(devmap, "no dev_map found")) + goto out; + + devmap_e.bpf_prog.fd = val; + if (!ASSERT_ERR(bpf_map__update_elem(devmap, &key, sizeof(key), + &devmap_e, sizeof(devmap_e), + BPF_ANY), + "update dev_map")) + goto out; + /* Attach BPF program to RX interface. */
ret = bpf_xdp_attach(rx_ifindex, diff --git a/tools/testing/selftests/bpf/progs/xdp_metadata.c b/tools/testing/selftests/bpf/progs/xdp_metadata.c index 31ca229bb3c0f182483334a4ab0bd204acae20f3..09bb8a038d528cf26c5b314cc927915ac2796bf0 100644 --- a/tools/testing/selftests/bpf/progs/xdp_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_metadata.c @@ -19,6 +19,13 @@ struct { __type(value, __u32); } prog_arr SEC(".maps");
+struct { + __uint(type, BPF_MAP_TYPE_DEVMAP); + __uint(key_size, sizeof(__u32)); + __uint(value_size, sizeof(struct bpf_devmap_val)); + __uint(max_entries, 1); +} dev_map SEC(".maps"); + extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, __u64 *timestamp) __ksym; extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, @@ -95,4 +102,10 @@ int rx(struct xdp_md *ctx) return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS); }
+SEC("xdp") +int redirect(struct xdp_md *ctx) +{ + return bpf_redirect_map(&dev_map, ctx->rx_queue_index, XDP_PASS); +} + char _license[] SEC("license") = "GPL";
--- base-commit: 5cffad0a5c8f0cc53ce9fe7cff7bc67c3a97c406 change-id: 20250422-xdp-prog-bound-fix-9f30f3e134aa
Best regards,
On 04/23, Lorenzo Bianconi wrote:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Acked-by: Stanislav Fomichev sdf@fomichev.me
Thank you!
On 4/23/25 10:44 AM, Lorenzo Bianconi wrote:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++
The change lgtm. Please separate the selftest changes to patch 2. Thanks.
pw-bot: cr
On Apr 23, Martin KaFai Lau wrote:
On 4/23/25 10:44 AM, Lorenzo Bianconi wrote:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++
The change lgtm. Please separate the selftest changes to patch 2. Thanks.
ack, I will do in v3.
Regards, Lorenzo
pw-bot: cr
Lorenzo Bianconi lorenzo@kernel.org writes:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp){ enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) {
Hmm, so this allows devbound programs in tail call maps, right? But there's no guarantee that a tail call map will always be used for a particular device, is there? For instance, it could be shared between multiple XDP programs, bound to different devices, thus getting the wrong kfunc.
Or you could even have dev-bound programs tail-called from non-dev-bound programs with this change AFAICT?
In other words, I think this is too relaxed, your change in v1 that only relaxed cpumap and devmap checks here was better.
In fact, I don't really see why bpf_check_tail_call() needs to look at devmap/cpumap at all, so maybe just changing the map_type_contains_progs() call to only match tail call maps is better?
-Toke
On 04/24, Toke Høiland-Jørgensen wrote:
Lorenzo Bianconi lorenzo@kernel.org writes:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp){ enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) {Hmm, so this allows devbound programs in tail call maps, right? But there's no guarantee that a tail call map will always be used for a particular device, is there? For instance, it could be shared between multiple XDP programs, bound to different devices, thus getting the wrong kfunc.
Won't this (devbound progs in tail call maps) be still prohibited by a bpf_prog_map_compatible check in prog_fd_array_get_ptr?
Stanislav Fomichev stfomichev@gmail.com writes:
On 04/24, Toke Høiland-Jørgensen wrote:
Lorenzo Bianconi lorenzo@kernel.org writes:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp){ enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) {Hmm, so this allows devbound programs in tail call maps, right? But there's no guarantee that a tail call map will always be used for a particular device, is there? For instance, it could be shared between multiple XDP programs, bound to different devices, thus getting the wrong kfunc.
Won't this (devbound progs in tail call maps) be still prohibited by a bpf_prog_map_compatible check in prog_fd_array_get_ptr?
Yeah, you're right, I misremembered the check and somehow convinced myself that the check in bpf_check_tail_call() was the one that prevented dev-bound programs from being loaded into tail call maps...
-Toke
Lorenzo Bianconi lorenzo@kernel.org writes:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp){ enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) {Hmm, so this allows devbound programs in tail call maps, right? But there's no guarantee that a tail call map will always be used for a particular device, is there? For instance, it could be shared between multiple XDP programs, bound to different devices, thus getting the wrong kfunc.
According to my understanding the following path will be executed just for dev-bound program that performs XDP_REDIRECT into a BPF_MAP_TYPE_PROG_ARRAY:
bpf_prog_select_runtime() -> bpf_check_tail_call() -> __bpf_prog_map_compatible()
while for XDP program inserted into BPF_MAP_TYPE_PROG_ARRAY we will continue running bpf_prog_map_compatible() so we will forbid inserting ev-bound programs. This is even tested into xdp_metadata selftest:
https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/pr...
It seems to me v2 is not more relaxed than v1. Am I missing something?
Regards, Lorenzo
Or you could even have dev-bound programs tail-called from non-dev-bound programs with this change AFAICT?
In other words, I think this is too relaxed, your change in v1 that only relaxed cpumap and devmap checks here was better.
In fact, I don't really see why bpf_check_tail_call() needs to look at devmap/cpumap at all, so maybe just changing the map_type_contains_progs() call to only match tail call maps is better?
-Toke
Lorenzo Bianconi lorenzo@kernel.org writes:
Lorenzo Bianconi lorenzo@kernel.org writes:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconi lorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1: https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp){ enum bpf_prog_type prog_type = resolve_prog_type(fp); bool ret; @@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux; @@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) {Hmm, so this allows devbound programs in tail call maps, right? But there's no guarantee that a tail call map will always be used for a particular device, is there? For instance, it could be shared between multiple XDP programs, bound to different devices, thus getting the wrong kfunc.
According to my understanding the following path will be executed just for dev-bound program that performs XDP_REDIRECT into a BPF_MAP_TYPE_PROG_ARRAY:
bpf_prog_select_runtime() -> bpf_check_tail_call() -> __bpf_prog_map_compatible()
while for XDP program inserted into BPF_MAP_TYPE_PROG_ARRAY we will continue running bpf_prog_map_compatible() so we will forbid inserting ev-bound programs. This is even tested into xdp_metadata selftest:
https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/pr...
It seems to me v2 is not more relaxed than v1. Am I missing something?
No, you're right; see my reply to Stanislav - I misremembered the logic :)
-Toke
On 23/04/2025 19.44, Lorenzo Bianconi wrote:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconilorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1:https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)@@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
- static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux;
@@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) { ret = -EINVAL; goto out;
Does this change allow us to have a dev_bound BPF-prog that have tail-call BPF-progs that are not dev_bound?
The use-case is a dev_bound BPF-prog that reads e.g. HW vlan, store this in data_meta (or a per CPU array), and then tail-calls another BPF-prog that reads the data stored (from data_meta area). Maybe this is already supported before?
--Jesper
On Apr 24, Jesper Dangaard Brouer wrote:
On 23/04/2025 19.44, Lorenzo Bianconi wrote:
In the current implementation if the program is dev-bound to a specific device, it will not be possible to perform XDP_REDIRECT into a DEVMAP or CPUMAP even if the program is running in the driver NAPI context and it is not attached to any map entry. This seems in contrast with the explanation available in bpf_prog_map_compatible routine. Fix the issue introducing __bpf_prog_map_compatible utility routine in order to avoid bpf_prog_is_dev_bound() check running bpf_check_tail_call() at program load time (bpf_prog_select_runtime()). Continue forbidding to attach a dev-bound program to XDP maps (BPF_MAP_TYPE_PROG_ARRAY, BPF_MAP_TYPE_DEVMAP and BPF_MAP_TYPE_CPUMAP).
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs") Signed-off-by: Lorenzo Bianconilorenzo@kernel.org
Changes in v2:
- Introduce __bpf_prog_map_compatible() utility routine in order to skip bpf_prog_is_dev_bound check in bpf_check_tail_call()
- Extend xdp_metadata selftest
- Link to v1:https://lore.kernel.org/r/20250422-xdp-prog-bound-fix-v1-1-0b581fa186fe@kern...
kernel/bpf/core.c | 27 +++++++++++++--------- .../selftests/bpf/prog_tests/xdp_metadata.c | 22 +++++++++++++++++- tools/testing/selftests/bpf/progs/xdp_metadata.c | 13 +++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index ba6b6118cf504041278d05417c4212d57be6fca0..a3e571688421196c3ceaed62b3b59b62a0258a8c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2358,8 +2358,8 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, return 0; } -bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)+static bool __bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)@@ -2368,14 +2368,6 @@ bool bpf_prog_map_compatible(struct bpf_map *map, if (fp->kprobe_override) return false;
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(aux))
return false;- spin_lock(&map->owner.lock); if (!map->owner.type) { /* There's no owner yet where we could check for
@@ -2409,6 +2401,19 @@ bool bpf_prog_map_compatible(struct bpf_map *map, return ret; } +bool bpf_prog_map_compatible(struct bpf_map *map, const struct bpf_prog *fp) +{
- /* XDP programs inserted into maps are not guaranteed to run on
* a particular netdev (and can run outside driver context entirely* in the case of devmap and cpumap). Until device checks* are implemented, prohibit adding dev-bound programs to program maps.*/- if (bpf_prog_is_dev_bound(fp->aux))
return false;- return __bpf_prog_map_compatible(map, fp);
+}
- static int bpf_check_tail_call(const struct bpf_prog *fp) { struct bpf_prog_aux *aux = fp->aux;
@@ -2421,7 +2426,7 @@ static int bpf_check_tail_call(const struct bpf_prog *fp) if (!map_type_contains_progs(map)) continue;
if (!bpf_prog_map_compatible(map, fp)) {
if (!__bpf_prog_map_compatible(map, fp)) { ret = -EINVAL; goto out;Does this change allow us to have a dev_bound BPF-prog that have tail-call BPF-progs that are not dev_bound?
The use-case is a dev_bound BPF-prog that reads e.g. HW vlan, store this in data_meta (or a per CPU array), and then tail-calls another BPF-prog that reads the data stored (from data_meta area). Maybe this is already supported before?
I think this patch allows a dev-bound program to run hw-metadata kfuncs and perform XDP_REDIRECT into a prog_array but you will not be able to read these info via hw-metadata kfuncs in a tail-call program since just dev-bound programs are currently allowed to do that (and you can't insert a dev-bound programs in BPF_MAP_TYPE_PROG_ARRAY).
Regards, Lorenzo
--Jesper
linux-kselftest-mirror@lists.linaro.org
-
Jesper Dangaard Brouer -
Lorenzo Bianconi -
Martin KaFai Lau -
Stanislav Fomichev -
Toke Høiland-Jørgensen