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

[David Laight <david.laight.linux@gmail.com>]

On Sun, 23 Aug 2026 08:58:32 -0700
Mina Almasry <almasrymina@google.com> wrote:

> On Sat, Aug 22, 2026 at 3:57 AM Pavel Begunkov <asml.silence@gmail.com> 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 <asml.silence@gmail.com>
> > ---
> >  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)) {

That might be more readable with the ! moved outside the ():
> +           !((flags & MSG_ZEROCOPY) && sock_flag(sk, SOCK_ZEROCOPY) &&
> +            zc == MSG_ZEROCOPY)) {

David

>                 err = -EINVAL;
>                 goto out_err;
>         }
> 
> It will return EINVAL instead of EOPNOTSUPP but I think that is OK.
>