Re: [PATCH net 1/1] tcp: reject non zerocopy devmem tx

On Sat, Aug 22, 2026 at 3:57 AM Pavel Begunkov <[email protected]> wrote:
>
> Device memory send doesn't work without zero copy, prevent
> tcp_sendmsg_locked() from falling back to the copy mode for devmem in
> case there is no NETIF_F_SG. Currently, it'd try to copy from iovec
> filled with dma-buf offsets, which would normally fail with EFAULT, but
> it's still better to handle it more explicitly. A recent net-iov / page
> mixing fix also needs this patch.
>
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
>  net/ipv4/tcp.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 455441f1b694..f403830af65f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1162,6 +1162,10 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>                                         binding = NULL;
>                                         goto out_err;
>                                 }
> +                               if (zc != MSG_ZEROCOPY) {
> +                                       err = -EOPNOTSUPP;
> +                                       goto out_err;
> +                               }
>                         }
>                 }
>         } else if (unlikely(msg->msg_flags & MSG_SPLICE_PAGES) && size) {
> --
> 2.54.0
>

Thanks, I was indeed putting together patches to fix all the
pre-existing issues. I think the code assumes devmem send means
binding is not NULL, and the fact that we efault on copying is lucky,
we should not even attempt copying.

How about we reduce the complexity of an already-long
tcp_sendmsg_locked function by combining this check with the other
ones? I was thinking this:

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d6..1e34f50770446 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1170,7 +1170,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct
msghdr *msg, size_t size)
        }

        if (!sockc_err && sockc.dmabuf_id &&
-           (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
+           (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY) ||
+            zc != MSG_ZEROCOPY)) {
                err = -EINVAL;
                goto out_err;
        }

It will return EINVAL instead of EOPNOTSUPP but I think that is OK.

-- 
Thanks,
Mina