[PATCH net] tcp: reject devmem tx with fastopen and repair

[PATCH net] tcp: reject devmem tx with fastopen and repair

[Kaifeng Wang]

tcp_sendmsg_locked() enforces that devmem TX can only proceed if the
zero-copy path is active and a valid dmabuf binding exists. However,
subsequent branches in tcp_sendmsg_locked() can still intercept the
message before it reaches the devmem zero-copy loop:

1. TCP Fast Open (MSG_FASTOPEN or DEFER_CONNECT):
   If TCP_FASTOPEN_CONNECT is set, the socket may have a valid dst with
   NETIF_F_SG (so zc == MSG_ZEROCOPY and binding is present), but
   tcp_sendmsg_fastopen() -> tcp_send_syn_data() will use
   copy_page_from_iter() to byte-copy from the iterator. Since iov_base
   represents dma-buf offsets rather than user virtual addresses, this
   misinterprets offsets as user pointers and copies arbitrary user memory
   into the SYN packet.

2. TCP repair mode:
   If tp->repair is enabled with TCP_RECV_QUEUE, tcp_send_rcvq() similarly
   calls skb_copy_datagram_from_iter(), byte-copying from the iterator.

Neither path supports or makes sense for devmem transmission. Reject devmem
sends if Fast Open or repair mode is active.

This pre-existing issue was identified by Sashiko AI review on commit
125755776bc6 ("tcp: reject non zerocopy devmem tx") and has not been
hit in production.

Fixes: 125755776bc6 ("tcp: reject non zerocopy devmem tx")
Fixes: bd61848900bf ("net: devmem: Implement TX path")
Signed-off-by: Kaifeng Wang <kaifengw@google.com>
---
 net/ipv4/tcp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 87ef6d5cbfeb..225e758c194a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1169,7 +1169,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 			zc = MSG_SPLICE_PAGES;
 	}
 
-	if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
+	if (!sockc_err && sockc.dmabuf_id &&
+	    (zc != MSG_ZEROCOPY || !binding || tp->repair ||
+	     (flags & MSG_FASTOPEN) || inet_test_bit(DEFER_CONNECT, sk))) {
 		err = -EINVAL;
 		goto out_err;
 	}
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH net] tcp: reject devmem tx with fastopen and repair

[Stanislav Fomichev]

On 10/05, Kaifeng Wang wrote:
> tcp_sendmsg_locked() enforces that devmem TX can only proceed if the
> zero-copy path is active and a valid dmabuf binding exists. However,
> subsequent branches in tcp_sendmsg_locked() can still intercept the
> message before it reaches the devmem zero-copy loop:
> 
> 1. TCP Fast Open (MSG_FASTOPEN or DEFER_CONNECT):
>    If TCP_FASTOPEN_CONNECT is set, the socket may have a valid dst with
>    NETIF_F_SG (so zc == MSG_ZEROCOPY and binding is present), but
>    tcp_sendmsg_fastopen() -> tcp_send_syn_data() will use
>    copy_page_from_iter() to byte-copy from the iterator. Since iov_base
>    represents dma-buf offsets rather than user virtual addresses, this
>    misinterprets offsets as user pointers and copies arbitrary user memory
>    into the SYN packet.
> 
> 2. TCP repair mode:
>    If tp->repair is enabled with TCP_RECV_QUEUE, tcp_send_rcvq() similarly
>    calls skb_copy_datagram_from_iter(), byte-copying from the iterator.
> 
> Neither path supports or makes sense for devmem transmission. Reject devmem
> sends if Fast Open or repair mode is active.
> 
> This pre-existing issue was identified by Sashiko AI review on commit
> 125755776bc6 ("tcp: reject non zerocopy devmem tx") and has not been
> hit in production.

Flagged by an AI review: we do tp->repair check before
sk_stream_wait_connect which drops/requires the socket lock. So
technically someone can setsockopt(tcp_repair) which the connection
handshake is happening. Sounds reasonable?

Re: [PATCH net] tcp: reject devmem tx with fastopen and repair

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new devmem rejection in tcp_sendmsg_locked()
  (net/ipv4/tcp.c:1172-1177) is wrapped in `!sockc_err`, so it does…
- [Medium] In tcp_sendmsg_locked(), the new `tp->repair` rejection
  (net/ipv4/tcp.c:1172-1177) runs once at the top, while the socket lock…

--

tcp: reject devmem tx with fastopen and repair

This commit extends the devmem TX check in tcp_sendmsg_locked() so that it
also rejects sends when TCP Fast Open (MSG_FASTOPEN or DEFER_CONNECT) or TCP
repair mode is active. Both paths byte-copy from the iterator and would treat
dma-buf offsets as user pointers.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 87ef6d5cbfeba..225e758c194aa 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1169,7 +1169,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>  			zc = MSG_SPLICE_PAGES;
>  	}
>  
> -	if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
> +	if (!sockc_err && sockc.dmabuf_id &&
> +	    (zc != MSG_ZEROCOPY || !binding || tp->repair ||
> +	     (flags & MSG_FASTOPEN) || inet_test_bit(DEFER_CONNECT, sk))) {

[Severity: Medium]
Can the !sockc_err gate let the Fast Open and repair cases through?

sock_cmsg_send() processes cmsgs in order. It returns at the first error and
does not undo fields that earlier cmsgs already set. __sock_cmsg_send() stores
dmabuf_id as soon as it sees SCM_DEVMEM_DMABUF:

net/core/sock.c:__sock_cmsg_send() {
    ...
	case SCM_DEVMEM_DMABUF:
		if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
			return -EINVAL;
		sockc->dmabuf_id = *(u32 *)CMSG_DATA(cmsg);
		break;
    ...
}

Suppose a valid SCM_DEVMEM_DMABUF is followed by a cmsg that fails. Two
examples:

  SCM_TS_OPT_ID, which always returns -EINVAL on TCP
  SO_PRIORITY with a value the caller may not set, which returns -EPERM

In that case sockc.dmabuf_id stays set and sockc_err is non-zero, so the new
check is skipped entirely.

The Fast Open branch runs whether or not sockc_err is set:

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

tcp_sendmsg_fastopen()->tcp_send_syn_data() then calls copy_page_from_iter()
and treats the iov_base values as user addresses. When copied_syn > 0 the code
jumps to out, so sockc_err is never returned and the send looks successful.

The repair TCP_RECV_QUEUE path works the same way. tcp_send_rcvq() runs and
jumps to out_nopush before the later "if (sockc_err)" check.

Could the Fast Open and repair conditions be tested against sockc.dmabuf_id
without the !sockc_err gate? Another option would be to return sockc_err
before these early copy paths when dmabuf_id is set.

>  		err = -EINVAL;
>  		goto out_err;
>  	}

[Severity: Medium]
Is it enough to check tp->repair only once, at the top of the function?

If the socket is still in SYN_SENT or SYN_RECV, tcp_sendmsg_locked() later
calls sk_stream_wait_connect(). While it sleeps, sk_wait_event() drops the
socket lock via release_sock().

During that window, another thread with CAP_NET_ADMIN in the netns user
namespace can run:

  setsockopt(TCP_REPAIR, TCP_REPAIR_ON)
    tcp_can_repair_sock() only requires sk_state != TCP_LISTEN
  setsockopt(TCP_REPAIR_QUEUE, TCP_RECV_QUEUE)
    do_tcp_setsockopt() only requires tp->repair

Once the connection is established, the sender wakes up and reads tp->repair
again:

	if (unlikely(tp->repair)) {
		if (tp->repair_queue == TCP_RECV_QUEUE) {
			copied = tcp_send_rcvq(sk, msg, size);
			goto out_nopush;
		}

tcp_send_rcvq() then calls skb_copy_datagram_from_iter() on the devmem
iterator even though binding is held. This is the repair case the commit
message describes.

The devmem setup can pass the entry check in SYN_SENT. Connect has already
set sk_route_caps and the dst, so zc == MSG_ZEROCOPY and
net_devmem_get_binding() succeeds.

Would rejecting a non-NULL binding inside the tp->repair branch cover this?
Repeating the check after the wait would also work.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005214002.3226574-1-kaifengw%40google.com

Re: [PATCH net] tcp: reject devmem tx with fastopen and repair

[Pavel Begunkov]

On 10/5/26 22:40, Kaifeng Wang wrote:
> tcp_sendmsg_locked() enforces that devmem TX can only proceed if the
> zero-copy path is active and a valid dmabuf binding exists. However,
> subsequent branches in tcp_sendmsg_locked() can still intercept the
> message before it reaches the devmem zero-copy loop:
> 
> 1. TCP Fast Open (MSG_FASTOPEN or DEFER_CONNECT):
>     If TCP_FASTOPEN_CONNECT is set, the socket may have a valid dst with
>     NETIF_F_SG (so zc == MSG_ZEROCOPY and binding is present), but
>     tcp_sendmsg_fastopen() -> tcp_send_syn_data() will use
>     copy_page_from_iter() to byte-copy from the iterator. Since iov_base
>     represents dma-buf offsets rather than user virtual addresses, this
>     misinterprets offsets as user pointers and copies arbitrary user memory
>     into the SYN packet.
> 
> 2. TCP repair mode:
>     If tp->repair is enabled with TCP_RECV_QUEUE, tcp_send_rcvq() similarly
>     calls skb_copy_datagram_from_iter(), byte-copying from the iterator.
> 
> Neither path supports or makes sense for devmem transmission. Reject devmem
> sends if Fast Open or repair mode is active.
> 
> This pre-existing issue was identified by Sashiko AI review on commit
> 125755776bc6 ("tcp: reject non zerocopy devmem tx") and has not been
> hit in production.
> 
> Fixes: 125755776bc6 ("tcp: reject non zerocopy devmem tx")
> Fixes: bd61848900bf ("net: devmem: Implement TX path")
> Signed-off-by: Kaifeng Wang <kaifengw@google.com>
> ---
>   net/ipv4/tcp.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 87ef6d5cbfeb..225e758c194a 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1169,7 +1169,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>   			zc = MSG_SPLICE_PAGES;
>   	}
>   
> -	if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
> +	if (!sockc_err && sockc.dmabuf_id &&
> +	    (zc != MSG_ZEROCOPY || !binding || tp->repair ||
> +	     (flags & MSG_FASTOPEN) || inet_test_bit(DEFER_CONNECT, sk))) {

It becomes unhandy, and taking into account the issue Stan commented
about, I think it'd make sense to do the checks under the repair and
fast open "ifs".

if (((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) &&
     !tcp_passive_fastopen(sk)) {
	if (binding) // fail;
}

if (unlikely(tp->repair)) {
	if (binding) // fail;	
}

-- 
Pavel Begunkov

Re: [PATCH net] tcp: reject devmem tx with fastopen and repair

[Kaifeng Wang]

> Is it enough to check tp->repair only once, at the top of the function?

Agreed. As Stan and Pavel suggested, I will move the `if (binding)`
checks directly into the Fast Open and `tp->repair` branches in v2.
pw-bot: cr

On Thu, Oct 8, 2026 at 5:54 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 10/5/26 22:40, Kaifeng Wang wrote:
> > tcp_sendmsg_locked() enforces that devmem TX can only proceed if the
> > zero-copy path is active and a valid dmabuf binding exists. However,
> > subsequent branches in tcp_sendmsg_locked() can still intercept the
> > message before it reaches the devmem zero-copy loop:
> >
> > 1. TCP Fast Open (MSG_FASTOPEN or DEFER_CONNECT):
> >     If TCP_FASTOPEN_CONNECT is set, the socket may have a valid dst with
> >     NETIF_F_SG (so zc == MSG_ZEROCOPY and binding is present), but
> >     tcp_sendmsg_fastopen() -> tcp_send_syn_data() will use
> >     copy_page_from_iter() to byte-copy from the iterator. Since iov_base
> >     represents dma-buf offsets rather than user virtual addresses, this
> >     misinterprets offsets as user pointers and copies arbitrary user memory
> >     into the SYN packet.
> >
> > 2. TCP repair mode:
> >     If tp->repair is enabled with TCP_RECV_QUEUE, tcp_send_rcvq() similarly
> >     calls skb_copy_datagram_from_iter(), byte-copying from the iterator.
> >
> > Neither path supports or makes sense for devmem transmission. Reject devmem
> > sends if Fast Open or repair mode is active.
> >
> > This pre-existing issue was identified by Sashiko AI review on commit
> > 125755776bc6 ("tcp: reject non zerocopy devmem tx") and has not been
> > hit in production.
> >
> > Fixes: 125755776bc6 ("tcp: reject non zerocopy devmem tx")
> > Fixes: bd61848900bf ("net: devmem: Implement TX path")
> > Signed-off-by: Kaifeng Wang <kaifengw@google.com>
> > ---
> >   net/ipv4/tcp.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 87ef6d5cbfeb..225e758c194a 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -1169,7 +1169,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> >                       zc = MSG_SPLICE_PAGES;
> >       }
> >
> > -     if (!sockc_err && sockc.dmabuf_id && (zc != MSG_ZEROCOPY || !binding)) {
> > +     if (!sockc_err && sockc.dmabuf_id &&
> > +         (zc != MSG_ZEROCOPY || !binding || tp->repair ||
> > +          (flags & MSG_FASTOPEN) || inet_test_bit(DEFER_CONNECT, sk))) {
>
> It becomes unhandy, and taking into account the issue Stan commented
> about, I think it'd make sense to do the checks under the repair and
> fast open "ifs".
>
> if (((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) &&
>      !tcp_passive_fastopen(sk)) {
>         if (binding) // fail;
> }
>
> if (unlikely(tp->repair)) {
>         if (binding) // fail;
> }
>
> --
> Pavel Begunkov
>