* [PATCH net v5 1/2] tun: keep a kernel copy of the socket filter program
2026-10-08 8:04 [PATCH net v5 0/2] tun: fix re-attaching the socket filter Rongguang Wei
@ 2026-10-08 8:04 ` Rongguang Wei
2026-10-08 8:04 ` [PATCH net v5 2/2] selftests: net: add TAP socket filter attach tests Rongguang Wei
2026-10-10 8:00 ` [PATCH net v5 0/2] tun: fix re-attaching the socket filter Simon Horman
2 siblings, 0 replies; 4+ messages in thread
From: Rongguang Wei @ 2026-10-08 8:04 UTC (permalink / raw)
To: netdev
Cc: willemdebruijn.kernel, jasowangio, andrew+netdev, davem, kuba,
Rongguang Wei, Willem de Bruijn
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.
Rebuilding the filter from that pointer is not reliable. With the inverted
check below, a failed read (-EFAULT for an unmapped address, -EINVAL for
bytes that are not a valid classic BPF program) was not fatal: for a new
tfile err is overwritten by "err = 0", so the queue was attached without a
filter. And when the read succeeded, the early return meant the queue was
never published at all.
Keep the program in the kernel instead: tun->fprog_kern holds the
instructions, and each queue gets its own program built from it with the
new sk_attach_filter_kern(), the kernel memory counterpart of
sk_attach_filter(). 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.
Also fix the inverted check at the same time when tun_attach() returns
early when sk_attach_filter_kern() succeeds instead of when it fails.
Neither change works on its own: with only the kernel copy, every re-attach
returns 0 without publishing the queue; with only the check fixed, a
re-attach that used to succeed without installing a filter would start to
fail.
sk_attach_filter_kern() builds the program with bpf_prog_create(), which
does not keep an original program, so SO_GET_FILTER returns -EACCES and
sock_diag omits the filter for sockets that use it. tun sockets are not
exposed as file descriptors, so this is not user visible.
Because the length is checked in tun_copy_filter() before anything is
replaced, a TUNATTACHFILTER with a bad header now fails early and leaves
the previously attached filter in place; before this series the failure
happened on queue 0 and cleared filter_attached.
The inverted check was discovered by manual code inspection first [1], and
the review of v1 reported the other things.
[1] https://lore.kernel.org/netdev/20260923025653.59348-1-clementwei90@163.com/
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>
---
drivers/net/tun.c | 56 ++++++++++++++++++++++++++++++++++++++----
include/linux/filter.h | 1 +
net/core/filter.c | 37 ++++++++++++++++++++++++++++
3 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5a302709a68a..b58ad67b77ad 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -197,6 +197,7 @@ struct tun_struct {
int sndbuf;
struct tap_filter txflt;
struct sock_fprog fprog;
+ struct sock_fprog_kern fprog_kern;
/* protected by rtnl lock */
bool filter_attached;
u32 msg_enable;
@@ -722,12 +723,47 @@ static void tun_force_wake_queue(struct tun_struct *tun,
spin_unlock_bh(&tfile->tx_ring.consumer_lock);
}
+/* Copy the filter that @argp points at into the kernel, so that it can be
+ * installed again later, independent of the ioctl caller's address space.
+ * tun->fprog and tun->fprog_kern are updated only once the copy succeeded.
+ */
+static int tun_copy_filter(struct tun_struct *tun, struct sock_fprog __user *argp)
+{
+ struct sock_fprog fprog;
+ struct sock_filter *insns;
+
+ if (copy_from_user(&fprog, argp, sizeof(fprog)))
+ return -EFAULT;
+
+ if (!fprog.len || fprog.len > BPF_MAXINSNS)
+ return -EINVAL;
+
+ insns = kmalloc_array(fprog.len, sizeof(struct sock_filter),
+ GFP_KERNEL_ACCOUNT);
+ if (!insns)
+ return -ENOMEM;
+
+ if (copy_from_user(insns, fprog.filter,
+ fprog.len * sizeof(struct sock_filter))) {
+ kfree(insns);
+ return -EFAULT;
+ }
+
+ kfree(tun->fprog_kern.filter);
+ tun->fprog_kern.len = fprog.len;
+ tun->fprog_kern.filter = insns;
+ tun->fprog = fprog;
+
+ return 0;
+}
+
static int tun_attach(struct tun_struct *tun, struct file *file,
bool skip_filter, bool napi, bool napi_frags,
bool publish_tun)
{
struct tun_file *tfile = file->private_data;
struct net_device *dev = tun->dev;
+ bool rollback_filter = false;
int err;
err = security_tun_dev_attach(tfile->socket.sk, tun->security);
@@ -752,10 +788,11 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
/* Re-attach the filter to persist device */
if (!skip_filter && (tun->filter_attached == true)) {
lock_sock(tfile->socket.sk);
- err = sk_attach_filter(&tun->fprog, tfile->socket.sk);
+ err = sk_attach_filter_kern(&tun->fprog_kern, tfile->socket.sk);
release_sock(tfile->socket.sk);
- if (!err)
+ if (err)
goto out;
+ rollback_filter = true;
}
if (!tfile->detached &&
@@ -817,6 +854,11 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
WRITE_ONCE(tun->numqueues, tun->numqueues + 1);
tun_set_real_num_queues(tun);
out:
+ if (err && rollback_filter) {
+ lock_sock(tfile->socket.sk);
+ sk_detach_filter(tfile->socket.sk);
+ release_sock(tfile->socket.sk);
+ }
return err;
}
@@ -2397,6 +2439,7 @@ static void tun_free_netdev(struct net_device *dev)
security_tun_dev_free_security(tun->security);
__tun_set_ebpf(tun, &tun->steering_prog, NULL);
__tun_set_ebpf(tun, &tun->filter_prog, NULL);
+ kfree(tun->fprog_kern.filter);
}
static void tun_setup(struct net_device *dev)
@@ -3059,6 +3102,9 @@ static void tun_detach_filter(struct tun_struct *tun, int n)
release_sock(tfile->socket.sk);
}
+ kfree(tun->fprog_kern.filter);
+ tun->fprog_kern.filter = NULL;
+ tun->fprog_kern.len = 0;
tun->filter_attached = false;
}
@@ -3070,7 +3116,7 @@ static int tun_attach_filter(struct tun_struct *tun)
for (i = 0; i < tun->numqueues; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
lock_sock(tfile->socket.sk);
- ret = sk_attach_filter(&tun->fprog, tfile->socket.sk);
+ ret = sk_attach_filter_kern(&tun->fprog_kern, tfile->socket.sk);
release_sock(tfile->socket.sk);
if (ret) {
tun_detach_filter(tun, i);
@@ -3418,8 +3464,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
ret = -EINVAL;
if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
break;
- ret = -EFAULT;
- if (copy_from_user(&tun->fprog, argp, sizeof(tun->fprog)))
+ ret = tun_copy_filter(tun, argp);
+ if (ret)
break;
ret = tun_attach_filter(tun);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 39decde7fc73..0de5a738fb26 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1218,6 +1218,7 @@ int bpf_prog_create_from_user(struct bpf_prog **pfp, struct sock_fprog *fprog,
void bpf_prog_destroy(struct bpf_prog *fp);
int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
+int sk_attach_filter_kern(struct sock_fprog_kern *fprog, struct sock *sk);
int sk_attach_bpf(u32 ufd, struct sock *sk);
int sk_reuseport_attach_filter(struct sock_fprog *fprog, struct sock *sk);
int sk_reuseport_attach_bpf(u32 ufd, struct sock *sk);
diff --git a/net/core/filter.c b/net/core/filter.c
index 70dc621672f2..53cdf4ec4ef5 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1567,6 +1567,43 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
}
EXPORT_SYMBOL_GPL(sk_attach_filter);
+/**
+ * sk_attach_filter_kern - attach a classic BPF filter held in kernel memory
+ * @fprog: program to attach, with @fprog->filter pointing at kernel memory
+ * @sk: socket to attach the filter to
+ *
+ * Attach a copy of a classic BPF program that the kernel already holds, the
+ * way sk_attach_filter() does for one that still lives in user space. The
+ * caller must hold the socket lock; SOCK_FILTER_LOCKED is checked here.
+ *
+ * The program is built by bpf_prog_create(), which does not keep an original
+ * copy of the instructions. sk_get_filter() therefore returns -EACCES and
+ * sock_diag leaves the filter out for a socket that uses this helper. A
+ * classic BPF program cannot be duplicated with bpf_prog_copy(), so keeping
+ * the original would cost another allocation and a copy per attach.
+ */
+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);
+ 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net v5 2/2] selftests: net: add TAP socket filter attach tests
2026-10-08 8:04 [PATCH net v5 0/2] tun: fix re-attaching the socket filter Rongguang Wei
2026-10-08 8:04 ` [PATCH net v5 1/2] tun: keep a kernel copy of the socket filter program Rongguang Wei
@ 2026-10-08 8:04 ` Rongguang Wei
2026-10-10 8:00 ` [PATCH net v5 0/2] tun: fix re-attaching the socket filter Simon Horman
2 siblings, 0 replies; 4+ messages in thread
From: Rongguang Wei @ 2026-10-08 8:04 UTC (permalink / raw)
To: netdev
Cc: willemdebruijn.kernel, jasowangio, andrew+netdev, davem, kuba,
Rongguang Wei, Willem de Bruijn
From: Rongguang Wei <weirongguang@kylinos.cn>
reattach_filter_without_user_buffer() attaches a filter, makes the buffer
the program was copied from unreadable, detaches the queue and attaches it
again. It then attaches a second queue from a fresh socket, which starts
without a filter of its own, so a clear IFF_NOFILTER shows that the filter
was installed again from the kernel copy.
attach_filter_bad_len_keeps_program() checks that a rejected
TUNATTACHFILTER leaves the installed program and the filter state that
TUNGETIFF reports alone, and that a queue attached afterwards still gets
the filter.
attach_filter_nofilter_flag() and detach_filter_clears_reattach() cover the
ways to attach a queue without a filter.
Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
tools/testing/selftests/net/tun.c | 197 ++++++++++++++++++++++++++++++
1 file changed, 197 insertions(+)
diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
index abe488bac50b..9e57c7ecf6b7 100644
--- a/tools/testing/selftests/net/tun.c
+++ b/tools/testing/selftests/net/tun.c
@@ -8,8 +8,10 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <linux/filter.h>
#include <linux/if_tun.h>
#include <sys/ioctl.h>
+#include <sys/mman.h>
#include <sys/socket.h>
#include "kselftest_harness.h"
@@ -542,6 +544,201 @@ TEST_F(tun, reattach_close_delete)
EXPECT_EQ(tun_delete(self->ifname), 0);
}
+/* accept: return skb->len */
+static const struct sock_filter filter_accept[] = {
+ BPF_STMT(BPF_LD | BPF_W | BPF_LEN, 0),
+ BPF_STMT(BPF_ALU | BPF_ADD | BPF_K, 0),
+ BPF_STMT(BPF_RET | BPF_A, 0),
+};
+
+/* Put the instructions in an anonymous mapping, so that the test can make the
+ * address unreadable afterwards.
+ */
+static void *filter_alloc(const struct sock_filter *insns, unsigned int len)
+{
+ void *p;
+
+ p = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (p == MAP_FAILED)
+ return NULL;
+
+ memcpy(p, insns, len * sizeof(*insns));
+
+ return p;
+}
+
+static int filter_attach(int fd, const struct sock_filter *insns, unsigned int len)
+{
+ struct sock_fprog fp = {
+ .len = len,
+ .filter = (struct sock_filter *)insns,
+ };
+
+ return ioctl(fd, TUNATTACHFILTER, (void *)&fp);
+}
+
+static int filter_get(int fd, struct sock_fprog *fp)
+{
+ return ioctl(fd, TUNGETFILTER, (void *)fp);
+}
+
+static int tun_get_iff_flags(int fd)
+{
+ struct ifreq ifr = { 0 };
+
+ if (ioctl(fd, TUNGETIFF, (void *)&ifr) < 0)
+ return -1;
+
+ return ifr.ifr_flags;
+}
+
+/*
+ * A queue can be attached long after the filter was configured, from a
+ * process that does not necessarily map the buffer the program was copied
+ * from, so the kernel has to keep its own copy of the program. Here the
+ * mapping is made unreadable before the queue is attached again, and a
+ * second queue is then attached from a fresh socket, which starts without a
+ * filter of its own.
+ */
+TEST_F(tun, reattach_filter_without_user_buffer)
+{
+ struct sock_fprog gf = { 0 };
+ struct ifreq ifr = { 0 };
+ short flags = 0;
+ void *prog;
+ int fd, ret;
+
+ prog = filter_alloc(filter_accept, ARRAY_SIZE(filter_accept));
+ ASSERT_NE(prog, NULL);
+ ASSERT_EQ(filter_attach(self->fd, prog, ARRAY_SIZE(filter_accept)), 0);
+
+ EXPECT_EQ(tun_detach(self->fd, self->ifname), 0);
+
+ /* This memory is no longer readable */
+ ASSERT_EQ(mprotect(prog, getpagesize(), PROT_NONE), 0);
+
+ ret = tun_attach(self->fd, self->ifname);
+ EXPECT_EQ(ret, 0);
+
+ flags = tun_get_iff_flags(self->fd);
+ EXPECT_GE(flags, 0);
+ /* The queue is attached again, not left disabled */
+ EXPECT_EQ(flags & IFF_DETACH_QUEUE, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, 0);
+
+ /* The two checks below document the expected state, they cannot
+ * tell a reinstalled filter from no filter: tun_detach() leaves
+ * sk_filter alone, and TUNGETFILTER only returns the header.
+ */
+ EXPECT_EQ(filter_get(self->fd, &gf), 0);
+ EXPECT_EQ(gf.len, ARRAY_SIZE(filter_accept));
+
+ /* A new queue starts without a filter, so a clear IFF_NOFILTER shows
+ * that the filter was installed again from the kernel copy.
+ */
+ fd = open("/dev/net/tun", O_RDWR);
+ ASSERT_GE(fd, 0);
+
+ strcpy(ifr.ifr_name, self->ifname);
+ ifr.ifr_flags = IFF_TAP | IFF_MULTI_QUEUE;
+ EXPECT_GE(ioctl(fd, TUNSETIFF, (void *)&ifr), 0);
+
+ flags = tun_get_iff_flags(fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, 0);
+
+ close(fd);
+
+ ASSERT_EQ(mprotect(prog, getpagesize(), PROT_READ | PROT_WRITE), 0);
+ ASSERT_EQ(munmap(prog, getpagesize()), 0);
+}
+
+/* A new queue attached with IFF_NOFILTER must not get the filter that is
+ * configured on the device.
+ */
+TEST_F(tun, attach_filter_nofilter_flag)
+{
+ struct ifreq ifr = { 0 };
+ short flags = 0;
+ int fd;
+
+ ASSERT_EQ(filter_attach(self->fd, filter_accept, ARRAY_SIZE(filter_accept)), 0);
+
+ fd = open("/dev/net/tun", O_RDWR);
+ ASSERT_GE(fd, 0);
+
+ strcpy(ifr.ifr_name, self->ifname);
+ ifr.ifr_flags = IFF_TAP | IFF_MULTI_QUEUE | IFF_NOFILTER;
+ EXPECT_GE(ioctl(fd, TUNSETIFF, (void *)&ifr), 0);
+
+ flags = tun_get_iff_flags(fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, IFF_NOFILTER);
+
+ close(fd);
+}
+
+/* After TUNDETACHFILTER a later attach must not install the filter again */
+TEST_F(tun, detach_filter_clears_reattach)
+{
+ short flags = 0;
+
+ ASSERT_EQ(filter_attach(self->fd, filter_accept, ARRAY_SIZE(filter_accept)), 0);
+ EXPECT_EQ(ioctl(self->fd, TUNDETACHFILTER, 0), 0);
+
+ flags = tun_get_iff_flags(self->fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, IFF_NOFILTER);
+
+ EXPECT_EQ(tun_detach(self->fd, self->ifname), 0);
+ EXPECT_EQ(tun_attach(self->fd, self->ifname), 0);
+
+ flags = tun_get_iff_flags(self->fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, IFF_NOFILTER);
+}
+
+/* A TUNATTACHFILTER with a bad length must leave the installed program and the
+ * filter state that TUNGETIFF reports alone.
+ */
+TEST_F(tun, attach_filter_bad_len_keeps_program)
+{
+ struct sock_fprog gf = { 0 };
+ struct ifreq ifr = { 0 };
+ short flags = 0;
+ int fd;
+
+ ASSERT_EQ(filter_attach(self->fd, filter_accept, ARRAY_SIZE(filter_accept)), 0);
+
+ errno = 0;
+ EXPECT_EQ(filter_attach(self->fd, filter_accept, 0), -1);
+ EXPECT_EQ(errno, EINVAL);
+
+ EXPECT_EQ(filter_get(self->fd, &gf), 0);
+ EXPECT_EQ(gf.len, ARRAY_SIZE(filter_accept));
+
+ flags = tun_get_iff_flags(self->fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, 0);
+
+ /* A queue attached afterwards still gets the filter, so the rejected
+ * ioctl left the saved program and filter_attached alone.
+ */
+ fd = open("/dev/net/tun", O_RDWR);
+ ASSERT_GE(fd, 0);
+
+ strcpy(ifr.ifr_name, self->ifname);
+ ifr.ifr_flags = IFF_TAP | IFF_MULTI_QUEUE;
+ EXPECT_GE(ioctl(fd, TUNSETIFF, (void *)&ifr), 0);
+
+ flags = tun_get_iff_flags(fd);
+ EXPECT_GE(flags, 0);
+ EXPECT_EQ(flags & IFF_NOFILTER, 0);
+
+ close(fd);
+}
+
FIXTURE(tun_vnet_udptnl)
{
char ifname[IFNAMSIZ];
--
2.25.1
No virus found
Checked by Hillstone Network AntiVirus
^ permalink raw reply related [flat|nested] 4+ messages in thread