ndo_bpf() takes over the reference it is passed only on success, the
caller puts it back itself on failure. netvsc_xdp_set() takes that one
plus num_chn - 1 more for its channels, and the rollback after the VF
refuses the program clears the channels again, putting all num_chn of
them. dev_xdp_install() then puts the one it passed in a second time,
and the program can be freed while the fd which loaded it still points
at it.
The VF refuses a program netvsc has already committed to when the
program is single-buffer and the VF has header-data split enabled, when
the VF has a memory provider bound, or when the VF's driver has
conditions of its own.
The rollback clears the channels rather than putting back the program
they ran before, too. A refused replace or removal then leaves the
synthetic path running no program at all, while the core and the VF
keep the old one. Put the old program back instead.
Reported by Sashiko during core rework. Unverified and untested.
Cc: stable+noautosel@kernel.org # untested fix to unlikely driver error path
Fixes: 184367dce4f7 ("hv_netvsc: Fix XDP refcnt for synthetic and VF NICs")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
- put the old program back on the channels rather than clearing them
(Kameron)
- reword the comment on the extra reference, it's the caller's, which
isn't always the core
- Tested-by tags from v1 not carried over, the rollback changed
v1: https://lore.kernel.org/20261001014131.310771-2-kuba@kernel.org (local)
---
drivers/net/hyperv/netvsc_bpf.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index 1dd3755d9e6d..fcc60729384e 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -197,6 +197,7 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
struct net_device *vf_netdev = rtnl_dereference(ndevctx->vf_netdev);
struct netlink_ext_ack *extack = bpf->extack;
+ struct bpf_prog *old_prog;
int ret;
if (!nvdev || nvdev->destroy) {@@ -205,10 +206,17 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
switch (bpf->command) {
case XDP_SETUP_PROG:
+ /* Hold on to the old program, to put it back on the channels
+ * if the VF refuses the change.
+ */
+ old_prog = netvsc_xdp_get(nvdev);
+ if (old_prog)
+ bpf_prog_inc(old_prog);
+
ret = netvsc_xdp_set(dev, bpf->prog, extack, nvdev);
if (ret)
- return ret;
+ goto put_old_prog;
ret = netvsc_vf_setxdp(vf_netdev, bpf->prog);
@@ -216,9 +224,22 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
netdev_err(dev, "vf_setxdp failed:%d\n", ret);
NL_SET_ERR_MSG_MOD(extack, "vf_setxdp failed");
- netvsc_xdp_set(dev, NULL, extack, nvdev);
+ /* The caller hands us its reference to bpf->prog only
+ * on success, take one so that putting the num_chn
+ * channel references below doesn't consume it.
+ */
+ if (bpf->prog)
+ bpf_prog_inc(bpf->prog);
+ /* Can't fail, old_prog has passed the same checks
+ * before. The channels take over our reference to it.
+ */
+ netvsc_xdp_set(dev, old_prog, extack, nvdev);
+ return ret;
}
+put_old_prog:
+ if (old_prog)
+ bpf_prog_put(old_prog);
return ret;
default:
--
2.56.0