Re: [PATCH net v3 2/2] net: tcp: block standard payload injection into devmem skbs

[Eric Dumazet]

On Tue, Aug 11, 2026 at 9:54 PM Mina Almasry <almasrymina@xxxxxxxxxx> wrote:
>
> Protect tcp_sendmsg_locked() from mistakenly appending non-zerocopy
> page fragments to unreadable devmem skbs. Create a new segment instead.
>
> Fixes: bd61848900bff ("net: devmem: Implement TX path")
> Cc: Pavel Begunkov <asml.silence@xxxxxxxxx>
> Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> Cc: Bobby Eshleman <bobbyeshleman@xxxxxxxxx>
> Reviewed-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> Reviewed-by: Bobby Eshleman <bobbyeshleman@xxxxxxxx>
> Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> ---
>  net/ipv4/tcp.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 455441f1b6949..186a36c698798 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1278,6 +1278,11 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>                 if (copy > msg_data_left(msg))
>                         copy = msg_data_left(msg);
>
> +               if (zc != MSG_ZEROCOPY && unlikely(!skb_frags_readable(skb))) {

This seems wrong, as @binding could be NULL or not ?
Also testing the condition right after a fresh skb was allocated is
adding unecessary cost.

> +                       tcp_mark_push(tp, skb);
> +                       goto new_segment;
> +               }
> +
>                 if (zc == 0) {
>                         bool merge = true;
>                         int i = skb_shinfo(skb)->nr_frags;
> --
> 2.55.0.679.g6767b8d81c-goog
>

What about instead:

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 455441f1b694904172cfa1d8e7bac7076b60cb24..b4237d0e994d6f9d754d2167023e3981a40b58f4
100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1240,7 +1240,8 @@ int tcp_sendmsg_locked(struct sock *sk, struct
msghdr *msg, size_t size)

                trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);

-               if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
+               if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
+                   unlikely(skb_frags_readable(skb) != !binding)) {
                        bool first_skb;

 new_segment: