Re: [PATCH v2 1/4] tun: fix inverted error check when re-attaching the filter

Does this change the behavior for cases where tun->fprog.filter holds a
__user pointer from a different process?

When TUNATTACHFILTER runs, it copies only the sock_fprog header:

    case TUNATTACHFILTER:
        ...
        if (copy_from_user(&tun->fprog, argp, sizeof(tun->fprog)))

This means tun->fprog.filter still points into the address space of the
process that issued that ioctl. When sk_attach_filter() runs here, it
calls __get_filter() which does:

    copy_from_user(prog->insns, fprog->filter, fsize)

in the context of the current process.

For a persistent TAP, if process A creates it and runs TUNATTACHFILTER
with a filter array on its stack or heap, then closes the fd, and process
B later opens /dev/net/tun and runs TUNSETIFF on the same name without
IFF_NOFILTER, the fprog.filter pointer may not be valid in process B's
address space.

Before this patch, when sk_attach_filter() failed (which was actually the
success case due to the inverted check), the err value was overwritten by
later operations (xdp_rxq_info_reg() followed by 'err = 0'), so the queue
was published and TUNSETIFF succeeded, just without a filter installed.

After this patch, if the pointer is unmapped, __get_filter() returns
-EFAULT and TUNSETIFF now fails. If it happens to be mapped but holds
other data, bpf_prepare_filter() usually returns -EINVAL and TUNSETIFF
fails. When the bytes happen to pass validation, they become the filter of
a queue that actually gets published.

This affects both the persistent TAP case described above and multi-queue
setups where another queue is opened after the buffer holding the original
filter has been freed or reused.

The next patch in the series, 508f529200c5 ("tun: keep a kernel copy of
the socket filter program"), addresses this by storing a kernel copy of
the filter program. That commit's changelog describes exactly this
behavior: "unmapped there, the attach fails with -EFAULT; mapped,
whatever bytes it holds become the filter of the new queue". It relies on
8c6a2cf8bd0a ("net: filter: add sk_attach_filter_kern() function").

Until that commit is applied, this patch changes the re-attach behavior
from succeeding (without filter) to failing (with -EFAULT or -EINVAL).
Both commits carry the same Fixes: tag, so a stable backport or bisect
that includes only this commit would encounter the problem.

Would it make sense to reorder these commits, putting the kernel-copy
change before the inverted-check fix, so every step of the series works?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36552651982