* [PATCH v2 net-next] net: fix GSO for SG-enabled devices.
@ 2021-01-19 16:56 Paolo Abeni
2021-01-20 2:17 ` Xin Long
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2021-01-19 16:56 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Jakub Kicinski, Alexander Duyck, Xin Long
The commit dbd50f238dec ("net: move the hsize check to the else
block in skb_segment") introduced a data corruption for devices
supporting scatter-gather.
The problem boils down to signed/unsigned comparison given
unexpected results: if signed 'hsize' is negative, it will be
considered greater than a positive 'len', which is unsigned.
This commit addresses resorting to the old checks order, so that
'hsize' never has a negative value when compared with 'len'.
v1 -> v2:
- reorder hsize checks instead of explicit cast (Alex)
Bisected-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Fixes: dbd50f238dec ("net: move the hsize check to the else block in skb_segment")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/core/skbuff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e835193cabcc3..cf2c4dcf42579 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3938,10 +3938,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
skb_release_head_state(nskb);
__skb_push(nskb, doffset);
} else {
+ if (hsize < 0)
+ hsize = 0;
if (hsize > len || !sg)
hsize = len;
- else if (hsize < 0)
- hsize = 0;
nskb = __alloc_skb(hsize + doffset + headroom,
GFP_ATOMIC, skb_alloc_rx_flag(head_skb),
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 net-next] net: fix GSO for SG-enabled devices.
2021-01-19 16:56 [PATCH v2 net-next] net: fix GSO for SG-enabled devices Paolo Abeni
@ 2021-01-20 2:17 ` Xin Long
2021-01-20 2:43 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Xin Long @ 2021-01-20 2:17 UTC (permalink / raw)
To: Paolo Abeni
Cc: network dev, David S . Miller, Jakub Kicinski, Alexander Duyck
On Wed, Jan 20, 2021 at 12:57 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> The commit dbd50f238dec ("net: move the hsize check to the else
> block in skb_segment") introduced a data corruption for devices
> supporting scatter-gather.
>
> The problem boils down to signed/unsigned comparison given
> unexpected results: if signed 'hsize' is negative, it will be
> considered greater than a positive 'len', which is unsigned.
>
> This commit addresses resorting to the old checks order, so that
> 'hsize' never has a negative value when compared with 'len'.
>
> v1 -> v2:
> - reorder hsize checks instead of explicit cast (Alex)
>
> Bisected-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Fixes: dbd50f238dec ("net: move the hsize check to the else block in skb_segment")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/core/skbuff.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index e835193cabcc3..cf2c4dcf42579 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3938,10 +3938,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> skb_release_head_state(nskb);
> __skb_push(nskb, doffset);
> } else {
> + if (hsize < 0)
> + hsize = 0;
> if (hsize > len || !sg)
> hsize = len;
> - else if (hsize < 0)
> - hsize = 0;
>
> nskb = __alloc_skb(hsize + doffset + headroom,
> GFP_ATOMIC, skb_alloc_rx_flag(head_skb),
> --
> 2.26.2
>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 net-next] net: fix GSO for SG-enabled devices.
2021-01-20 2:17 ` Xin Long
@ 2021-01-20 2:43 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-01-20 2:43 UTC (permalink / raw)
To: Xin Long, Paolo Abeni; +Cc: network dev, David S . Miller, Alexander Duyck
On Wed, 20 Jan 2021 10:17:44 +0800 Xin Long wrote:
> On Wed, Jan 20, 2021 at 12:57 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > The commit dbd50f238dec ("net: move the hsize check to the else
> > block in skb_segment") introduced a data corruption for devices
> > supporting scatter-gather.
> >
> > The problem boils down to signed/unsigned comparison given
> > unexpected results: if signed 'hsize' is negative, it will be
> > considered greater than a positive 'len', which is unsigned.
> >
> > This commit addresses resorting to the old checks order, so that
> > 'hsize' never has a negative value when compared with 'len'.
> >
> > v1 -> v2:
> > - reorder hsize checks instead of explicit cast (Alex)
> >
> > Bisected-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> > Fixes: dbd50f238dec ("net: move the hsize check to the else block in skb_segment")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
I'm hitting this as well, so applied, thanks!
> > ---
> Reviewed-by: Xin Long <lucien.xin@gmail.com>
One review tag is enough ;) apparently patchwork doesn't know to dedup
them :S
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-20 3:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 16:56 [PATCH v2 net-next] net: fix GSO for SG-enabled devices Paolo Abeni
2021-01-20 2:17 ` Xin Long
2021-01-20 2:43 ` Jakub Kicinski
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.