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

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

[Pavel Begunkov]

Devmem tcp tx doesn't work without zero-copy, however it's not currently
enforced if NETIF_F_SG isn't present. In this case, tcp_sendmsg_locked()
will try the copy path and try to copy data from an iovec which consists
of offsets into the dma-buf and would normally fail. Moreover,
d9c56501c72fd ("net: tcp: block mixing readable and unreadable frags")
relies on that and assumes that the devmem binding is present IFF we're
using the zero-copy path, which can be used to mix net-iov and pages in
a single skb, and break invariants. Let's reject devmem tx without
zero-copy.

Note, the parameter check the patch is modifying is too loose, we can
create an io_uring request with dmabuf_id and all ZC flags, but which
won't have the binding. We replace it with stricter validation.

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Fixes: d9c56501c72fd ("net: tcp: block mixing readable and unreadable frags")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---

v2: combine with post ubuf assignment dmabuf_id checks

 net/ipv4/tcp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d..10f58281be17 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1169,8 +1169,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 			zc = MSG_SPLICE_PAGES;
 	}
 
-	if (!sockc_err && sockc.dmabuf_id &&
-	    (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
+	if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
 		err = -EINVAL;
 		goto out_err;
 	}
-- 
2.54.0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete enforcement: devmem TX can still reach a byte-copy
  path that misinterprets dma-buf offsets as user virtual addresses.

--

tcp: reject non zerocopy devmem tx

The patch replaces the MSG_ZEROCOPY/SOCK_ZEROCOPY test that guards devmem
TX in tcp_sendmsg_locked() with a stricter check that requires the
zero-copy path to actually be in use and the dmabuf binding to have been
obtained.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1c867a3024446..562752352afe4 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1169,8 +1169,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>  			zc = MSG_SPLICE_PAGES;
>  	}
>  
> -	if (!sockc_err && sockc.dmabuf_id &&
> -	    (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
> +	if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
>  		err = -EINVAL;
>  		goto out_err;
>  	}

[Severity: High]
Does this check still leave a devmem copy path reachable via TCP Fast
Open?  Right after it, tcp_sendmsg_locked() does:

	if (unlikely(flags & MSG_FASTOPEN ||
		     inet_test_bit(DEFER_CONNECT, sk)) &&
	    !tp->repair) {
		err = tcp_sendmsg_fastopen(sk, msg, &copied_syn, size, uarg);

Neither tcp_sendmsg_fastopen() nor tcp_connect()->tcp_send_syn_data()
gets the binding, and tcp_send_syn_data() copies straight out of the
same iterator:

net/ipv4/tcp_output.c:tcp_send_syn_data() {
	...
	space = copy_page_from_iter(pfrag->page, pfrag->offset,
				    space, &fo->data->msg_iter);
	...
	skb_fill_page_desc(syn_data, 0, pfrag->page,
			   pfrag->offset, space);
	...
	skb_zcopy_set(syn_data, fo->uarg, NULL);
}

For a devmem send the iovec entries are dma-buf offsets, not addresses:

net/core/datagram.c:zerocopy_fill_skb_from_devmem() {
	/* Devmem filling works by taking an IOVEC from the user where the
	 * iov_addrs are interpreted as an offset in bytes into the dma-buf to
	 * send from. We do not support other iter types.
	 */
}

So does copy_page_from_iter() here read whatever the caller happens to
have mapped at the numeric value of those offsets, put it into a regular
page frag, advance the iterator past the corresponding dma-buf range,
and then report those bytes back through fo->copied -> copied_syn ->
copied as if they had been sent?

Is the path reachable without MSG_FASTOPEN as well?  tcp_v4_connect()
runs sk_setup_caps(sk, &rt->dst) before tcp_fastopen_defer_connect(), so
with TCP_FASTOPEN_CONNECT the socket has a dst and NETIF_F_SG in
sk_route_caps while DEFER_CONNECT is still set.  A subsequent
sendmsg(MSG_ZEROCOPY) carrying SCM_DEVMEM_DMABUF would then get
zc == MSG_ZEROCOPY and a valid binding from net_devmem_get_binding(),
pass the new check, and land in tcp_send_syn_data().

There also looks to be a second residual route through repair mode.  The
tp->repair branch below reaches tcp_send_rcvq(), which copies from the
same iterator with no binding awareness:

net/ipv4/tcp_input.c:tcp_send_rcvq() {
	...
	err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, size);
	...
}

That one needs CAP_NET_ADMIN, so it is less interesting, but it is still
a devmem iterator being byte-copied.

One more detail on the Fast Open case: msg_zerocopy_alloc() skips
accounting for devmem uargs:

net/core/skbuff.c:msg_zerocopy_alloc() {
	...
	if (likely(!devmem) && mm_account_pinned_pages(&uarg->mmp, size)) {
	...
}

so attaching that uarg to a copied page frag in tcp_send_syn_data()
would leave the frag unaccounted too.

Would it make sense to also reject dmabuf_id together with
MSG_FASTOPEN/DEFER_CONNECT and with repair-queue sends, or to teach
those paths about the binding?  The commit message mentions neither, so
it is hard to tell from the changelog whether these were considered,
which also matters for backporting given the two Fixes: tags.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/fdc2478d8f21268d7078556409887d8e6ba0ad32.1788529053.git.asml.silence%40gmail.com

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

[Mina Almasry]

On Wed, Sep 9, 2026 at 6:45 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> - [High] Incomplete enforcement: devmem TX can still reach a byte-copy
>   path that misinterprets dma-buf offsets as user virtual addresses.
>


Sorry, this slipped my inbox the first time around, but now I see the
Sashiko review.

> --
>
> tcp: reject non zerocopy devmem tx
>
> The patch replaces the MSG_ZEROCOPY/SOCK_ZEROCOPY test that guards devmem
> TX in tcp_sendmsg_locked() with a stricter check that requires the
> zero-copy path to actually be in use and the dmabuf binding to have been
> obtained.
>
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 1c867a3024446..562752352afe4 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -1169,8 +1169,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> >                       zc = MSG_SPLICE_PAGES;
> >       }
> >
> > -     if (!sockc_err && sockc.dmabuf_id &&
> > -         (!(flags & MSG_ZEROCOPY) || !sock_flag(sk, SOCK_ZEROCOPY))) {
> > +     if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
> >               err = -EINVAL;
> >               goto out_err;
> >       }
>
> [Severity: High]
> Does this check still leave a devmem copy path reachable via TCP Fast
> Open?  Right after it, tcp_sendmsg_locked() does:
>
>         if (unlikely(flags & MSG_FASTOPEN ||
>                      inet_test_bit(DEFER_CONNECT, sk)) &&
>             !tp->repair) {
>                 err = tcp_sendmsg_fastopen(sk, msg, &copied_syn, size, uarg);
>
> Neither tcp_sendmsg_fastopen() nor tcp_connect()->tcp_send_syn_data()
> gets the binding, and tcp_send_syn_data() copies straight out of the
> same iterator:
>
> net/ipv4/tcp_output.c:tcp_send_syn_data() {
>         ...
>         space = copy_page_from_iter(pfrag->page, pfrag->offset,
>                                     space, &fo->data->msg_iter);
>         ...
>         skb_fill_page_desc(syn_data, 0, pfrag->page,
>                            pfrag->offset, space);
>         ...
>         skb_zcopy_set(syn_data, fo->uarg, NULL);
> }
>
> For a devmem send the iovec entries are dma-buf offsets, not addresses:
>
> net/core/datagram.c:zerocopy_fill_skb_from_devmem() {
>         /* Devmem filling works by taking an IOVEC from the user where the
>          * iov_addrs are interpreted as an offset in bytes into the dma-buf to
>          * send from. We do not support other iter types.
>          */
> }
>
> So does copy_page_from_iter() here read whatever the caller happens to
> have mapped at the numeric value of those offsets, put it into a regular
> page frag, advance the iterator past the corresponding dma-buf range,
> and then report those bytes back through fo->copied -> copied_syn ->
> copied as if they had been sent?
>
> Is the path reachable without MSG_FASTOPEN as well?  tcp_v4_connect()
> runs sk_setup_caps(sk, &rt->dst) before tcp_fastopen_defer_connect(), so
> with TCP_FASTOPEN_CONNECT the socket has a dst and NETIF_F_SG in
> sk_route_caps while DEFER_CONNECT is still set.  A subsequent
> sendmsg(MSG_ZEROCOPY) carrying SCM_DEVMEM_DMABUF would then get
> zc == MSG_ZEROCOPY and a valid binding from net_devmem_get_binding(),
> pass the new check, and land in tcp_send_syn_data().
>
> There also looks to be a second residual route through repair mode.  The
> tp->repair branch below reaches tcp_send_rcvq(), which copies from the
> same iterator with no binding awareness:
>
> net/ipv4/tcp_input.c:tcp_send_rcvq() {
>         ...
>         err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, size);
>         ...
> }
>
> That one needs CAP_NET_ADMIN, so it is less interesting, but it is still
> a devmem iterator being byte-copied.
>
> One more detail on the Fast Open case: msg_zerocopy_alloc() skips
> accounting for devmem uargs:
>
> net/core/skbuff.c:msg_zerocopy_alloc() {
>         ...
>         if (likely(!devmem) && mm_account_pinned_pages(&uarg->mmp, size)) {
>         ...
> }
>
> so attaching that uarg to a copied page frag in tcp_send_syn_data()
> would leave the frag unaccounted too.
>
> Would it make sense to also reject dmabuf_id together with
> MSG_FASTOPEN/DEFER_CONNECT and with repair-queue sends, or to teach
> those paths about the binding?  The commit message mentions neither, so
> it is hard to tell from the changelog whether these were considered,
> which also matters for backporting given the two Fixes: tags.
>

Yes, I think we'd need to exclude MSG_FASTOPEN and DEFER_CONNECT.
Those are set by the userspace right? So it doesn't matter much if we
don't support them with devmem. Not sure how to correctly exclude
repair-queue sends.

I guess it's pointing to more pre-existing issues :( But this patch on
its own is fine.

Reviewed-by: Mina Almasry <almasrymina@google.com>

I can try to follow up on the other prexisting issues.

-- 
Thanks,
Mina

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

[Paolo Abeni]

On 9/9/26 5:57 PM, Mina Almasry wrote:
> On Wed, Sep 9, 2026 at 6:45 AM <netdev-bot+sashiko@kernel.org> wrote:
>> Would it make sense to also reject dmabuf_id together with
>> MSG_FASTOPEN/DEFER_CONNECT and with repair-queue sends, or to teach
>> those paths about the binding?  The commit message mentions neither, so
>> it is hard to tell from the changelog whether these were considered,
>> which also matters for backporting given the two Fixes: tags.
>>
> 
> Yes, I think we'd need to exclude MSG_FASTOPEN and DEFER_CONNECT.
> Those are set by the userspace right? So it doesn't matter much if we
> don't support them with devmem. Not sure how to correctly exclude
> repair-queue sends.
FTR, I agree that the sashiko comments refers to a pre-existing and
different issue that should be addressed separately. I'm actually
quite surprised the model did not mark it as such.

/P

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri,  4 Sep 2026 14:43:07 +0100 you wrote:
> Devmem tcp tx doesn't work without zero-copy, however it's not currently
> enforced if NETIF_F_SG isn't present. In this case, tcp_sendmsg_locked()
> will try the copy path and try to copy data from an iovec which consists
> of offsets into the dma-buf and would normally fail. Moreover,
> d9c56501c72fd ("net: tcp: block mixing readable and unreadable frags")
> relies on that and assumes that the devmem binding is present IFF we're
> using the zero-copy path, which can be used to mix net-iov and pages in
> a single skb, and break invariants. Let's reject devmem tx without
> zero-copy.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/1] tcp: reject non zerocopy devmem tx
    https://git.kernel.org/netdev/net/c/125755776bc6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Pavel Begunkov]

On 9/9/26 16:57, Mina Almasry wrote:
> On Wed, Sep 9, 2026 at 6:45 AM <netdev-bot+sashiko@kernel.org> wrote:
...>> so attaching that uarg to a copied page frag in tcp_send_syn_data()
>> would leave the frag unaccounted too.
>>
>> Would it make sense to also reject dmabuf_id together with
>> MSG_FASTOPEN/DEFER_CONNECT and with repair-queue sends, or to teach
>> those paths about the binding?  The commit message mentions neither, so
>> it is hard to tell from the changelog whether these were considered,
>> which also matters for backporting given the two Fixes: tags.
>>
> 
> Yes, I think we'd need to exclude MSG_FASTOPEN and DEFER_CONNECT.
> Those are set by the userspace right? So it doesn't matter much if we
> don't support them with devmem. Not sure how to correctly exclude
> repair-queue sends.

I don't know any other user of socket repair apart from CRIU,
and devmem + CRIU is quite a narrow use case, I doubt they yet
do that, and IIRC they don't support restoring any of the
GPUs state for it to be useful. IOW, failing repair with devmem
payload sounds like an option.

> I guess it's pointing to more pre-existing issues :( But this patch on
> its own is fine.
> 
> Reviewed-by: Mina Almasry <almasrymina@google.com>

Thanks

> I can try to follow up on the other prexisting issues.

Great, then I'll leave it to you

-- 
Pavel Begunkov