Re: [PATCH bpf 2/2] bpf, netkit: reject offloaded programs on attach

Jiayuan Chen <[email protected]>
Newsgroups dev.linux.lists.sashiko,org.kernel.vger.bpf
Message-ID <[email protected]>
On 4/24/26 11:37 AM, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [High] The patch prevents attaching offloaded programs during link creation and program attach, but fails to secure the link update path, allowing a complete bypass of the fix.
> --
>
> commit 4b4a6a3785c138df3a0a76f92652aa68a0ea77d6
> Author: Jiayuan Chen <[email protected]>
>
> bpf, netkit: reject offloaded programs on attach
>
> This commit rejects offloaded BPF programs in netkit_prog_attach() and
> netkit_link_attach() to prevent them from triggering a WARN in
> bpf_prog_warn_on_exec() when executed on the software path.
>
>> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
>> index 7b56a7ad7a496..d845a24c5915d 100644
>> --- a/drivers/net/netkit.c
>> +++ b/drivers/net/netkit.c
> [ ... ]
>
>> @@ -1000,6 +1003,9 @@ int netkit_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>>   	struct net_device *dev;
>>   	int ret;
>>   
>> +	if (bpf_prog_is_offloaded(prog->aux))
>> +		return -EINVAL;
>> +
> Is it also necessary to reject offloaded programs in netkit_link_update()?
>
> It looks like a user could bypass this fix by creating a netkit link with a
> non-offloaded program, and then using BPF_LINK_UPDATE to replace it with an
> offloaded program.
>
> The netkit_link_update() callback directly passes the new program to
> bpf_mprog_attach() without checking bpf_prog_is_offloaded():
>
> static int netkit_link_update(struct bpf_link *link, struct bpf_prog *nprog,
> 			      struct bpf_prog *oprog)
> {
> 	...
> 	ret = bpf_mprog_attach(entry, &entry_new, nprog, link, oprog,
> 			       BPF_F_REPLACE | BPF_F_ID,
> 			       link->prog->aux->id, 0);
> 	...
> }
>
> Note that tcx_link_update() in the earlier tcx fix appears to suffer from
> the exact same omission.
>

You're right, will fix netkit_link_update() in v2.


>>   	rtnl_lock();
>>   	dev = netkit_dev_fetch(current->nsproxy->net_ns,
>>   			       attr->link_create.target_ifindex,
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.