[PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Pavel Begunkov]

We should either have net_iov or page backed frags in a single skb,
otherwise it blows up down the stack. Don't allow mixing in
zerocopy_fill_skb_from_devmem().

Fixes: bd61848900bff ("net: devmem: Implement TX path")
Cc: stable@vger.kernel.org
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 net/core/datagram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index c285c6465923..35febc1c25fa 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -712,6 +712,9 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
 	size_t virt_addr, size, off;
 	struct net_iov *niov;
 
+	if (i && skb_frags_readable(skb))
+		return -EEXIST;
+
 	/* 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.
-- 
2.54.0

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Bobby Eshleman]

On Wed, Jul 22, 2026 at 11:58:46AM +0100, Pavel Begunkov wrote:
> We should either have net_iov or page backed frags in a single skb,
> otherwise it blows up down the stack. Don't allow mixing in
> zerocopy_fill_skb_from_devmem().
> 
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  net/core/datagram.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index c285c6465923..35febc1c25fa 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -712,6 +712,9 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
>  	size_t virt_addr, size, off;
>  	struct net_iov *niov;
>  
> +	if (i && skb_frags_readable(skb))
> +		return -EEXIST;

Are we trying to hit the -EEXIST handler in tcp_sendmsg_locked() so that
we start a new skb? If so, I think we might need to plumb this -EEXIST
case through skb_zerocopy_iter_stream() too?

Best,
Bobby

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Pavel Begunkov]

On 7/22/26 18:59, Bobby Eshleman wrote:
> On Wed, Jul 22, 2026 at 11:58:46AM +0100, Pavel Begunkov wrote:
>> We should either have net_iov or page backed frags in a single skb,
>> otherwise it blows up down the stack. Don't allow mixing in
>> zerocopy_fill_skb_from_devmem().
>>
>> Fixes: bd61848900bff ("net: devmem: Implement TX path")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   net/core/datagram.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/net/core/datagram.c b/net/core/datagram.c
>> index c285c6465923..35febc1c25fa 100644
>> --- a/net/core/datagram.c
>> +++ b/net/core/datagram.c
>> @@ -712,6 +712,9 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
>>   	size_t virt_addr, size, off;
>>   	struct net_iov *niov;
>>   
>> +	if (i && skb_frags_readable(skb))
>> +		return -EEXIST;
> 
> Are we trying to hit the -EEXIST handler in tcp_sendmsg_locked() so that
> we start a new skb? If so, I think we might need to plumb this -EEXIST
> case through skb_zerocopy_iter_stream() too?

Easier to EFAULT. It'd more consistent, and I don't care how tcp takes
it specifically.

-- 
Pavel Begunkov

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Mina Almasry]

On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> We should either have net_iov or page backed frags in a single skb,
> otherwise it blows up down the stack. Don't allow mixing in
> zerocopy_fill_skb_from_devmem().
>
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

It's true that we don't support mixing niov types and doing so would
blow up, but this is an unnecessary defensive check imo. The calling
code should not (and does not, I hope) have an edge case where it
tries to mix and match niov types.

Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
may be fine?

Also we don't really support mixing different niov sub-types in an skb
(like IO_URING + DMABUF) afaict, so might as well go the extra mile
and check that it's all devmem niovs specifically.

And might as well put the check in skb_add_rx_frag_netmem. It's the
same situation on RX, we don't support mixing there (and I hope no
code path leads to mixing today).

-- 
Thanks,
Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Pavel Begunkov]

On 7/22/26 20:49, Mina Almasry wrote:
> On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> We should either have net_iov or page backed frags in a single skb,
>> otherwise it blows up down the stack. Don't allow mixing in
>> zerocopy_fill_skb_from_devmem().
>>
>> Fixes: bd61848900bff ("net: devmem: Implement TX path")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> It's true that we don't support mixing niov types and doing so would
> blow up, but this is an unnecessary defensive check imo. The calling
> code should not (and does not, I hope) have an edge case where it
> tries to mix and match niov types.

Nope, that can easily happen.

> Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> may be fine?
> 
> Also we don't really support mixing different niov sub-types in an skb
> (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> and check that it's all devmem niovs specifically.

All those type intermixing rules might be flaky, maybe we should
start with DEBUG_NET_WARN_ON, but at least for tx from a quick
look it's handled by ubuf_info checks.

> And might as well put the check in skb_add_rx_frag_netmem. It's the
> same situation on RX, we don't support mixing there (and I hope no
> code path leads to mixing today).

A DEBUG_NET_WARN_ON there might be a very good idea, I agree.

-- 
Pavel Begunkov

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Bobby Eshleman]

On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >
> > We should either have net_iov or page backed frags in a single skb,
> > otherwise it blows up down the stack. Don't allow mixing in
> > zerocopy_fill_skb_from_devmem().
> >
> > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> It's true that we don't support mixing niov types and doing so would
> blow up, but this is an unnecessary defensive check imo. The calling
> code should not (and does not, I hope) have an edge case where it
> tries to mix and match niov types.
> 
> Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> may be fine?
> 
> Also we don't really support mixing different niov sub-types in an skb
> (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> and check that it's all devmem niovs specifically.
> 
> And might as well put the check in skb_add_rx_frag_netmem. It's the
> same situation on RX, we don't support mixing there (and I hope no
> code path leads to mixing today).

Hey Mina and Pavel,

I was able to confirm this mixing case does exist.

It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
== 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
(though same-frag merge is disallowed).

Adding a mode to ncdevmem that sends mixed devmem and non-devmem
messages, it can be caught hacking this in:

/* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
 * and non-devmem (page, readable) fragments. Such an skb must never exist.
 */
static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
{
	bool readable = false, unreadable = false;
	int i;

	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
		if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
			unreadable = true;
		else
			readable = true;
	}
	return readable && unreadable;
}

static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
{
	...
	BUG_ON(!skb || !tcp_skb_pcount(skb));

	WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
		  "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
		  skb_shinfo(skb)->nr_frags, skb->len);
	...
}


Resulting in:

[   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
[   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
[   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
   ...
[   85.915617]  tcp_write_xmit+0x47b/0x17d0
[   85.915802]  __tcp_push_pending_frames+0x38/0x100
[   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
[   85.916244]  tcp_sendmsg+0x2c/0x50
[   85.916903]  do_syscall_64+0x11c/0x610


My feeling is that we should guard against this when
tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
zc == 0 case, I think we need to check if the queue tail is unreadable
and 'goto new_segment' if it is?

This is a different case than Pavel's patch addresses though, where the
new sendmsg is devmem and write queue tail is readable.

Best,
Bobby

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Mina Almasry]

On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
>
> On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > >
> > > We should either have net_iov or page backed frags in a single skb,
> > > otherwise it blows up down the stack. Don't allow mixing in
> > > zerocopy_fill_skb_from_devmem().
> > >
> > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> >
> > It's true that we don't support mixing niov types and doing so would
> > blow up, but this is an unnecessary defensive check imo. The calling
> > code should not (and does not, I hope) have an edge case where it
> > tries to mix and match niov types.
> >
> > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > may be fine?
> >
> > Also we don't really support mixing different niov sub-types in an skb
> > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > and check that it's all devmem niovs specifically.
> >
> > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > same situation on RX, we don't support mixing there (and I hope no
> > code path leads to mixing today).
>
> Hey Mina and Pavel,
>
> I was able to confirm this mixing case does exist.
>
> It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> (though same-frag merge is disallowed).
>
> Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> messages, it can be caught hacking this in:
>
> /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
>  * and non-devmem (page, readable) fragments. Such an skb must never exist.
>  */
> static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> {
>         bool readable = false, unreadable = false;
>         int i;
>
>         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
>                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
>                         unreadable = true;
>                 else
>                         readable = true;
>         }
>         return readable && unreadable;
> }
>
> static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> {
>         ...
>         BUG_ON(!skb || !tcp_skb_pcount(skb));
>
>         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
>                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
>                   skb_shinfo(skb)->nr_frags, skb->len);
>         ...
> }
>
>
> Resulting in:
>
> [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
>    ...
> [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> [   85.916244]  tcp_sendmsg+0x2c/0x50
> [   85.916903]  do_syscall_64+0x11c/0x610
>
>
> My feeling is that we should guard against this when
> tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> zc == 0 case, I think we need to check if the queue tail is unreadable
> and 'goto new_segment' if it is?
>
> This is a different case than Pavel's patch addresses though, where the
> new sendmsg is devmem and write queue tail is readable.
>

:( Yep looks like we have a couple of bugs in the TX mixing. Sorry
about that. We do indeed need to fix this ASAP.

I don't think it's enough to check readable vs unreadable, no? Because
I think appending io_uring niovs to a devmem skb will still blow up
and vise versa, even though both are unreadable, right? Or is io_uring
saved from this somehow in both cases?

If io_uring is vulnerable to this as well, then fixing this becomes a
bit more hairy because this is a TCP fast path. We don't have bits in
the skb header telling us exactly what the skb memtype is (only
readable vs not), so we'll have to check skb_frag_is_net_iov(frags[0])
and netmem_to_net_iov(frags[0]->netmem)->niov_type to check the exact
type of the skb memtype, and that may be a lot of cachelines to fetch
in the fast path. :(


-- 
Thanks,
Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Bobby Eshleman]

On Thu, Jul 23, 2026 at 11:35:15AM -0700, Mina Almasry wrote:
> On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> >
> > On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > > >
> > > > We should either have net_iov or page backed frags in a single skb,
> > > > otherwise it blows up down the stack. Don't allow mixing in
> > > > zerocopy_fill_skb_from_devmem().
> > > >
> > > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > >
> > > It's true that we don't support mixing niov types and doing so would
> > > blow up, but this is an unnecessary defensive check imo. The calling
> > > code should not (and does not, I hope) have an edge case where it
> > > tries to mix and match niov types.
> > >
> > > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > > may be fine?
> > >
> > > Also we don't really support mixing different niov sub-types in an skb
> > > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > > and check that it's all devmem niovs specifically.
> > >
> > > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > > same situation on RX, we don't support mixing there (and I hope no
> > > code path leads to mixing today).
> >
> > Hey Mina and Pavel,
> >
> > I was able to confirm this mixing case does exist.
> >
> > It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> > == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> > (though same-frag merge is disallowed).
> >
> > Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> > messages, it can be caught hacking this in:
> >
> > /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
> >  * and non-devmem (page, readable) fragments. Such an skb must never exist.
> >  */
> > static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> > {
> >         bool readable = false, unreadable = false;
> >         int i;
> >
> >         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> >                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
> >                         unreadable = true;
> >                 else
> >                         readable = true;
> >         }
> >         return readable && unreadable;
> > }
> >
> > static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> > {
> >         ...
> >         BUG_ON(!skb || !tcp_skb_pcount(skb));
> >
> >         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
> >                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
> >                   skb_shinfo(skb)->nr_frags, skb->len);
> >         ...
> > }
> >
> >
> > Resulting in:
> >
> > [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> > [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> > [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
> >    ...
> > [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> > [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> > [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> > [   85.916244]  tcp_sendmsg+0x2c/0x50
> > [   85.916903]  do_syscall_64+0x11c/0x610
> >
> >
> > My feeling is that we should guard against this when
> > tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> > zc == 0 case, I think we need to check if the queue tail is unreadable
> > and 'goto new_segment' if it is?
> >
> > This is a different case than Pavel's patch addresses though, where the
> > new sendmsg is devmem and write queue tail is readable.
> >
> 
> :( Yep looks like we have a couple of bugs in the TX mixing. Sorry
> about that. We do indeed need to fix this ASAP.
> 
> I don't think it's enough to check readable vs unreadable, no? Because
> I think appending io_uring niovs to a devmem skb will still blow up
> and vise versa, even though both are unreadable, right? Or is io_uring
> saved from this somehow in both cases?

I think for the above case it is okay because if the current sendmsg()
is zc==0, then we don't care if the tail skb is iou or devmem as
skb->unreadable tells us enough to avoid appending the non-zc sendmsg.

I'm realizing this a different mixing issue than what Pavel is seeing
though, probably needs a separate patch.

Best,
Bobby

> 
> If io_uring is vulnerable to this as well, then fixing this becomes a
> bit more hairy because this is a TCP fast path. We don't have bits in
> the skb header telling us exactly what the skb memtype is (only
> readable vs not), so we'll have to check skb_frag_is_net_iov(frags[0])
> and netmem_to_net_iov(frags[0]->netmem)->niov_type to check the exact
> type of the skb memtype, and that may be a lot of cachelines to fetch
> in the fast path. :
> 
> 
> -- 
> Thanks,
> Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Mina Almasry]

On Thu, Jul 23, 2026 at 11:35 AM Mina Almasry <almasrymina@google.com> wrote:
>
> On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> >
> > On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > > >
> > > > We should either have net_iov or page backed frags in a single skb,
> > > > otherwise it blows up down the stack. Don't allow mixing in
> > > > zerocopy_fill_skb_from_devmem().
> > > >
> > > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > >
> > > It's true that we don't support mixing niov types and doing so would
> > > blow up, but this is an unnecessary defensive check imo. The calling
> > > code should not (and does not, I hope) have an edge case where it
> > > tries to mix and match niov types.
> > >
> > > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > > may be fine?
> > >
> > > Also we don't really support mixing different niov sub-types in an skb
> > > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > > and check that it's all devmem niovs specifically.
> > >
> > > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > > same situation on RX, we don't support mixing there (and I hope no
> > > code path leads to mixing today).
> >
> > Hey Mina and Pavel,
> >
> > I was able to confirm this mixing case does exist.
> >
> > It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> > == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> > (though same-frag merge is disallowed).
> >
> > Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> > messages, it can be caught hacking this in:
> >
> > /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
> >  * and non-devmem (page, readable) fragments. Such an skb must never exist.
> >  */
> > static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> > {
> >         bool readable = false, unreadable = false;
> >         int i;
> >
> >         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> >                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
> >                         unreadable = true;
> >                 else
> >                         readable = true;
> >         }
> >         return readable && unreadable;
> > }
> >
> > static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> > {
> >         ...
> >         BUG_ON(!skb || !tcp_skb_pcount(skb));
> >
> >         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
> >                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
> >                   skb_shinfo(skb)->nr_frags, skb->len);
> >         ...
> > }
> >
> >
> > Resulting in:
> >
> > [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> > [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> > [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
> >    ...
> > [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> > [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> > [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> > [   85.916244]  tcp_sendmsg+0x2c/0x50
> > [   85.916903]  do_syscall_64+0x11c/0x610
> >
> >
> > My feeling is that we should guard against this when
> > tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> > zc == 0 case, I think we need to check if the queue tail is unreadable
> > and 'goto new_segment' if it is?
> >
> > This is a different case than Pavel's patch addresses though, where the
> > new sendmsg is devmem and write queue tail is readable.
> >
>
> :( Yep looks like we have a couple of bugs in the TX mixing. Sorry
> about that. We do indeed need to fix this ASAP.
>
> I don't think it's enough to check readable vs unreadable, no? Because
> I think appending io_uring niovs to a devmem skb will still blow up
> and vise versa, even though both are unreadable, right? Or is io_uring
> saved from this somehow in both cases?
>
> If io_uring is vulnerable to this as well, then fixing this becomes a
> bit more hairy because this is a TCP fast path. We don't have bits in
> the skb header telling us exactly what the skb memtype is (only
> readable vs not), so we'll have to check skb_frag_is_net_iov(frags[0])
> and netmem_to_net_iov(frags[0]->netmem)->niov_type to check the exact
> type of the skb memtype, and that may be a lot of cachelines to fetch
> in the fast path. :(
>
>

Responding to my question here:

So AFAICT we can't actually have io_uring net_iovs in this path.
IO_uring ZC deos not support TX, and (the LLM) thinks that if we
receive an io_uring zc rx packet and forward it, it's still not going
to hit the tcp_sendmsg_locked path.

So this seems sufficient to me, Reviewed-by: Mina Almasry
<almasrymina@google.com>

-- 
Thanks,
Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Mina Almasry]

On Thu, Jul 23, 2026 at 4:10 PM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
>
> On Thu, Jul 23, 2026 at 11:35:15AM -0700, Mina Almasry wrote:
> > On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> > >
> > > On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > > > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > > > >
> > > > > We should either have net_iov or page backed frags in a single skb,
> > > > > otherwise it blows up down the stack. Don't allow mixing in
> > > > > zerocopy_fill_skb_from_devmem().
> > > > >
> > > > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > > >
> > > > It's true that we don't support mixing niov types and doing so would
> > > > blow up, but this is an unnecessary defensive check imo. The calling
> > > > code should not (and does not, I hope) have an edge case where it
> > > > tries to mix and match niov types.
> > > >
> > > > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > > > may be fine?
> > > >
> > > > Also we don't really support mixing different niov sub-types in an skb
> > > > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > > > and check that it's all devmem niovs specifically.
> > > >
> > > > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > > > same situation on RX, we don't support mixing there (and I hope no
> > > > code path leads to mixing today).
> > >
> > > Hey Mina and Pavel,
> > >
> > > I was able to confirm this mixing case does exist.
> > >
> > > It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> > > == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> > > (though same-frag merge is disallowed).
> > >
> > > Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> > > messages, it can be caught hacking this in:
> > >
> > > /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
> > >  * and non-devmem (page, readable) fragments. Such an skb must never exist.
> > >  */
> > > static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> > > {
> > >         bool readable = false, unreadable = false;
> > >         int i;
> > >
> > >         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > >                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
> > >                         unreadable = true;
> > >                 else
> > >                         readable = true;
> > >         }
> > >         return readable && unreadable;
> > > }
> > >
> > > static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> > > {
> > >         ...
> > >         BUG_ON(!skb || !tcp_skb_pcount(skb));
> > >
> > >         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
> > >                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
> > >                   skb_shinfo(skb)->nr_frags, skb->len);
> > >         ...
> > > }
> > >
> > >
> > > Resulting in:
> > >
> > > [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> > > [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> > > [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
> > >    ...
> > > [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> > > [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> > > [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> > > [   85.916244]  tcp_sendmsg+0x2c/0x50
> > > [   85.916903]  do_syscall_64+0x11c/0x610
> > >
> > >
> > > My feeling is that we should guard against this when
> > > tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> > > zc == 0 case, I think we need to check if the queue tail is unreadable
> > > and 'goto new_segment' if it is?
> > >
> > > This is a different case than Pavel's patch addresses though, where the
> > > new sendmsg is devmem and write queue tail is readable.
> > >
> >
> > :( Yep looks like we have a couple of bugs in the TX mixing. Sorry
> > about that. We do indeed need to fix this ASAP.
> >
> > I don't think it's enough to check readable vs unreadable, no? Because
> > I think appending io_uring niovs to a devmem skb will still blow up
> > and vise versa, even though both are unreadable, right? Or is io_uring
> > saved from this somehow in both cases?
>
> I think for the above case it is okay because if the current sendmsg()
> is zc==0, then we don't care if the tail skb is iou or devmem as
> skb->unreadable tells us enough to avoid appending the non-zc sendmsg.
>
> I'm realizing this a different mixing issue than what Pavel is seeing
> though, probably needs a separate patch.
>

Yes, you're reproducing a different edge case that results in mixing.
Do you plan to send a fix for that or should I take a look?

-- 
Thanks,
Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Mina Almasry]

On Fri, Jul 24, 2026 at 10:25 AM Mina Almasry <almasrymina@google.com> wrote:
>
> On Thu, Jul 23, 2026 at 11:35 AM Mina Almasry <almasrymina@google.com> wrote:
> >
> > On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> > >
> > > On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > > > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > > > >
> > > > > We should either have net_iov or page backed frags in a single skb,
> > > > > otherwise it blows up down the stack. Don't allow mixing in
> > > > > zerocopy_fill_skb_from_devmem().
> > > > >
> > > > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > > >
> > > > It's true that we don't support mixing niov types and doing so would
> > > > blow up, but this is an unnecessary defensive check imo. The calling
> > > > code should not (and does not, I hope) have an edge case where it
> > > > tries to mix and match niov types.
> > > >
> > > > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > > > may be fine?
> > > >
> > > > Also we don't really support mixing different niov sub-types in an skb
> > > > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > > > and check that it's all devmem niovs specifically.
> > > >
> > > > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > > > same situation on RX, we don't support mixing there (and I hope no
> > > > code path leads to mixing today).
> > >
> > > Hey Mina and Pavel,
> > >
> > > I was able to confirm this mixing case does exist.
> > >
> > > It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> > > == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> > > (though same-frag merge is disallowed).
> > >
> > > Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> > > messages, it can be caught hacking this in:
> > >
> > > /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
> > >  * and non-devmem (page, readable) fragments. Such an skb must never exist.
> > >  */
> > > static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> > > {
> > >         bool readable = false, unreadable = false;
> > >         int i;
> > >
> > >         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > >                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
> > >                         unreadable = true;
> > >                 else
> > >                         readable = true;
> > >         }
> > >         return readable && unreadable;
> > > }
> > >
> > > static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> > > {
> > >         ...
> > >         BUG_ON(!skb || !tcp_skb_pcount(skb));
> > >
> > >         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
> > >                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
> > >                   skb_shinfo(skb)->nr_frags, skb->len);
> > >         ...
> > > }
> > >
> > >
> > > Resulting in:
> > >
> > > [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> > > [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> > > [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
> > >    ...
> > > [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> > > [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> > > [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> > > [   85.916244]  tcp_sendmsg+0x2c/0x50
> > > [   85.916903]  do_syscall_64+0x11c/0x610
> > >
> > >
> > > My feeling is that we should guard against this when
> > > tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> > > zc == 0 case, I think we need to check if the queue tail is unreadable
> > > and 'goto new_segment' if it is?
> > >
> > > This is a different case than Pavel's patch addresses though, where the
> > > new sendmsg is devmem and write queue tail is readable.
> > >
> >
> > :( Yep looks like we have a couple of bugs in the TX mixing. Sorry
> > about that. We do indeed need to fix this ASAP.
> >
> > I don't think it's enough to check readable vs unreadable, no? Because
> > I think appending io_uring niovs to a devmem skb will still blow up
> > and vise versa, even though both are unreadable, right? Or is io_uring
> > saved from this somehow in both cases?
> >
> > If io_uring is vulnerable to this as well, then fixing this becomes a
> > bit more hairy because this is a TCP fast path. We don't have bits in
> > the skb header telling us exactly what the skb memtype is (only
> > readable vs not), so we'll have to check skb_frag_is_net_iov(frags[0])
> > and netmem_to_net_iov(frags[0]->netmem)->niov_type to check the exact
> > type of the skb memtype, and that may be a lot of cachelines to fetch
> > in the fast path. :(
> >
> >
>
> Responding to my question here:
>
> So AFAICT we can't actually have io_uring net_iovs in this path.
> IO_uring ZC deos not support TX, and (the LLM) thinks that if we
> receive an io_uring zc rx packet and forward it, it's still not going
> to hit the tcp_sendmsg_locked path.
>
> So this seems sufficient to me, Reviewed-by: Mina Almasry
> <almasrymina@google.com>
>

Sorry for the spam but I checked the sashiko feedback after responding:

https://sashiko.dev/#/patchset/06f0d5ce07dd8593a69239cfa56745cb9c7d957c.1784717791.git.asml.silence%40gmail.com

I think Sashiko is correct that the EEXIST is not getting propopagated
correctly to the caller by skb_zerocopy_iter_stream(). I think we need
to fix that actually.

The other issue Sashiko is pointing to is the same as what Bobby is
pointing to in the other thread I think.

-- 
Thanks,
Mina

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Bobby Eshleman]

On Fri, Jul 24, 2026 at 10:26:42AM -0700, Mina Almasry wrote:
> On Thu, Jul 23, 2026 at 4:10 PM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> >
> > On Thu, Jul 23, 2026 at 11:35:15AM -0700, Mina Almasry wrote:
> > > On Thu, Jul 23, 2026 at 10:24 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> > > >
> > > > On Wed, Jul 22, 2026 at 12:49:07PM -0700, Mina Almasry wrote:
> > > > > On Wed, Jul 22, 2026 at 3:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > > > > >
> > > > > > We should either have net_iov or page backed frags in a single skb,
> > > > > > otherwise it blows up down the stack. Don't allow mixing in
> > > > > > zerocopy_fill_skb_from_devmem().
> > > > > >
> > > > > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > > > >
> > > > > It's true that we don't support mixing niov types and doing so would
> > > > > blow up, but this is an unnecessary defensive check imo. The calling
> > > > > code should not (and does not, I hope) have an edge case where it
> > > > > tries to mix and match niov types.
> > > > >
> > > > > Maybe a DEBUG_NET_WARN_ON check to help catch bugs in the calling code
> > > > > may be fine?
> > > > >
> > > > > Also we don't really support mixing different niov sub-types in an skb
> > > > > (like IO_URING + DMABUF) afaict, so might as well go the extra mile
> > > > > and check that it's all devmem niovs specifically.
> > > > >
> > > > > And might as well put the check in skb_add_rx_frag_netmem. It's the
> > > > > same situation on RX, we don't support mixing there (and I hope no
> > > > > code path leads to mixing today).
> > > >
> > > > Hey Mina and Pavel,
> > > >
> > > > I was able to confirm this mixing case does exist.
> > > >
> > > > It looks like in tcp_sendmsg_locked() when new message is non-devmem (zc
> > > > == 0) and tcp_write_queue_tail is devmem, the skb collapsing is allowed
> > > > (though same-frag merge is disallowed).
> > > >
> > > > Adding a mode to ncdevmem that sends mixed devmem and non-devmem
> > > > messages, it can be caught hacking this in:
> > > >
> > > > /* DEBUG (DROP ME): detect a TX skb that mixes devmem (net_iov, unreadable)
> > > >  * and non-devmem (page, readable) fragments. Such an skb must never exist.
> > > >  */
> > > > static bool tcp_dbg_skb_frags_mixed(const struct sk_buff *skb)
> > > > {
> > > >         bool readable = false, unreadable = false;
> > > >         int i;
> > > >
> > > >         for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> > > >                 if (skb_frag_is_net_iov(&skb_shinfo(skb)->frags[i]))
> > > >                         unreadable = true;
> > > >                 else
> > > >                         readable = true;
> > > >         }
> > > >         return readable && unreadable;
> > > > }
> > > >
> > > > static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, ...)
> > > > {
> > > >         ...
> > > >         BUG_ON(!skb || !tcp_skb_pcount(skb));
> > > >
> > > >         WARN_ONCE(tcp_dbg_skb_frags_mixed(skb),
> > > >                   "DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=%u len=%u)\n",
> > > >                   skb_shinfo(skb)->nr_frags, skb->len);
> > > >         ...
> > > > }
> > > >
> > > >
> > > > Resulting in:
> > > >
> > > > [   85.908005] DEBUG: TX skb mixes devmem+non-devmem frags (nr_frags=2 len=24)
> > > > [   85.908342] WARNING: net/ipv4/tcp_output.c:1570 at __tcp_transmit_skb+0x9bb/0x1030, CPU#2: ncdevmem/278
> > > > [   85.910646] RIP: 0010:__tcp_transmit_skb+0x9c2/0x1030
> > > >    ...
> > > > [   85.915617]  tcp_write_xmit+0x47b/0x17d0
> > > > [   85.915802]  __tcp_push_pending_frames+0x38/0x100
> > > > [   85.916025]  tcp_sendmsg_locked+0xe51/0x1280
> > > > [   85.916244]  tcp_sendmsg+0x2c/0x50
> > > > [   85.916903]  do_syscall_64+0x11c/0x610
> > > >
> > > >
> > > > My feeling is that we should guard against this when
> > > > tcp_sendmsg_locked() is doing its "new segment or not" calculus.  In the
> > > > zc == 0 case, I think we need to check if the queue tail is unreadable
> > > > and 'goto new_segment' if it is?
> > > >
> > > > This is a different case than Pavel's patch addresses though, where the
> > > > new sendmsg is devmem and write queue tail is readable.
> > > >
> > >
> > > :( Yep looks like we have a couple of bugs in the TX mixing. Sorry
> > > about that. We do indeed need to fix this ASAP.
> > >
> > > I don't think it's enough to check readable vs unreadable, no? Because
> > > I think appending io_uring niovs to a devmem skb will still blow up
> > > and vise versa, even though both are unreadable, right? Or is io_uring
> > > saved from this somehow in both cases?
> >
> > I think for the above case it is okay because if the current sendmsg()
> > is zc==0, then we don't care if the tail skb is iou or devmem as
> > skb->unreadable tells us enough to avoid appending the non-zc sendmsg.
> >
> > I'm realizing this a different mixing issue than what Pavel is seeing
> > though, probably needs a separate patch.
> >
> 
> Yes, you're reproducing a different edge case that results in mixing.
> Do you plan to send a fix for that or should I take a look?

I have a fix in the works and plan on sending it soon.

Best,
Bobby

Re: [PATCH net 1/1] net: devmem: prevent net-iov / page mixing

[Bobby Eshleman]

On Wed, Jul 22, 2026 at 07:58:27PM +0100, Pavel Begunkov wrote:
> On 7/22/26 18:59, Bobby Eshleman wrote:
> > On Wed, Jul 22, 2026 at 11:58:46AM +0100, Pavel Begunkov wrote:
> > > We should either have net_iov or page backed frags in a single skb,
> > > otherwise it blows up down the stack. Don't allow mixing in
> > > zerocopy_fill_skb_from_devmem().
> > > 
> > > Fixes: bd61848900bff ("net: devmem: Implement TX path")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > > ---
> > >   net/core/datagram.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > index c285c6465923..35febc1c25fa 100644
> > > --- a/net/core/datagram.c
> > > +++ b/net/core/datagram.c
> > > @@ -712,6 +712,9 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
> > >   	size_t virt_addr, size, off;
> > >   	struct net_iov *niov;
> > > +	if (i && skb_frags_readable(skb))
> > > +		return -EEXIST;
> > 
> > Are we trying to hit the -EEXIST handler in tcp_sendmsg_locked() so that
> > we start a new skb? If so, I think we might need to plumb this -EEXIST
> > case through skb_zerocopy_iter_stream() too?
> 
> Easier to EFAULT. It'd more consistent, and I don't care how tcp takes
> it specifically.
> 
> -- 
> Pavel Begunkov
> 

One socket is allowed to queue non-devmem followed by devmem, and EFAULT
will break this case. I think EEXIST is right, just needs to propagate
up.

Best,
Bobby