Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <64ed7188a2745_9cf208e1@penguin.notmuch> <20230831081702.101342-1-mkhalfella@purestorage.com>
In-Reply-To: <20230831081702.101342-1-mkhalfella@purestorage.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Thu, 31 Aug 2023 10:47:21 +0200
Message-ID: <CANn89iLNACt6vh5OAjDmRxpQ3-DkuUP_Vfh9AZGpGinzwJ-1dw@mail.gmail.com>
Subject: Re: [PATCH v3] skbuff: skb_segment, Call zero copy functions before
 using skbuff frags
To:     Mohamed Khalfella <mkhalfella@purestorage.com>
Cc:     willemdebruijn.kernel@gmail.com, alexanderduyck@fb.com,
        bpf@vger.kernel.org, brouer@redhat.com, davem@davemloft.net,
        dhowells@redhat.com, keescook@chromium.org, kuba@kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        pabeni@redhat.com, willemb@google.com, stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Aug 31, 2023 at 10:17=E2=80=AFAM Mohamed Khalfella
<mkhalfella@purestorage.com> wrote:
>
> Commit bf5c25d60861 ("skbuff: in skb_segment, call zerocopy functions
> once per nskb") added the call to zero copy functions in skb_segment().
> The change introduced a bug in skb_segment() because skb_orphan_frags()
> may possibly change the number of fragments or allocate new fragments
> altogether leaving nrfrags and frag to point to the old values. This can
> cause a panic with stacktrace like the one below.
>
>

> In this case calling skb_orphan_frags() updated nr_frags leaving nrfrags
> local variable in skb_segment() stale. This resulted in the code hitting
> i >=3D nrfrags prematurely and trying to move to next frag_skb using
> list_skb pointer, which was NULL, and caused kernel panic. Move the call
> to zero copy functions before using frags and nr_frags.
>
> Fixes: bf5c25d60861 ("skbuff: in skb_segment, call zerocopy functions onc=
e per nskb")
> Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
> Reported-by: Amit Goyal <agoyal@purestorage.com>
> Cc: stable@vger.kernel.org
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>