From: Jakub Kicinski <kuba@kernel.org>
To: Jiefeng <jiefeng.z.zhang@gmail.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, pabeni@redhat.com,
andrew+netdev@lunn.ch, edumazet@google.com,
linux-kernel@vger.kernel.org, irusskikh@marvell.com
Subject: Re: [PATCH net] net: atlantic: fix fragment overflow handling in RX path
Date: Wed, 19 Nov 2025 19:03:33 -0800 [thread overview]
Message-ID: <20251119190333.398954bf@kernel.org> (raw)
In-Reply-To: <CADEc0q4sEACJY03CYxOWPPvPrB=n7==2KqHz57AY+CR6gSJjAw@mail.gmail.com>
On Wed, 19 Nov 2025 16:38:13 +0800 Jiefeng wrote:
> And I have encountered this crash in production with an
> Aquantia(AQtion AQC113) 10G NIC[Antigua 10G]:
Ah you're actually seeing a crash! Thanks a lot for the additional info,
I thought this is something you found with static code analysis!
Please include the stack trace and more info in the commit message,
makes it easier for others encountering the crash to compare.
(Drop the timestamps from the crash lines, tho, it's not important)
> The atlantic hardware supports multi-descriptor packet reception
> (RSC). When a large packet arrives, the hardware splits it across
> multiple descriptors, where each descriptor can hold up to 2048 bytes
> (AQ_CFG_RX_FRAME_MAX).
>
> There is a logic bug in the code. The code already counts fragments in
> the multi-descriptor loop
> (frag_cnt at aq_ring.c:559), but the check at aq_ring.c:568 only considers
> frag_cnt, not the additional fragment from the first buffer
> (if buff->len > hdr_len). The actual fragment addition happens later
> (aq_ring.c:634-667):
> - One fragment from the first buffer (if hdr_len < buff->len)
> - Plus frag_cnt fragments from subsequent descriptors
>
> This can exceed MAX_SKB_FRAGS (17) in edge cases:
> - If frag_cnt = 17 (the check passes)
> - And the first buffer has a fragment (buff->len > hdr_len)
> - Then total = 1 + 17 = 18 > MAX_SKB_FRAGS
Got it, would it make more sense to fix the existing check?
(assume there will be an extra frag if buff->len > AQ_CFG_RX_HDR_SIZE)
Or fix adding the zeroth frag? (if frag_cnt == max do not extract the
zeroth frag). Extracting the zeroth frag is just to make SW GRO/skb
freeing slightly faster, it's not necessary for correctness.
> While the hardware MTU limit is 16334 bytes (B0/ATL2), which should
> only need ~8 fragments, there are edge cases like LRO aggregation
> or hardware anomalies that could produce more descriptors.
>
> The panic occurred because skb_add_rx_frag() was called with an index
> >= MAX_SKB_FRAGS, causing an out-of-bounds write. The fix ensures
> we check the total fragment count (first buffer fragment + frag_cnt)
> before calling skb_add_rx_frag().
next prev parent reply other threads:[~2025-11-20 3:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-18 7:04 [PATCH net] net: atlantic: fix fragment overflow handling in RX path jiefeng.z.zhang
2025-11-18 20:24 ` Jakub Kicinski
2025-11-19 8:38 ` Jiefeng
2025-11-20 3:03 ` Jakub Kicinski [this message]
2025-11-20 13:06 ` Jiefeng
2025-11-22 1:36 ` Jiefeng
2025-11-22 2:01 ` Jakub Kicinski
2025-11-24 14:08 ` Jiefeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251119190333.398954bf@kernel.org \
--to=kuba@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=irusskikh@marvell.com \
--cc=jiefeng.z.zhang@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).