linux-kernel - Re: [PATCH net] net: atlantic: fix fragment overflow handling in RX path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251119190333.398954bf@kernel.org>
Date: Wed, 19 Nov 2025 19:03:33 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Jiefeng <jiefeng.z.zhang@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, pabeni@...hat.com,
 andrew+netdev@...n.ch, edumazet@...gle.com, linux-kernel@...r.kernel.org,
 irusskikh@...vell.com
Subject: Re: [PATCH net] net: atlantic: fix fragment overflow handling in RX
 path

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().