| Message ID | 20221220222043.3348718-9-sdf@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | xdp: hints via kfuncs | expand |
On 12/20/22 2:20 PM, Stanislav Fomichev wrote: > diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c > index 0e3fc743e0a8..60978a1f9baa 100644 > --- a/kernel/bpf/offload.c > +++ b/kernel/bpf/offload.c > @@ -187,17 +187,13 @@ static void __bpf_offload_dev_netdev_unregister(struct bpf_offload_dev *offdev, > kfree(ondev); > } > > -int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > +static int __bpf_prog_dev_bound_init(struct bpf_prog *prog, struct net_device *netdev) > { > struct bpf_offload_netdev *ondev; > struct bpf_prog_offload *offload; > int err; > > - if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && > - attr->prog_type != BPF_PROG_TYPE_XDP) > - return -EINVAL; > - > - if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) > + if (!netdev) Is this !netdev test needed? > return -EINVAL; > > offload = kzalloc(sizeof(*offload), GFP_USER); > @@ -205,21 +201,13 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > return -ENOMEM; > > offload->prog = prog; > + offload->netdev = netdev; > > - offload->netdev = dev_get_by_index(current->nsproxy->net_ns, > - attr->prog_ifindex); > - err = bpf_dev_offload_check(offload->netdev); > - if (err) > - goto err_maybe_put; > - > - prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); > - > - down_write(&bpf_devs_lock); > ondev = bpf_offload_find_netdev(offload->netdev); > if (!ondev) { > if (bpf_prog_is_offloaded(prog->aux)) { > err = -EINVAL; > - goto err_unlock; > + goto err_free; > } > > /* When only binding to the device, explicitly > @@ -227,25 +215,80 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > */ > err = __bpf_offload_dev_netdev_register(NULL, offload->netdev); > if (err) > - goto err_unlock; > + goto err_free; > ondev = bpf_offload_find_netdev(offload->netdev); > } > offload->offdev = ondev->offdev; > prog->aux->offload = offload; > list_add_tail(&offload->offloads, &ondev->progs); > - dev_put(offload->netdev); > - up_write(&bpf_devs_lock); > > return 0; > -err_unlock: > - up_write(&bpf_devs_lock); > -err_maybe_put: > - if (offload->netdev) > - dev_put(offload->netdev); > +err_free: > kfree(offload); > return err; > } > > +int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > +{ > + struct net_device *netdev; > + int err; > + > + if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && > + attr->prog_type != BPF_PROG_TYPE_XDP) > + return -EINVAL; > + > + if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) > + return -EINVAL; > + > + netdev = dev_get_by_index(current->nsproxy->net_ns, attr->prog_ifindex); > + if (!netdev) > + return -EINVAL; > + > + down_write(&bpf_devs_lock); > + err = bpf_dev_offload_check(netdev); > + if (err) > + goto out; > + > + prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); nit. move the bpf_dev_offload_check() and offload_requested assignment out. I don't think they need lock protection so that it is clear what the lock is protecting in the future reading. It seems the original code have them outside also. > + > + err = __bpf_prog_dev_bound_init(prog, netdev); > + if (err) > + goto out; nit. goto can be saved. > + > +out: > + dev_put(netdev); > + up_write(&bpf_devs_lock); > + return err; > +} > + > +int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, struct bpf_prog *old_prog) > +{ > + int err; > + > + if (!bpf_prog_is_dev_bound(old_prog->aux)) > + return 0; > + > + if (bpf_prog_is_offloaded(old_prog->aux)) > + return -EINVAL; > + > + down_write(&bpf_devs_lock); > + if (!old_prog->aux->offload) { > + err = -EINVAL; > + goto out; > + } > + > + new_prog->aux->dev_bound = old_prog->aux->dev_bound; > + new_prog->aux->offload_requested = old_prog->aux->offload_requested; nit. Same here, I think the initialization can be moved outside of the lock. > + > + err = __bpf_prog_dev_bound_init(new_prog, old_prog->aux->offload->netdev); > + if (err) > + goto out; goto can be saved. > + > +out: > + up_write(&bpf_devs_lock); > + return err; > +} > + > int bpf_prog_offload_verifier_prep(struct bpf_prog *prog) > { > struct bpf_prog_offload *offload; > @@ -687,6 +730,22 @@ bool bpf_offload_dev_match(struct bpf_prog *prog, struct net_device *netdev) > } > EXPORT_SYMBOL_GPL(bpf_offload_dev_match); > > +bool bpf_prog_dev_bound_match(struct bpf_prog *lhs, struct bpf_prog *rhs) > +{ > + bool ret; > + > + if (bpf_prog_is_offloaded(lhs->aux) != bpf_prog_is_offloaded(rhs->aux)) > + return false; > + > + down_read(&bpf_devs_lock); > + ret = lhs->aux->offload && rhs->aux->offload && > + lhs->aux->offload->netdev && > + lhs->aux->offload->netdev == rhs->aux->offload->netdev; > + up_read(&bpf_devs_lock); > + > + return ret; > +} > + > bool bpf_offload_prog_map_match(struct bpf_prog *prog, struct bpf_map *map) > { > struct bpf_offloaded_map *offmap; > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 11c558be4992..64a68e8fb072 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -2605,6 +2605,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) > goto free_prog_sec; > } > > + if (type == BPF_PROG_TYPE_EXT && dst_prog) { Does it also need to test the bpf_prog_is_dev_bound(dst_prog->aux)? Otherwise, the bpf_prog_dev_bound_inherit() below will fail on everything for !CONFIG_NET. > + err = bpf_prog_dev_bound_inherit(prog, dst_prog); > + if (err) > + goto free_prog_sec; > + } > + > /* find program type: socket_filter vs tracing_filter */ > err = find_prog_type(type, prog); > if (err < 0) > @@ -3021,6 +3027,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > goto out_put_prog; > } > > + if (bpf_prog_is_dev_bound(prog->aux) && Like here. > + !bpf_prog_dev_bound_match(prog, tgt_prog)) { > + err = -EINVAL; > + goto out_put_prog; > + } > + > key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id); > } > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 320451a0be3e..64f4d2b5824f 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -16537,11 +16537,6 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > if (tgt_prog) { > struct bpf_prog_aux *aux = tgt_prog->aux; > > - if (bpf_prog_is_dev_bound(tgt_prog->aux)) { > - bpf_log(log, "Replacing device-bound programs not supported\n"); > - return -EINVAL; > - } > - > for (i = 0; i < aux->func_info_cnt; i++) > if (aux->func_info[i].type_id == btf_id) { > subprog = i;
On Thu, Dec 22, 2022 at 4:37 PM Martin KaFai Lau <martin.lau@linux.dev> wrote: > > On 12/20/22 2:20 PM, Stanislav Fomichev wrote: > > diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c > > index 0e3fc743e0a8..60978a1f9baa 100644 > > --- a/kernel/bpf/offload.c > > +++ b/kernel/bpf/offload.c > > @@ -187,17 +187,13 @@ static void __bpf_offload_dev_netdev_unregister(struct bpf_offload_dev *offdev, > > kfree(ondev); > > } > > > > -int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > > +static int __bpf_prog_dev_bound_init(struct bpf_prog *prog, struct net_device *netdev) > > { > > struct bpf_offload_netdev *ondev; > > struct bpf_prog_offload *offload; > > int err; > > > > - if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && > > - attr->prog_type != BPF_PROG_TYPE_XDP) > > - return -EINVAL; > > - > > - if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) > > + if (!netdev) > > Is this !netdev test needed? Seems safe to drop. _inherit has a 'old_prog->aux->offload' check and _init has a check after dev_get_by_index. > > return -EINVAL; > > > > offload = kzalloc(sizeof(*offload), GFP_USER); > > @@ -205,21 +201,13 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > > return -ENOMEM; > > > > offload->prog = prog; > > + offload->netdev = netdev; > > > > - offload->netdev = dev_get_by_index(current->nsproxy->net_ns, > > - attr->prog_ifindex); > > - err = bpf_dev_offload_check(offload->netdev); > > - if (err) > > - goto err_maybe_put; > > - > > - prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); > > - > > - down_write(&bpf_devs_lock); > > ondev = bpf_offload_find_netdev(offload->netdev); > > if (!ondev) { > > if (bpf_prog_is_offloaded(prog->aux)) { > > err = -EINVAL; > > - goto err_unlock; > > + goto err_free; > > } > > > > /* When only binding to the device, explicitly > > @@ -227,25 +215,80 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > > */ > > err = __bpf_offload_dev_netdev_register(NULL, offload->netdev); > > if (err) > > - goto err_unlock; > > + goto err_free; > > ondev = bpf_offload_find_netdev(offload->netdev); > > } > > offload->offdev = ondev->offdev; > > prog->aux->offload = offload; > > list_add_tail(&offload->offloads, &ondev->progs); > > - dev_put(offload->netdev); > > - up_write(&bpf_devs_lock); > > > > return 0; > > -err_unlock: > > - up_write(&bpf_devs_lock); > > -err_maybe_put: > > - if (offload->netdev) > > - dev_put(offload->netdev); > > +err_free: > > kfree(offload); > > return err; > > } > > > > +int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) > > +{ > > + struct net_device *netdev; > > + int err; > > + > > + if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && > > + attr->prog_type != BPF_PROG_TYPE_XDP) > > + return -EINVAL; > > + > > + if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) > > + return -EINVAL; > > + > > + netdev = dev_get_by_index(current->nsproxy->net_ns, attr->prog_ifindex); > > + if (!netdev) > > + return -EINVAL; > > + > > + down_write(&bpf_devs_lock); > > + err = bpf_dev_offload_check(netdev); > > + if (err) > > + goto out; > > + > > + prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); > > nit. move the bpf_dev_offload_check() and offload_requested assignment out. I > don't think they need lock protection so that it is clear what the lock is > protecting in the future reading. It seems the original code have them outside > also. Sure. > > + > > + err = __bpf_prog_dev_bound_init(prog, netdev); > > + if (err) > > + goto out; > > nit. goto can be saved. Ack, will drop here; although, will still keep the goto above for dev_put(netdev). > > + > > +out: > > + dev_put(netdev); > > + up_write(&bpf_devs_lock); > > + return err; > > +} > > + > > +int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, struct bpf_prog *old_prog) > > +{ > > + int err; > > + > > + if (!bpf_prog_is_dev_bound(old_prog->aux)) > > + return 0; > > + > > + if (bpf_prog_is_offloaded(old_prog->aux)) > > + return -EINVAL; > > + > > + down_write(&bpf_devs_lock); > > + if (!old_prog->aux->offload) { > > + err = -EINVAL; > > + goto out; > > + } > > + > > + new_prog->aux->dev_bound = old_prog->aux->dev_bound; > > + new_prog->aux->offload_requested = old_prog->aux->offload_requested; > > nit. Same here, I think the initialization can be moved outside of the lock. Agreed. Seems like this will cause bpf_prog_dev_bound_destroy to be called when we return with an error below; but seems safe since we're also doing an 'aux->offload' check in there. > > + > > + err = __bpf_prog_dev_bound_init(new_prog, old_prog->aux->offload->netdev); > > + if (err) > > + goto out; > > goto can be saved. Thx. > > + > > +out: > > + up_write(&bpf_devs_lock); > > + return err; > > +} > > + > > int bpf_prog_offload_verifier_prep(struct bpf_prog *prog) > > { > > struct bpf_prog_offload *offload; > > @@ -687,6 +730,22 @@ bool bpf_offload_dev_match(struct bpf_prog *prog, struct net_device *netdev) > > } > > EXPORT_SYMBOL_GPL(bpf_offload_dev_match); > > > > +bool bpf_prog_dev_bound_match(struct bpf_prog *lhs, struct bpf_prog *rhs) > > +{ > > + bool ret; > > + > > + if (bpf_prog_is_offloaded(lhs->aux) != bpf_prog_is_offloaded(rhs->aux)) > > + return false; > > + > > + down_read(&bpf_devs_lock); > > + ret = lhs->aux->offload && rhs->aux->offload && > > + lhs->aux->offload->netdev && > > + lhs->aux->offload->netdev == rhs->aux->offload->netdev; > > + up_read(&bpf_devs_lock); > > + > > + return ret; > > +} > > + > > bool bpf_offload_prog_map_match(struct bpf_prog *prog, struct bpf_map *map) > > { > > struct bpf_offloaded_map *offmap; > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 11c558be4992..64a68e8fb072 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -2605,6 +2605,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) > > goto free_prog_sec; > > } > > > > + if (type == BPF_PROG_TYPE_EXT && dst_prog) { > > Does it also need to test the bpf_prog_is_dev_bound(dst_prog->aux)? Otherwise, > the bpf_prog_dev_bound_inherit() below will fail on everything for !CONFIG_NET. We do the following in bpf_prog_dev_bound_inherit which should be enough? if (!bpf_prog_is_dev_bound(old_prog->aux)) return 0; Or am I missing something? > > + err = bpf_prog_dev_bound_inherit(prog, dst_prog); > > + if (err) > > + goto free_prog_sec; > > + } > > + > > /* find program type: socket_filter vs tracing_filter */ > > err = find_prog_type(type, prog); > > if (err < 0) > > @@ -3021,6 +3027,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > > goto out_put_prog; > > } > > > > + if (bpf_prog_is_dev_bound(prog->aux) && > > Like here. > > > + !bpf_prog_dev_bound_match(prog, tgt_prog)) { > > + err = -EINVAL; > > + goto out_put_prog; > > + } > > + > > key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id); > > } > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index 320451a0be3e..64f4d2b5824f 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -16537,11 +16537,6 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, > > if (tgt_prog) { > > struct bpf_prog_aux *aux = tgt_prog->aux; > > > > - if (bpf_prog_is_dev_bound(tgt_prog->aux)) { > > - bpf_log(log, "Replacing device-bound programs not supported\n"); > > - return -EINVAL; > > - } > > - > > for (i = 0; i < aux->func_info_cnt; i++) > > if (aux->func_info[i].type_id == btf_id) { > > subprog = i; >
On 12/22/22 8:06 PM, Stanislav Fomichev wrote: >>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>> index 11c558be4992..64a68e8fb072 100644 >>> --- a/kernel/bpf/syscall.c >>> +++ b/kernel/bpf/syscall.c >>> @@ -2605,6 +2605,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) >>> goto free_prog_sec; >>> } >>> >>> + if (type == BPF_PROG_TYPE_EXT && dst_prog) { >> Does it also need to test the bpf_prog_is_dev_bound(dst_prog->aux)? Otherwise, >> the bpf_prog_dev_bound_inherit() below will fail on everything for !CONFIG_NET. > We do the following in bpf_prog_dev_bound_inherit which should be enough? > > if (!bpf_prog_is_dev_bound(old_prog->aux)) > return 0; > > Or am I missing something? The inline one in include/linux/bpf.h will be called instead when CONFIG_NET is not set: static inline int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, struct bpf_prog *old_prog) { return -EOPNOTSUPP; }
On Tue, Jan 3, 2023 at 5:51 PM Martin KaFai Lau <martin.lau@linux.dev> wrote: > > On 12/22/22 8:06 PM, Stanislav Fomichev wrote: > >>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > >>> index 11c558be4992..64a68e8fb072 100644 > >>> --- a/kernel/bpf/syscall.c > >>> +++ b/kernel/bpf/syscall.c > >>> @@ -2605,6 +2605,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) > >>> goto free_prog_sec; > >>> } > >>> > >>> + if (type == BPF_PROG_TYPE_EXT && dst_prog) { > >> Does it also need to test the bpf_prog_is_dev_bound(dst_prog->aux)? Otherwise, > >> the bpf_prog_dev_bound_inherit() below will fail on everything for !CONFIG_NET. > > We do the following in bpf_prog_dev_bound_inherit which should be enough? > > > > if (!bpf_prog_is_dev_bound(old_prog->aux)) > > return 0; > > > > Or am I missing something? > > The inline one in include/linux/bpf.h will be called instead when CONFIG_NET is > not set: > > static inline int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, > struct bpf_prog *old_prog) > { > return -EOPNOTSUPP; > } Ah, I totally missed the fact you were talking about !CONFIG_NET, thanks for clarifying. Yeah, will add that extra bpf_prog_is_dev_bound(dst_prog->aux) you've mentioned, thx!
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 969f53691dd4..f1bacd8ee402 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2482,6 +2482,7 @@ void unpriv_ebpf_notify(int new_state); #if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL) void *bpf_dev_bound_resolve_kfunc(struct bpf_prog *prog, u32 func_id); int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr); +int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, struct bpf_prog *old_prog); void bpf_dev_bound_netdev_unregister(struct net_device *dev); static inline bool bpf_prog_is_dev_bound(const struct bpf_prog_aux *aux) @@ -2494,6 +2495,8 @@ static inline bool bpf_prog_is_offloaded(const struct bpf_prog_aux *aux) return aux->offload_requested; } +bool bpf_prog_dev_bound_match(struct bpf_prog *lhs, struct bpf_prog *rhs); + static inline bool bpf_map_is_offloaded(struct bpf_map *map) { return unlikely(map->ops == &bpf_map_offload_ops); @@ -2527,6 +2530,12 @@ static inline int bpf_prog_dev_bound_init(struct bpf_prog *prog, return -EOPNOTSUPP; } +static inline int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, + struct bpf_prog *old_prog) +{ + return -EOPNOTSUPP; +} + static inline void bpf_dev_bound_netdev_unregister(struct net_device *dev) { } @@ -2541,6 +2550,11 @@ static inline bool bpf_prog_is_offloaded(struct bpf_prog_aux *aux) return false; } +static inline bool bpf_prog_dev_bound_match(struct bpf_prog *lhs, struct bpf_prog *rhs) +{ + return false; +} + static inline bool bpf_map_is_offloaded(struct bpf_map *map) { return false; diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c index 0e3fc743e0a8..60978a1f9baa 100644 --- a/kernel/bpf/offload.c +++ b/kernel/bpf/offload.c @@ -187,17 +187,13 @@ static void __bpf_offload_dev_netdev_unregister(struct bpf_offload_dev *offdev, kfree(ondev); } -int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) +static int __bpf_prog_dev_bound_init(struct bpf_prog *prog, struct net_device *netdev) { struct bpf_offload_netdev *ondev; struct bpf_prog_offload *offload; int err; - if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && - attr->prog_type != BPF_PROG_TYPE_XDP) - return -EINVAL; - - if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) + if (!netdev) return -EINVAL; offload = kzalloc(sizeof(*offload), GFP_USER); @@ -205,21 +201,13 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) return -ENOMEM; offload->prog = prog; + offload->netdev = netdev; - offload->netdev = dev_get_by_index(current->nsproxy->net_ns, - attr->prog_ifindex); - err = bpf_dev_offload_check(offload->netdev); - if (err) - goto err_maybe_put; - - prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); - - down_write(&bpf_devs_lock); ondev = bpf_offload_find_netdev(offload->netdev); if (!ondev) { if (bpf_prog_is_offloaded(prog->aux)) { err = -EINVAL; - goto err_unlock; + goto err_free; } /* When only binding to the device, explicitly @@ -227,25 +215,80 @@ int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) */ err = __bpf_offload_dev_netdev_register(NULL, offload->netdev); if (err) - goto err_unlock; + goto err_free; ondev = bpf_offload_find_netdev(offload->netdev); } offload->offdev = ondev->offdev; prog->aux->offload = offload; list_add_tail(&offload->offloads, &ondev->progs); - dev_put(offload->netdev); - up_write(&bpf_devs_lock); return 0; -err_unlock: - up_write(&bpf_devs_lock); -err_maybe_put: - if (offload->netdev) - dev_put(offload->netdev); +err_free: kfree(offload); return err; } +int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr) +{ + struct net_device *netdev; + int err; + + if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS && + attr->prog_type != BPF_PROG_TYPE_XDP) + return -EINVAL; + + if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY) + return -EINVAL; + + netdev = dev_get_by_index(current->nsproxy->net_ns, attr->prog_ifindex); + if (!netdev) + return -EINVAL; + + down_write(&bpf_devs_lock); + err = bpf_dev_offload_check(netdev); + if (err) + goto out; + + prog->aux->offload_requested = !(attr->prog_flags & BPF_F_XDP_DEV_BOUND_ONLY); + + err = __bpf_prog_dev_bound_init(prog, netdev); + if (err) + goto out; + +out: + dev_put(netdev); + up_write(&bpf_devs_lock); + return err; +} + +int bpf_prog_dev_bound_inherit(struct bpf_prog *new_prog, struct bpf_prog *old_prog) +{ + int err; + + if (!bpf_prog_is_dev_bound(old_prog->aux)) + return 0; + + if (bpf_prog_is_offloaded(old_prog->aux)) + return -EINVAL; + + down_write(&bpf_devs_lock); + if (!old_prog->aux->offload) { + err = -EINVAL; + goto out; + } + + new_prog->aux->dev_bound = old_prog->aux->dev_bound; + new_prog->aux->offload_requested = old_prog->aux->offload_requested; + + err = __bpf_prog_dev_bound_init(new_prog, old_prog->aux->offload->netdev); + if (err) + goto out; + +out: + up_write(&bpf_devs_lock); + return err; +} + int bpf_prog_offload_verifier_prep(struct bpf_prog *prog) { struct bpf_prog_offload *offload; @@ -687,6 +730,22 @@ bool bpf_offload_dev_match(struct bpf_prog *prog, struct net_device *netdev) } EXPORT_SYMBOL_GPL(bpf_offload_dev_match); +bool bpf_prog_dev_bound_match(struct bpf_prog *lhs, struct bpf_prog *rhs) +{ + bool ret; + + if (bpf_prog_is_offloaded(lhs->aux) != bpf_prog_is_offloaded(rhs->aux)) + return false; + + down_read(&bpf_devs_lock); + ret = lhs->aux->offload && rhs->aux->offload && + lhs->aux->offload->netdev && + lhs->aux->offload->netdev == rhs->aux->offload->netdev; + up_read(&bpf_devs_lock); + + return ret; +} + bool bpf_offload_prog_map_match(struct bpf_prog *prog, struct bpf_map *map) { struct bpf_offloaded_map *offmap; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 11c558be4992..64a68e8fb072 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2605,6 +2605,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr) goto free_prog_sec; } + if (type == BPF_PROG_TYPE_EXT && dst_prog) { + err = bpf_prog_dev_bound_inherit(prog, dst_prog); + if (err) + goto free_prog_sec; + } + /* find program type: socket_filter vs tracing_filter */ err = find_prog_type(type, prog); if (err < 0) @@ -3021,6 +3027,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, goto out_put_prog; } + if (bpf_prog_is_dev_bound(prog->aux) && + !bpf_prog_dev_bound_match(prog, tgt_prog)) { + err = -EINVAL; + goto out_put_prog; + } + key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id); } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 320451a0be3e..64f4d2b5824f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16537,11 +16537,6 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, if (tgt_prog) { struct bpf_prog_aux *aux = tgt_prog->aux; - if (bpf_prog_is_dev_bound(tgt_prog->aux)) { - bpf_log(log, "Replacing device-bound programs not supported\n"); - return -EINVAL; - } - for (i = 0; i < aux->func_info_cnt; i++) if (aux->func_info[i].type_id == btf_id) { subprog = i;