* [PATCH net-next] net: add missing check for TCP fraglist GRO
@ 2024-05-07 9:41 Felix Fietkau
2024-05-07 11:33 ` [EXTERNAL] " Suman Ghosh
2024-05-07 12:08 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Felix Fietkau @ 2024-05-07 9:41 UTC (permalink / raw
To: netdev, Eric Dumazet, David S. Miller, David Ahern,
Jakub Kicinski, Paolo Abeni, Willem de Bruijn
Cc: linux-kernel
It turns out that the existing checks do not guarantee that the skb can be
pulled up to the GRO offset. When using the usb r8152 network driver with
GRO fraglist, the BUG() in __skb_pull is often triggered.
Fix the crash by adding the missing check.
Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/ipv4/tcp_offload.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index c90704befd7b..a71d2e623f0c 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -353,6 +353,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb,
flush |= (__force int)(flags ^ tcp_flag_word(th2));
flush |= skb->ip_summed != p->ip_summed;
flush |= skb->csum_level != p->csum_level;
+ flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
flush |= NAPI_GRO_CB(p)->count >= 64;
if (flush || skb_gro_receive_list(p, skb))
--
2.44.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH net-next] net: add missing check for TCP fraglist GRO
2024-05-07 9:41 [PATCH net-next] net: add missing check for TCP fraglist GRO Felix Fietkau
@ 2024-05-07 11:33 ` Suman Ghosh
2024-05-07 11:43 ` Felix Fietkau
2024-05-07 11:51 ` Willem de Bruijn
2024-05-07 12:08 ` Eric Dumazet
1 sibling, 2 replies; 6+ messages in thread
From: Suman Ghosh @ 2024-05-07 11:33 UTC (permalink / raw
To: Felix Fietkau, netdev@vger.kernel.org, Eric Dumazet,
David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Willem de Bruijn
Cc: linux-kernel@vger.kernel.org
>----------------------------------------------------------------------
>It turns out that the existing checks do not guarantee that the skb can be
>pulled up to the GRO offset. When using the usb r8152 network driver with
>GRO fraglist, the BUG() in __skb_pull is often triggered.
>Fix the crash by adding the missing check.
>
>Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
[Suman] Since this is a fix, this should be pushed to "net".
>Signed-off-by: Felix Fietkau <nbd@nbd.name>
>---
> net/ipv4/tcp_offload.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index
>c90704befd7b..a71d2e623f0c 100644
>--- a/net/ipv4/tcp_offload.c
>+++ b/net/ipv4/tcp_offload.c
>@@ -353,6 +353,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head,
>struct sk_buff *skb,
> flush |= (__force int)(flags ^ tcp_flag_word(th2));
> flush |= skb->ip_summed != p->ip_summed;
> flush |= skb->csum_level != p->csum_level;
>+ flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
> flush |= NAPI_GRO_CB(p)->count >= 64;
>
> if (flush || skb_gro_receive_list(p, skb))
>--
>2.44.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] [PATCH net-next] net: add missing check for TCP fraglist GRO
2024-05-07 11:33 ` [EXTERNAL] " Suman Ghosh
@ 2024-05-07 11:43 ` Felix Fietkau
2024-05-07 11:57 ` Suman Ghosh
2024-05-07 11:51 ` Willem de Bruijn
1 sibling, 1 reply; 6+ messages in thread
From: Felix Fietkau @ 2024-05-07 11:43 UTC (permalink / raw
To: Suman Ghosh, netdev@vger.kernel.org, Eric Dumazet,
David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Willem de Bruijn
Cc: linux-kernel@vger.kernel.org
On 07.05.24 13:33, Suman Ghosh wrote:
>>----------------------------------------------------------------------
>>It turns out that the existing checks do not guarantee that the skb can be
>>pulled up to the GRO offset. When using the usb r8152 network driver with
>>GRO fraglist, the BUG() in __skb_pull is often triggered.
>>Fix the crash by adding the missing check.
>>
>>Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
> [Suman] Since this is a fix, this should be pushed to "net".
No, it is fixing a commit that was just pulled into -next.
- Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH net-next] net: add missing check for TCP fraglist GRO
2024-05-07 11:33 ` [EXTERNAL] " Suman Ghosh
2024-05-07 11:43 ` Felix Fietkau
@ 2024-05-07 11:51 ` Willem de Bruijn
1 sibling, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2024-05-07 11:51 UTC (permalink / raw
To: Suman Ghosh, Felix Fietkau, netdev@vger.kernel.org, Eric Dumazet,
David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Willem de Bruijn
Cc: linux-kernel@vger.kernel.org
Suman Ghosh wrote:
> >----------------------------------------------------------------------
> >It turns out that the existing checks do not guarantee that the skb can be
> >pulled up to the GRO offset. When using the usb r8152 network driver with
> >GRO fraglist, the BUG() in __skb_pull is often triggered.
> >Fix the crash by adding the missing check.
> >
> >Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
> [Suman] Since this is a fix, this should be pushed to "net".
The referenced patch has only landed in net-next yet.
> >Signed-off-by: Felix Fietkau <nbd@nbd.name>
Reviewed-by: Willem de Bruijn <willemb@google.com>
> >---
> > net/ipv4/tcp_offload.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index
> >c90704befd7b..a71d2e623f0c 100644
> >--- a/net/ipv4/tcp_offload.c
> >+++ b/net/ipv4/tcp_offload.c
> >@@ -353,6 +353,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head,
> >struct sk_buff *skb,
> > flush |= (__force int)(flags ^ tcp_flag_word(th2));
> > flush |= skb->ip_summed != p->ip_summed;
> > flush |= skb->csum_level != p->csum_level;
> >+ flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
> > flush |= NAPI_GRO_CB(p)->count >= 64;
The same check already exists in udp_gro_receive, which has for longer
been calling skb_gro_receive_list:
if (!pskb_may_pull(skb, skb_gro_offset(skb))) {
NAPI_GRO_CB(skb)->flush = 1;
return NULL;
}
Alternatively it would make sense to deduplicate the check and move it
to skb_gro_receive_list itself, before
skb_pull(skb, skb_gro_offset(skb));
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH net-next] net: add missing check for TCP fraglist GRO
2024-05-07 11:43 ` Felix Fietkau
@ 2024-05-07 11:57 ` Suman Ghosh
0 siblings, 0 replies; 6+ messages in thread
From: Suman Ghosh @ 2024-05-07 11:57 UTC (permalink / raw
To: Felix Fietkau, netdev@vger.kernel.org, Eric Dumazet,
David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Willem de Bruijn
Cc: linux-kernel@vger.kernel.org
> [Suman] Since this is a fix, this should be pushed to "net".
>
>No, it is fixing a commit that was just pulled into -next.
>
>- Felix
[Suman] Got it. Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: add missing check for TCP fraglist GRO
2024-05-07 9:41 [PATCH net-next] net: add missing check for TCP fraglist GRO Felix Fietkau
2024-05-07 11:33 ` [EXTERNAL] " Suman Ghosh
@ 2024-05-07 12:08 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2024-05-07 12:08 UTC (permalink / raw
To: Felix Fietkau
Cc: netdev, David S. Miller, David Ahern, Jakub Kicinski, Paolo Abeni,
Willem de Bruijn, linux-kernel
On Tue, May 7, 2024 at 11:41 AM Felix Fietkau <nbd@nbd.name> wrote:
>
> It turns out that the existing checks do not guarantee that the skb can be
> pulled up to the GRO offset. When using the usb r8152 network driver with
> GRO fraglist, the BUG() in __skb_pull is often triggered.
Why is it crashing ? I would expect tcp_gro_pull_header() to perform this early.
Please include a stack trace.
> Fix the crash by adding the missing check.
>
> Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
> net/ipv4/tcp_offload.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index c90704befd7b..a71d2e623f0c 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -353,6 +353,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb,
> flush |= (__force int)(flags ^ tcp_flag_word(th2));
> flush |= skb->ip_summed != p->ip_summed;
> flush |= skb->csum_level != p->csum_level;
> + flush |= !pskb_may_pull(skb, skb_gro_offset(skb));
> flush |= NAPI_GRO_CB(p)->count >= 64;
>
> if (flush || skb_gro_receive_list(p, skb))
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-07 12:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07 9:41 [PATCH net-next] net: add missing check for TCP fraglist GRO Felix Fietkau
2024-05-07 11:33 ` [EXTERNAL] " Suman Ghosh
2024-05-07 11:43 ` Felix Fietkau
2024-05-07 11:57 ` Suman Ghosh
2024-05-07 11:51 ` Willem de Bruijn
2024-05-07 12:08 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).