On Wed, Jan 20, 2021 at 12:57 AM Paolo Abeni <pab...@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.bae...@tessares.net>
> Fixes: dbd50f238dec ("net: move the hsize check to the else block in
> skb_segment")
> Signed-off-by: Paolo Abeni <pab...@redhat.com>
Reviewed-by: Xin Long <lucien....@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....@gmail.com>
