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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 4/24/26 11:37 AM, sashiko-bot@kernel.org 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 <jiayuan.chen@linux.dev>
>
> 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,