Re: [PATCH net v3 1/3] tun: keep a kernel copy of the socket filter program

[Rongguang Wei <clementwei90@163.com>]

on 2026/10/1 02:33, Willem de Bruijn wrote:
> Rongguang Wei wrote:
>> From: Rongguang Wei <weirongguang@kylinos.cn>
>>
>> TUNATTACHFILTER copies only the sock_fprog header, so tun->fprog.filter
>> stays a pointer into the address space of the process that issued the
>> ioctl. tun_attach() reads it again whenever a queue is attached to the
>> persistent device later on: unmapped there, the attach fails with -EFAULT;
>> mapped, whatever bytes it holds become the filter of the new queue.
>>
>> Keep the program in the kernel instead. tun->fprog_kern holds the
>> instructions and each queue gets its own program, built with
>> sk_attach_filter_kern().
>>
>> sk_attach_filter_kern() reads no user buffer and attaches the program like
>> sk_attach_filter() does. The caller must hold the socket lock. Failing the
>> attach releases it; so does the socket when the filter is replaced,
>> detached or the socket goes away.
>>
>> The copy is freed when the filter is detached or replaced and with the
>> device, and tun->fprog is left untouched so TUNGETFILTER keeps its uapi
>> behaviour.
>>
>> Fixes: 54f968d6efdb ("tuntap: move socket to tun_file")
>> Link: https://lore.kernel.org/netdev/179027275318.2160803.4185895144088175048@kernel.org/
>> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
> 
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> 
>> +int sk_attach_filter_kern(struct sock_fprog_kern *fprog, struct sock *sk)
>> +{
>> +	struct bpf_prog *prog;
>> +	int err;
>> +
>> +	if (sock_flag(sk, SOCK_FILTER_LOCKED))
>> +		return -EPERM;
>> +
>> +	err = bpf_prog_create(&prog, fprog);
> 
> Interesting small issue, not new with this patch:
> 
> The BPF program is not validated on copy_from_user, but on attach,
> possibly after the system call has already returned with success.
> Which results in a dev without filter, rather than a failed syscall.
> 
Got it, a device with no queues still returns success. I will look at it separately.
Thanks for the review.
>> +	if (err)
>> +		return err;
>> +
>> +	err = __sk_attach_prog(prog, sk);
>> +	if (err < 0) {
>> +		__bpf_prog_release(prog);
>> +		return err;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(sk_attach_filter_kern);
>> +
>>  int sk_reuseport_attach_filter(struct sock_fprog *fprog, struct sock *sk)
>>  {
>>  	struct bpf_prog *prog = __get_filter(fprog, sk);
>> -- 
>> 2.25.1
>>
>>
>> No virus found
>> 		Checked by Hillstone Network AntiVirus
>>
>