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