On Sat, Oct 24, 2026 at 08:36 AM Leon Hwang <[email protected]> wrote:
> Instead of passing bpf log or extack to bpf_xdp_link_attach(), pass an
> errmsg pointer to receive the kstrdup() error msg, because
[...]
> 2) passing extack will access underlying driver string out of dev_put()
> guard, which will crash kernel when the device is unloaded
> concurrently.
Netlink does exactly that.
rtnl_setlink() looks up the dev with __dev_get_by_index(), so no refcnt,
do_setlink() -> dev_change_xdp_fd(dev, extack, ...) calls into the same
drivers that set the same strings, rtnl is unlocked, and only then
netlink_rcv_skb() -> netlink_ack() does strlen(extack->_msg).
All extack users work this way. kstrdup() in one of them fixes nothing.
Instead of patches 1 and 2:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c0ae442..6f7fec2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4440,7 +4440,8 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff
*skb, struct net_device *d
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device
*dev,
struct netdev_queue *txq, int *ret);
-int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
+int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack);
u8 dev_xdp_prog_count(struct net_device *dev);
int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
int dev_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 90bc723..0ce75ee 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5862,8 +5862,12 @@ err_put:
}
#define BPF_LINK_CREATE_LAST_FIELD link_create.uprobe_multi.path_fd
-static int link_create(union bpf_attr *attr, bpfptr_t uattr)
+static int link_create(union bpf_attr *attr, bpfptr_t uattr, struct
bpf_common_attr *attr_common,
+ bpfptr_t uattr_common, u32 size_common)
{
+ struct netlink_ext_ack extack = {};
+ struct bpf_verifier_log *log;
+ struct bpf_log_attr attr_log;
struct bpf_prog *prog;
int ret;
@@ -5873,14 +5877,24 @@ static int link_create(union bpf_attr *attr, bpfptr_t
uattr)
if (attr->link_create.attach_type == BPF_STRUCT_OPS)
return bpf_struct_ops_link_create(attr);
+ log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common,
size_common);
+ if (IS_ERR(log))
+ return PTR_ERR(log);
+
prog = bpf_prog_get(attr->link_create.prog_fd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
+ if (IS_ERR(prog)) {
+ bpf_log(log, "Invalid prog_fd.\n");
+ ret = PTR_ERR(prog);
+ prog = NULL;
+ goto out;
+ }
ret = bpf_prog_attach_check_attach_type(prog,
attr->link_create.attach_type);
- if (ret)
+ if (ret) {
+ bpf_log(log, "Invalid attach_type.\n");
goto out;
+ }
switch (prog->type) {
case BPF_PROG_TYPE_CGROUP_SKB:
@@ -5931,7 +5945,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t
uattr)
break;
#ifdef CONFIG_NET
case BPF_PROG_TYPE_XDP:
- ret = bpf_xdp_link_attach(attr, prog);
+ ret = bpf_xdp_link_attach(attr, prog, &extack);
break;
case BPF_PROG_TYPE_SCHED_CLS:
if (attr->link_create.attach_type == BPF_TCX_INGRESS ||
@@ -5959,12 +5973,22 @@ static int link_create(union bpf_attr *attr, bpfptr_t
uattr)
ret = bpf_uprobe_multi_link_attach(attr, prog);
break;
default:
+ bpf_log(log, "Invalid prog type.\n");
ret = -EINVAL;
}
out:
- if (ret < 0)
- bpf_prog_put(prog);
+ if (ret < 0) {
+ int err;
+
+ if (extack._msg)
+ bpf_log(log, "%s\n", extack._msg);
+ err = bpf_log_attr_finalize(&attr_log, log);
+ ret = err ? err : ret;
+ if (prog)
+ bpf_prog_put(prog);
+ }
+ kfree(log);
return ret;
}
@@ -6536,7 +6560,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr,
unsigned int size,
err = bpf_map_do_batch(&attr, uattr.user, BPF_MAP_DELETE_BATCH);
break;
case BPF_LINK_CREATE:
- err = link_create(&attr, uattr);
+ err = link_create(&attr, uattr, &attr_common, uattr_common,
size_common);
break;
case BPF_LINK_UPDATE:
err = link_update(&attr);
diff --git a/net/core/dev.c b/net/core/dev.c
index 1a09e78..4aa5525 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10882,11 +10882,11 @@ static const struct bpf_link_ops bpf_xdp_link_lops = {
.update_prog = bpf_xdp_link_update,
};
-int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack)
{
struct net *net = current->nsproxy->net_ns;
struct bpf_link_primer link_primer;
- struct netlink_ext_ack extack = {};
struct bpf_xdp_link *link;
struct net_device *dev;
int err, fd;
@@ -10895,6 +10895,7 @@ int bpf_xdp_link_attach(const union bpf_attr *attr,
struct bpf_prog *prog)
dev = dev_get_by_index(net, attr->link_create.target_ifindex);
if (!dev) {
rtnl_unlock();
+ NL_SET_ERR_MSG(extack, "Invalid target_ifindex.");
return -EINVAL;
}
@@ -10916,14 +10917,14 @@ int bpf_xdp_link_attach(const union bpf_attr *attr,
struct bpf_prog *prog)
}
netdev_lock_ops(dev);
- err = dev_xdp_attach_link(dev, &extack, link);
+ err = dev_xdp_attach_link(dev, extack, link);
netdev_unlock_ops(dev);
rtnl_unlock();
if (err) {
link->dev = NULL;
bpf_link_cleanup(&link_primer);
- trace_bpf_xdp_link_attach_failed(extack._msg);
+ trace_bpf_xdp_link_attach_failed(extack->_msg);
goto out_put_dev;
}
I'm think I gave this suggestion earlier. Why did you go with strdup()?
pw-bot: cr
