Re: [PATCH bpf v6 2/4] bpf: Fix UAF due to missing link type check in mprog

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Wed Jul 22, 2026 at 3:23 AM EDT, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
>
> In bpf_mprog_link, the code does not check the link->type first before
> dereferencing link->prog->type. This missing validation allows a user to
> pass an abnormal non-netkit or non-tcx link via relative_fd. If doing
> BPF_LINK_UPDATE on the abnormal link, it can trigger a UAF issue.
>
> CPU0                                      CPU1
> netkit_link_prog_attach
> bpf_mprog_attach
> bpf_mprog_tuple_relative
> bpf_mprog_link
>   /* non-netkit or non-tcx link */
>   link = bpf_link_get_from_fd(id_or_fd);
>                                           BPF_LINK_UPDATE on relative link
>                                           ...
>                                           old_prog = xchg(&link->link.prog, new_prog);
>                                           bpf_prog_put(old_prog);
>   if (type && link->prog->type != type) <-- trigger UAF
>
> The reason for the UAF is that each subsystem provides its own
> protection for link->prog. Since there is no cross subsystem protection
> (if not considering the RCU of prog tear down), dereferencing the prog
> of an anchor link that does not belong to the current subsystem is not
> safe: it may have been freed. Therefore, we need to validate link->type
> to reject foreign anchors.
>
> Fix this by strictly validating link->type in bpf_mprog_link against the
> expected link type. mprog APIs is also adjusted to accept and pass down
> the expected link type. Meanwhile, add a comment explaining that when
> ptype == UNSPEC in bpf_mprog_detach, it acts as a wildcard.
>
> Fixes: 053c8e1f235d ("bpf: Add generic attach/detach/query API for multi-progs")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Reviewed-by: Amery Hung <ameryhung@gmail.com>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>

Unless I'm missing something, at the very least there's no need for the
extra argument in detach() since we pass UNSPEC to bpf_mprog_link() and
never use it otherwise.

Even for the attach case, aren't we required to provide a new link
for attachment, and so already have the ltype available to test
against?

> ---
>  drivers/net/netkit.c      | 13 ++++++-------
>  include/linux/bpf_mprog.h |  6 ++++--
>  kernel/bpf/mprog.c        | 23 ++++++++++++++---------
>  kernel/bpf/tcx.c          | 13 ++++++-------
>  4 files changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
> index a3931cd82132..99ddf2befb23 100644
> --- a/drivers/net/netkit.c
> +++ b/drivers/net/netkit.c
> @@ -768,7 +768,7 @@ int netkit_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  	}
>  	ret = bpf_mprog_attach(entry, &entry_new, prog, NULL, replace_prog,
>  			       attr->attach_flags, attr->relative_fd,
> -			       attr->expected_revision);
> +			       attr->expected_revision, BPF_LINK_TYPE_NETKIT);
>  	if (!ret) {
>  		if (entry != entry_new) {
>  			netkit_entry_update(dev, entry_new);
> @@ -802,7 +802,7 @@ int netkit_prog_detach(const union bpf_attr *attr, struct bpf_prog *prog)
>  		goto out;
>  	}
>  	ret = bpf_mprog_detach(entry, &entry_new, prog, NULL, attr->attach_flags,
> -			       attr->relative_fd, attr->expected_revision);
> +			       attr->relative_fd, attr->expected_revision, BPF_LINK_TYPE_NETKIT);
>  	if (!ret) {
>  		if (!bpf_mprog_total(entry_new))
>  			entry_new = NULL;
> @@ -850,7 +850,7 @@ static int netkit_link_prog_attach(struct bpf_link *link, u32 flags,
>  	ASSERT_RTNL();
>  	entry = netkit_entry_fetch(dev, true);
>  	ret = bpf_mprog_attach(entry, &entry_new, link->prog, link, NULL, flags,
> -			       id_or_fd, revision);
> +			       id_or_fd, revision, BPF_LINK_TYPE_NETKIT);
>  	if (!ret) {
>  		if (entry != entry_new) {
>  			netkit_entry_update(dev, entry_new);
> @@ -877,7 +877,7 @@ static void netkit_link_release(struct bpf_link *link)
>  		ret = -ENOENT;
>  		goto out;
>  	}
> -	ret = bpf_mprog_detach(entry, &entry_new, link->prog, link, 0, 0, 0);
> +	ret = bpf_mprog_detach(entry, &entry_new, link->prog, link, 0, 0, 0, BPF_LINK_TYPE_NETKIT);
>  	if (!ret) {
>  		if (!bpf_mprog_total(entry_new))
>  			entry_new = NULL;
> @@ -919,9 +919,8 @@ static int netkit_link_update(struct bpf_link *link, struct bpf_prog *nprog,
>  		ret = -ENOENT;
>  		goto out;
>  	}
> -	ret = bpf_mprog_attach(entry, &entry_new, nprog, link, oprog,
> -			       BPF_F_REPLACE | BPF_F_ID,
> -			       link->prog->aux->id, 0);
> +	ret = bpf_mprog_attach(entry, &entry_new, nprog, link, oprog, BPF_F_REPLACE | BPF_F_ID,
> +			       link->prog->aux->id, 0, BPF_LINK_TYPE_NETKIT);
>  	if (!ret) {
>  		WARN_ON_ONCE(entry != entry_new);
>  		oprog = xchg(&link->prog, nprog);
> diff --git a/include/linux/bpf_mprog.h b/include/linux/bpf_mprog.h
> index 0b9f4caeeb0a..1fbe1a923968 100644
> --- a/include/linux/bpf_mprog.h
> +++ b/include/linux/bpf_mprog.h
> @@ -321,12 +321,14 @@ int bpf_mprog_attach(struct bpf_mprog_entry *entry,
>  		     struct bpf_mprog_entry **entry_new,
>  		     struct bpf_prog *prog_new, struct bpf_link *link,
>  		     struct bpf_prog *prog_old,
> -		     u32 flags, u32 id_or_fd, u64 revision);
> +		     u32 flags, u32 id_or_fd, u64 revision,
> +		     enum bpf_link_type expected_link_type);
>  
>  int bpf_mprog_detach(struct bpf_mprog_entry *entry,
>  		     struct bpf_mprog_entry **entry_new,
>  		     struct bpf_prog *prog, struct bpf_link *link,
> -		     u32 flags, u32 id_or_fd, u64 revision);
> +		     u32 flags, u32 id_or_fd, u64 revision,
> +		     enum bpf_link_type expected_link_type);
>  
>  int bpf_mprog_query(const union bpf_attr *attr, union bpf_attr __user *uattr,
>  		    struct bpf_mprog_entry *entry);
> diff --git a/kernel/bpf/mprog.c b/kernel/bpf/mprog.c
> index 1394168062e8..b4a1b35ff569 100644
> --- a/kernel/bpf/mprog.c
> +++ b/kernel/bpf/mprog.c
> @@ -6,7 +6,7 @@
>  
>  static int bpf_mprog_link(struct bpf_tuple *tuple,
>  			  u32 id_or_fd, u32 flags,
> -			  enum bpf_prog_type type)
> +			  enum bpf_link_type type)
>  {
>  	struct bpf_link *link = ERR_PTR(-EINVAL);
>  	bool id = flags & BPF_F_ID;
> @@ -17,7 +17,7 @@ static int bpf_mprog_link(struct bpf_tuple *tuple,
>  		link = bpf_link_get_from_fd(id_or_fd);
>  	if (IS_ERR(link))
>  		return PTR_ERR(link);
> -	if (type && link->prog->type != type) {
> +	if (type && link->type != type) {
>  		bpf_link_put(link);
>  		return -EINVAL;
>  	}
> @@ -52,21 +52,22 @@ static int bpf_mprog_prog(struct bpf_tuple *tuple,
>  
>  static int bpf_mprog_tuple_relative(struct bpf_tuple *tuple,
>  				    u32 id_or_fd, u32 flags,
> -				    enum bpf_prog_type type)
> +				    enum bpf_link_type ltype,
> +				    enum bpf_prog_type ptype)
>  {
>  	bool link = flags & BPF_F_LINK;
>  	bool id = flags & BPF_F_ID;
>  
>  	memset(tuple, 0, sizeof(*tuple));
>  	if (link)
> -		return bpf_mprog_link(tuple, id_or_fd, flags, type);
> +		return bpf_mprog_link(tuple, id_or_fd, flags, ltype);
>  	/* If no relevant flag is set and no id_or_fd was passed, then
>  	 * tuple link/prog is just NULLed. This is the case when before/
>  	 * after selects first/last position without passing fd.
>  	 */
>  	if (!id && !id_or_fd)
>  		return 0;
> -	return bpf_mprog_prog(tuple, id_or_fd, flags, type);
> +	return bpf_mprog_prog(tuple, id_or_fd, flags, ptype);
>  }
>  
>  static void bpf_mprog_tuple_put(struct bpf_tuple *tuple)
> @@ -226,7 +227,8 @@ int bpf_mprog_attach(struct bpf_mprog_entry *entry,
>  		     struct bpf_mprog_entry **entry_new,
>  		     struct bpf_prog *prog_new, struct bpf_link *link,
>  		     struct bpf_prog *prog_old,
> -		     u32 flags, u32 id_or_fd, u64 revision)
> +		     u32 flags, u32 id_or_fd, u64 revision,
> +		     enum bpf_link_type expected_link_type)
>  {
>  	struct bpf_tuple rtuple, ntuple = {
>  		.prog = prog_new,
> @@ -243,6 +245,7 @@ int bpf_mprog_attach(struct bpf_mprog_entry *entry,
>  		return -EEXIST;
>  	ret = bpf_mprog_tuple_relative(&rtuple, id_or_fd,
>  				       flags & ~BPF_F_REPLACE,
> +				       expected_link_type,
>  				       prog_new->type);
>  	if (ret)
>  		return ret;
> @@ -328,7 +331,8 @@ static int bpf_mprog_fetch(struct bpf_mprog_entry *entry,
>  int bpf_mprog_detach(struct bpf_mprog_entry *entry,
>  		     struct bpf_mprog_entry **entry_new,
>  		     struct bpf_prog *prog, struct bpf_link *link,
> -		     u32 flags, u32 id_or_fd, u64 revision)
> +		     u32 flags, u32 id_or_fd, u64 revision,
> +		     enum bpf_link_type expected_link_type)
>  {
>  	struct bpf_tuple rtuple, dtuple = {
>  		.prog = prog,
> @@ -343,8 +347,9 @@ int bpf_mprog_detach(struct bpf_mprog_entry *entry,
>  	if (!bpf_mprog_total(entry))
>  		return -ENOENT;
>  	ret = bpf_mprog_tuple_relative(&rtuple, id_or_fd, flags,
> -				       prog ? prog->type :
> -				       BPF_PROG_TYPE_UNSPEC);
> +				       expected_link_type,
> +				       /* Use UNSPEC as wildcard when prog is NULL */
> +				       prog ? prog->type : BPF_PROG_TYPE_UNSPEC);
>  	if (ret)
>  		return ret;
>  	if (dtuple.prog) {
> diff --git a/kernel/bpf/tcx.c b/kernel/bpf/tcx.c
> index 02db0113b8e7..f208cef13a98 100644
> --- a/kernel/bpf/tcx.c
> +++ b/kernel/bpf/tcx.c
> @@ -38,7 +38,7 @@ int tcx_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  	}
>  	ret = bpf_mprog_attach(entry, &entry_new, prog, NULL, replace_prog,
>  			       attr->attach_flags, attr->relative_fd,
> -			       attr->expected_revision);
> +			       attr->expected_revision, BPF_LINK_TYPE_TCX);
>  	if (!ret) {
>  		if (entry != entry_new) {
>  			tcx_entry_update(dev, entry_new, ingress);
> @@ -76,7 +76,7 @@ int tcx_prog_detach(const union bpf_attr *attr, struct bpf_prog *prog)
>  		goto out;
>  	}
>  	ret = bpf_mprog_detach(entry, &entry_new, prog, NULL, attr->attach_flags,
> -			       attr->relative_fd, attr->expected_revision);
> +			       attr->relative_fd, attr->expected_revision, BPF_LINK_TYPE_TCX);
>  	if (!ret) {
>  		if (!tcx_entry_is_active(entry_new))
>  			entry_new = NULL;
> @@ -152,7 +152,7 @@ static int tcx_link_prog_attach(struct bpf_link *link, u32 flags, u32 id_or_fd,
>  	if (!entry)
>  		return -ENOMEM;
>  	ret = bpf_mprog_attach(entry, &entry_new, link->prog, link, NULL, flags,
> -			       id_or_fd, revision);
> +			       id_or_fd, revision, BPF_LINK_TYPE_TCX);
>  	if (!ret) {
>  		if (entry != entry_new) {
>  			tcx_entry_update(dev, entry_new, ingress);
> @@ -183,7 +183,7 @@ static void tcx_link_release(struct bpf_link *link)
>  		ret = -ENOENT;
>  		goto out;
>  	}
> -	ret = bpf_mprog_detach(entry, &entry_new, link->prog, link, 0, 0, 0);
> +	ret = bpf_mprog_detach(entry, &entry_new, link->prog, link, 0, 0, 0, BPF_LINK_TYPE_TCX);
>  	if (!ret) {
>  		if (!tcx_entry_is_active(entry_new))
>  			entry_new = NULL;
> @@ -229,9 +229,8 @@ static int tcx_link_update(struct bpf_link *link, struct bpf_prog *nprog,
>  		ret = -ENOENT;
>  		goto out;
>  	}
> -	ret = bpf_mprog_attach(entry, &entry_new, nprog, link, oprog,
> -			       BPF_F_REPLACE | BPF_F_ID,
> -			       link->prog->aux->id, 0);
> +	ret = bpf_mprog_attach(entry, &entry_new, nprog, link, oprog, BPF_F_REPLACE | BPF_F_ID,
> +			       link->prog->aux->id, 0, BPF_LINK_TYPE_TCX);
>  	if (!ret) {
>  		WARN_ON_ONCE(entry != entry_new);
>  		oprog = xchg(&link->prog, nprog);