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

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 07/27, Pavel Begunkov wrote:
> On 7/27/26 17:46, Stanislav Fomichev wrote:
> > On 07/27, 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>
> > > ---
> > > 
> > > v2: EEXIST -> EFAULT, as skb_zerocopy_iter_stream() doesn't handle
> > >      the former + for consistency.
> > > 
> > >   net/core/datagram.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > > index c285c6465923..173b5d97bd40 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 -EFAULT;
> > > +
> > 
> > Maybe we should do -EMSGSIZE? It is already properly plumbed via
> > skb_zerocopy_iter_stream (and you'll hit 'skb->len == orig_len')
> > and it hits 'new_segment' in tcp_sendmsg_locked?
> 
> I made it consistent with zerocopy_fill_skb_from_iter(), don't see
> the point of making it behaving differently from combination to
> combination. Do you have some use case for that?

Ah, ok, yeah, that makes sense, let's go with that!

Acked-by: Stanislav Fomichev <sdf@fomichev.me>