* [PATCH net] tcp: ensure skb->dev is NULL before leaving TCP stack
@ 2020-03-19 19:49 Eric Dumazet
2020-03-20 4:36 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Eric Dumazet @ 2020-03-19 19:49 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Martin Zaharinov,
Florian Westphal, Pablo Neira Ayuso
skb->rbnode is sharing three skb fields : next, prev, dev
When a packet is sent, TCP keeps the original skb (master)
in a rtx queue, which was converted to rbtree a while back.
__tcp_transmit_skb() is responsible to clone the master skb,
and add the TCP header to the clone before sending it
to network layer.
skb_clone() already clears skb->next and skb->prev, but copies
the master oskb->dev into the clone.
We need to clear skb->dev, otherwise lower layers could interpret
the value as a pointer to a netdev.
This old bug surfaced recently when commit 28f8bfd1ac94
("netfilter: Support iif matches in POSTROUTING") was merged.
Before this netfilter commit, skb->dev value was ignored and
changed before reaching dev_queue_xmit()
Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
Fixes: 28f8bfd1ac94 ("netfilter: Support iif matches in POSTROUTING")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Martin Zaharinov <micron10@gmail.com>
Cc: Florian Westphal <fw@strlen.de>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/ipv4/tcp_output.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 306e25d743e8de1bfe23d6e3b3a9fb0f23664912..e8cf8fde3d37dc3b455224d33d0bde0e585f989e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1109,6 +1109,10 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
if (unlikely(!skb))
return -ENOBUFS;
+ /* retransmit skbs might have a non zero value in skb->dev
+ * because skb->dev is aliased with skb->rbnode.rb_left
+ */
+ skb->dev = NULL;
}
inet = inet_sk(sk);
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net] tcp: ensure skb->dev is NULL before leaving TCP stack
2020-03-19 19:49 [PATCH net] tcp: ensure skb->dev is NULL before leaving TCP stack Eric Dumazet
@ 2020-03-20 4:36 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2020-03-20 4:36 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, micron10, fw, pablo
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 19 Mar 2020 12:49:55 -0700
> skb->rbnode is sharing three skb fields : next, prev, dev
>
> When a packet is sent, TCP keeps the original skb (master)
> in a rtx queue, which was converted to rbtree a while back.
>
> __tcp_transmit_skb() is responsible to clone the master skb,
> and add the TCP header to the clone before sending it
> to network layer.
>
> skb_clone() already clears skb->next and skb->prev, but copies
> the master oskb->dev into the clone.
>
> We need to clear skb->dev, otherwise lower layers could interpret
> the value as a pointer to a netdev.
>
> This old bug surfaced recently when commit 28f8bfd1ac94
> ("netfilter: Support iif matches in POSTROUTING") was merged.
>
> Before this netfilter commit, skb->dev value was ignored and
> changed before reaching dev_queue_xmit()
>
> Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
> Fixes: 28f8bfd1ac94 ("netfilter: Support iif matches in POSTROUTING")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Martin Zaharinov <micron10@gmail.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-20 4:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 19:49 [PATCH net] tcp: ensure skb->dev is NULL before leaving TCP stack Eric Dumazet
2020-03-20 4:36 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.