[PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[Shigeru Yoshida]

qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
NULL return from qede_build_skb(). When it returns NULL under memory
pressure, the functions still consume a BD from the ring before
returning NULL. The callers then recycle additional BDs, resulting in
one extra BD being consumed (off-by-one). This desynchronizes the BD
ring, which can corrupt DMA page reference counts and lead to SLUB
freelist corruption.

Commit 4e910dbe3650 ("qede: confirm skb is allocated before using")
added a NULL check inside qede_build_skb() to prevent a NULL pointer
dereference, but did not address the missing NULL checks in the
callers, making this off-by-one reachable.

Fix this by adding NULL checks for the return value of
qede_build_skb() in both qede_rx_build_skb() and
qede_tpa_rx_build_skb(), returning NULL immediately before any BD ring
manipulation.

Fixes: 4e910dbe3650 ("qede: confirm skb is allocated before using")
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
 drivers/net/ethernet/qlogic/qede/qede_fp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 33e18bb69774..c11e0d8f98aa 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -765,6 +765,9 @@ qede_tpa_rx_build_skb(struct qede_dev *edev,
 	struct sk_buff *skb;
 
 	skb = qede_build_skb(rxq, bd, len, pad);
+	if (unlikely(!skb))
+		return NULL;
+
 	bd->page_offset += rxq->rx_buf_seg_size;
 
 	if (bd->page_offset == PAGE_SIZE) {
@@ -812,6 +815,8 @@ qede_rx_build_skb(struct qede_dev *edev,
 	}
 
 	skb = qede_build_skb(rxq, bd, len, pad);
+	if (unlikely(!skb))
+		return NULL;
 
 	if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
 		/* Incr page ref count to reuse on allocation failure so
-- 
2.54.0

Re: [PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[Jamie Bainbridge]

On Wed, 1 Jul 2026 at 02:47, Shigeru Yoshida <syoshida@redhat.com> wrote:
>
> qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
> NULL return from qede_build_skb(). When it returns NULL under memory
> pressure, the functions still consume a BD from the ring before
> returning NULL. The callers then recycle additional BDs, resulting in
> one extra BD being consumed (off-by-one). This desynchronizes the BD
> ring, which can corrupt DMA page reference counts and lead to SLUB
> freelist corruption.

Good catch.

Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>

> Commit 4e910dbe3650 ("qede: confirm skb is allocated before using")
> added a NULL check inside qede_build_skb() to prevent a NULL pointer
> dereference, but did not address the missing NULL checks in the
> callers, making this off-by-one reachable.
>
> Fix this by adding NULL checks for the return value of
> qede_build_skb() in both qede_rx_build_skb() and
> qede_tpa_rx_build_skb(), returning NULL immediately before any BD ring
> manipulation.
>
> Fixes: 4e910dbe3650 ("qede: confirm skb is allocated before using")
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
>  drivers/net/ethernet/qlogic/qede/qede_fp.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> index 33e18bb69774..c11e0d8f98aa 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> @@ -765,6 +765,9 @@ qede_tpa_rx_build_skb(struct qede_dev *edev,
>         struct sk_buff *skb;
>
>         skb = qede_build_skb(rxq, bd, len, pad);
> +       if (unlikely(!skb))
> +               return NULL;
> +
>         bd->page_offset += rxq->rx_buf_seg_size;
>
>         if (bd->page_offset == PAGE_SIZE) {
> @@ -812,6 +815,8 @@ qede_rx_build_skb(struct qede_dev *edev,
>         }
>
>         skb = qede_build_skb(rxq, bd, len, pad);
> +       if (unlikely(!skb))
> +               return NULL;
>
>         if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
>                 /* Incr page ref count to reuse on allocation failure so
> --
> 2.54.0
>

Re: [PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[Jamie Bainbridge]

On Fri, 3 Jul 2026 at 09:52, Jamie Bainbridge
<jamie.bainbridge@gmail.com> wrote:
>
> On Wed, 1 Jul 2026 at 02:47, Shigeru Yoshida <syoshida@redhat.com> wrote:
> >
> > qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
> > NULL return from qede_build_skb(). When it returns NULL under memory
> > pressure, the functions still consume a BD from the ring before
> > returning NULL. The callers then recycle additional BDs, resulting in
> > one extra BD being consumed (off-by-one). This desynchronizes the BD
> > ring, which can corrupt DMA page reference counts and lead to SLUB
> > freelist corruption.
>
> Good catch.
>
> Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>

Sorry for the double mail.

I believe the Fixes: should be against the problematic original code:

Fixes: 8a8633978b842 ("qede: Add build_skb() support.")

because that is what you are fixing.

Jamie

> > Commit 4e910dbe3650 ("qede: confirm skb is allocated before using")
> > added a NULL check inside qede_build_skb() to prevent a NULL pointer
> > dereference, but did not address the missing NULL checks in the
> > callers, making this off-by-one reachable.
> >
> > Fix this by adding NULL checks for the return value of
> > qede_build_skb() in both qede_rx_build_skb() and
> > qede_tpa_rx_build_skb(), returning NULL immediately before any BD ring
> > manipulation.
> >
> > Fixes: 4e910dbe3650 ("qede: confirm skb is allocated before using")
> > Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> > ---
> >  drivers/net/ethernet/qlogic/qede/qede_fp.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> > index 33e18bb69774..c11e0d8f98aa 100644
> > --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> > +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> > @@ -765,6 +765,9 @@ qede_tpa_rx_build_skb(struct qede_dev *edev,
> >         struct sk_buff *skb;
> >
> >         skb = qede_build_skb(rxq, bd, len, pad);
> > +       if (unlikely(!skb))
> > +               return NULL;
> > +
> >         bd->page_offset += rxq->rx_buf_seg_size;
> >
> >         if (bd->page_offset == PAGE_SIZE) {
> > @@ -812,6 +815,8 @@ qede_rx_build_skb(struct qede_dev *edev,
> >         }
> >
> >         skb = qede_build_skb(rxq, bd, len, pad);
> > +       if (unlikely(!skb))
> > +               return NULL;
> >
> >         if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
> >                 /* Incr page ref count to reuse on allocation failure so
> > --
> > 2.54.0
> >

Re: [PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[Shigeru Yoshida]

Jamie Bainbridge <jamie.bainbridge@gmail.com> writes:

> On Fri, 3 Jul 2026 at 09:52, Jamie Bainbridge
> <jamie.bainbridge@gmail.com> wrote:
>>
>> On Wed, 1 Jul 2026 at 02:47, Shigeru Yoshida <syoshida@redhat.com> wrote:
>> >
>> > qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
>> > NULL return from qede_build_skb(). When it returns NULL under memory
>> > pressure, the functions still consume a BD from the ring before
>> > returning NULL. The callers then recycle additional BDs, resulting in
>> > one extra BD being consumed (off-by-one). This desynchronizes the BD
>> > ring, which can corrupt DMA page reference counts and lead to SLUB
>> > freelist corruption.
>>
>> Good catch.
>>
>> Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
>
> Sorry for the double mail.
>
> I believe the Fixes: should be against the problematic original code:
>
> Fixes: 8a8633978b842 ("qede: Add build_skb() support.")
>
> because that is what you are fixing.

Thank you for your review.

The problematic code was introduced in commit 8a8633978b84 ("qede: Add
build_skb() support."), so putting this in the Fixes tag would be
correct.

I'll send the v2 patch with the modified Fixes tag.

Thank you,
Shigeru

> Jamie
>
>> > Commit 4e910dbe3650 ("qede: confirm skb is allocated before using")
>> > added a NULL check inside qede_build_skb() to prevent a NULL pointer
>> > dereference, but did not address the missing NULL checks in the
>> > callers, making this off-by-one reachable.
>> >
>> > Fix this by adding NULL checks for the return value of
>> > qede_build_skb() in both qede_rx_build_skb() and
>> > qede_tpa_rx_build_skb(), returning NULL immediately before any BD ring
>> > manipulation.
>> >
>> > Fixes: 4e910dbe3650 ("qede: confirm skb is allocated before using")
>> > Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
>> > ---
>> >  drivers/net/ethernet/qlogic/qede/qede_fp.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
>> > index 33e18bb69774..c11e0d8f98aa 100644
>> > --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
>> > +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
>> > @@ -765,6 +765,9 @@ qede_tpa_rx_build_skb(struct qede_dev *edev,
>> >         struct sk_buff *skb;
>> >
>> >         skb = qede_build_skb(rxq, bd, len, pad);
>> > +       if (unlikely(!skb))
>> > +               return NULL;
>> > +
>> >         bd->page_offset += rxq->rx_buf_seg_size;
>> >
>> >         if (bd->page_offset == PAGE_SIZE) {
>> > @@ -812,6 +815,8 @@ qede_rx_build_skb(struct qede_dev *edev,
>> >         }
>> >
>> >         skb = qede_build_skb(rxq, bd, len, pad);
>> > +       if (unlikely(!skb))
>> > +               return NULL;
>> >
>> >         if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
>> >                 /* Incr page ref count to reuse on allocation failure so
>> > --
>> > 2.54.0
>> >

Re: [PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[Paolo Abeni]

On 7/5/26 5:53 PM, Shigeru Yoshida wrote:
> Jamie Bainbridge <jamie.bainbridge@gmail.com> writes:
> 
>> On Fri, 3 Jul 2026 at 09:52, Jamie Bainbridge
>> <jamie.bainbridge@gmail.com> wrote:
>>>
>>> On Wed, 1 Jul 2026 at 02:47, Shigeru Yoshida <syoshida@redhat.com> wrote:
>>>>
>>>> qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
>>>> NULL return from qede_build_skb(). When it returns NULL under memory
>>>> pressure, the functions still consume a BD from the ring before
>>>> returning NULL. The callers then recycle additional BDs, resulting in
>>>> one extra BD being consumed (off-by-one). This desynchronizes the BD
>>>> ring, which can corrupt DMA page reference counts and lead to SLUB
>>>> freelist corruption.
>>>
>>> Good catch.
>>>
>>> Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
>>
>> Sorry for the double mail.
>>
>> I believe the Fixes: should be against the problematic original code:
>>
>> Fixes: 8a8633978b842 ("qede: Add build_skb() support.")
>>
>> because that is what you are fixing.
> 
> Thank you for your review.
> 
> The problematic code was introduced in commit 8a8633978b84 ("qede: Add
> build_skb() support."), so putting this in the Fixes tag would be
> correct.
> 
> I'll send the v2 patch with the modified Fixes tag.

No need for a v2: I'll update the fixes tag while applying the patch.
The PW is already quite significant and I think it's currently better to
avoid repost if possible.

Thanks!

Paolo

Re: [PATCH net] qede: fix off-by-one in BD ring consumption on build_skb failure

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed,  1 Jul 2026 01:46:20 +0900 you wrote:
> qede_rx_build_skb() and qede_tpa_rx_build_skb() do not check for a
> NULL return from qede_build_skb(). When it returns NULL under memory
> pressure, the functions still consume a BD from the ring before
> returning NULL. The callers then recycle additional BDs, resulting in
> one extra BD being consumed (off-by-one). This desynchronizes the BD
> ring, which can corrupt DMA page reference counts and lead to SLUB
> freelist corruption.
> 
> [...]

Here is the summary with links:
  - [net] qede: fix off-by-one in BD ring consumption on build_skb failure
    https://git.kernel.org/netdev/net/c/a0a558ca7e75

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html