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

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

On Mon Jul 27, 2026 at 9:56 PM EDT, Pu Lehui wrote:
> Hi Emil,
>
> Gentle ping~, is that acceptable to you?

Hi Pu,

Sorry for the late reply. The updated patch looks way nicer,
imo we should go with it:

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

>
> On 2026/7/23 11:14, Pu Lehui wrote:
>> 
>> On 2026/7/23 0:25, Emil Tsalapatis wrote:
>>> 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?
>>>
>> Hi Emil,
>> 
>> The BPF_F_LINK flag only indicates that relative_fd refers to an anchor 
>> link or prog; it is independent of whether the object being attached or 
>> detached is a bare prog or a link. This means that when attaching or 
>> detaching a bare prog relative to an anchor link, we cannot obtain the 
>> expected_link_type because the link parameter passed in is NULL.
>> 
>> Furthermore, as you mentioned, we should support unconditional 
>> detachment. But in the current version, because we strictly check 
>> expected_link_type, the detach operation will fail if the relative link 
>> type does not match.
>> 
>> Therefore, we can simplify the approach to fix this UAF—similar to patch 
>> 3—by relying on RCU protection when accessing the prog type of a 
>> relative link. When attaching or detaching a bare prog or a link 
>> relative to an anchor link, we simply verify that 
>> relative_link->prog->type matches the bare prog->type or 
>> link->prog->type. This logic also naturally applies to unconditional 
>> detachment with relative link, such as when the bare prog or link->prog 
>> being detached is NULL.
>> 
>> And the patch will be follow, wdyt?
>> 
>> diff --git a/kernel/bpf/mprog.c b/kernel/bpf/mprog.c
>> index 1394168062e8..512821c21aa6 100644
>> --- a/kernel/bpf/mprog.c
>> +++ b/kernel/bpf/mprog.c
>> @@ -10,6 +10,8 @@ static int bpf_mprog_link(struct bpf_tuple *tuple,
>>   {
>>       struct bpf_link *link = ERR_PTR(-EINVAL);
>>       bool id = flags & BPF_F_ID;
>> +    bool type_mismatch = false;
>> +    struct bpf_prog *prog;
>> 
>>       if (id)
>>           link = bpf_link_by_id(id_or_fd);
>> @@ -17,13 +19,20 @@ 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) {
>> +
>> +    rcu_read_lock();
>> +    prog = READ_ONCE(link->prog); <-- relative_link->prog
>> +    if (!prog || (type && prog->type != type))
>> +        type_mismatch = true;
>> +    rcu_read_unlock();
>> +
>> +    if (type_mismatch) {
>>           bpf_link_put(link);
>>           return -EINVAL;
>>       }
>> 
>>       tuple->link = link;
>> -    tuple->prog = link->prog;
>> +    tuple->prog = prog;
>>       return 0;
>>   }
>> 
>>>> ---
>>>>   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);
>>