[PATCH v2 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[PATCH v2 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Mashiro Chen]

The ax25_cb fragmentation reassembly accumulator:

  ax25->fraglen += skb->len;

operates on the unsigned short field 'fraglen' declared in ax25_cb:

  unsigned short  paclen, fragno, fraglen;

When fragments accumulate with a combined payload exceeding 65535
bytes, fraglen wraps to near zero.  The subsequent allocation:

  skb = alloc_skb(AX25_MAX_HEADER_LEN + ax25->fraglen, GFP_ATOMIC);

then allocates a tiny buffer.  Every skb_put() call in the copy loop
that follows writes far beyond the allocated headroom, corrupting
the kernel heap.

An attacker on an AX.25 link that supports multi-fragment I-frames
(AX25_SEG_FIRST / AX25_SEG_REM mechanism) can trigger this by
sending enough continuation fragments to wrap the 16-bit counter.
With AX.25 segment numbers limited to 7 bits (max 127 continuation
fragments), a fragment payload of ~516 bytes per fragment is
sufficient to overflow.

Fix mirrors the identical bug fixed in NET/ROM (nr_in.c): check for
overflow before adding skb->len to fraglen, and abort fragment
reassembly cleanly if the limit would be exceeded.

Cc: stable@vger.kernel.org
Cc: linux-hams@vger.kernel.org
Acked-by: Joerg Reuter <jreuter@yaina.de>
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 net/ax25/ax25_in.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index d75b3e9ed93de8..68202c19b19e3f 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -41,6 +41,11 @@ static int ax25_rx_fragment(ax25_cb *ax25, struct sk_buff *skb)
 				/* Enqueue fragment */
 				ax25->fragno = *skb->data & AX25_SEG_REM;
 				skb_pull(skb, 1);	/* skip fragno */
+				if ((unsigned int)ax25->fraglen + skb->len > USHRT_MAX) {
+					skb_queue_purge(&ax25->frag_queue);
+					ax25->fragno = 0;
+					return 1;
+				}
 				ax25->fraglen += skb->len;
 				skb_queue_tail(&ax25->frag_queue, skb);
 
-- 
2.53.0

Re: [PATCH v2 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Jakub Kicinski]

On Thu,  9 Apr 2026 10:50:26 +0800 Mashiro Chen wrote:
> Fix mirrors the identical bug fixed in NET/ROM (nr_in.c): check for
> overflow before adding skb->len to fraglen, and abort fragment
> reassembly cleanly if the limit would be exceeded.

Same problem as reported by Simon on the netrom patch applies here.

nit: I don't think you need to cast ax25->fraglen to unsigned int
in the comparison. since it's added with skb->len it should get
auto-prompted to unsigned int.
-- 
pw-bot: cr

Re: [PATCH v2 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[David Laight]

On Sun, 12 Apr 2026 13:17:51 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu,  9 Apr 2026 10:50:26 +0800 Mashiro Chen wrote:
> > Fix mirrors the identical bug fixed in NET/ROM (nr_in.c): check for
> > overflow before adding skb->len to fraglen, and abort fragment
> > reassembly cleanly if the limit would be exceeded.  
> 
> Same problem as reported by Simon on the netrom patch applies here.
> 
> nit: I don't think you need to cast ax25->fraglen to unsigned int
> in the comparison. since it's added with skb->len it should get
> auto-prompted to unsigned int.

It wouldn't matter if that comparison were signed.

Or change the type of ax25->fraglen to be 32bits and do the
sanity check for overlong packets later in the code.
I had a quick look at the header and the structure hasn't
been size-optimised...

	David

[PATCH v3 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Mashiro Chen]

ax25_rx_fragment() accumulates fragment lengths into ax25_cb->fraglen,
which is an unsigned short. When the total exceeds 65535, fraglen wraps
around to a small value. The subsequent alloc_skb(fraglen) allocates a
too-small buffer, and skb_put() in the copy loop triggers skb_over_panic().

Add pskb_may_pull(skb, 1) at function entry to ensure the segmentation
header byte is in the linear data area before dereferencing skb->data.
This also rejects zero-length skbs, which the original code did not
check for.

Three issues in the overflow error path are also fixed:
First, the current skb, after skb_pull(skb, 1), is neither enqueued
nor freed before returning 1, leaking it. Add kfree_skb(skb) before
the return.
Second, ax25->fraglen is not reset after skb_queue_purge(). Add
ax25->fraglen = 0 to restore a consistent state.
Third, the explicit (unsigned int) cast on fraglen is unnecessary: the
addition with skb->len (unsigned int) promotes fraglen automatically.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 net/ax25/ax25_in.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index 68202c19b19e3f..e1834e11bb0b6a 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -35,15 +35,20 @@ static int ax25_rx_fragment(ax25_cb *ax25, struct sk_buff *skb)
 {
 	struct sk_buff *skbn, *skbo;
 
+	if (!pskb_may_pull(skb, 1))
+		return 0;
+
 	if (ax25->fragno != 0) {
 		if (!(*skb->data & AX25_SEG_FIRST)) {
 			if ((ax25->fragno - 1) == (*skb->data & AX25_SEG_REM)) {
 				/* Enqueue fragment */
 				ax25->fragno = *skb->data & AX25_SEG_REM;
 				skb_pull(skb, 1);	/* skip fragno */
-				if ((unsigned int)ax25->fraglen + skb->len > USHRT_MAX) {
+				if (ax25->fraglen + skb->len > USHRT_MAX) {
+					kfree_skb(skb);
 					skb_queue_purge(&ax25->frag_queue);
 					ax25->fragno = 0;
+					ax25->fraglen = 0;
 					return 1;
 				}
 				ax25->fraglen += skb->len;
-- 
2.53.0

Re: [PATCH v2 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Mashiro Chen]

Hi Jakub, Simon

v3 has addressed the review comments on v2:
1. Add pskb_may_pull(skb, 1) before dereferencing skb->data
2. Remove the unnecessary (unsigned int) cast on fraglen
3. Fix skb leak in overflow path that kfree_skb(skb) before return 1
4. Reset ax25->fraglen = 0 after purge


P.S.:
the reassembly copy loop at ax25_in.c:75 uses 
skb_copy_from_linear_data(skbo, dst, skbo->len), which is equivalent to 
memcpy(skbo->data, dst, skbo->len).
If a queued skbo contains non-linear data, which means data_len > 0, 
this silently reads only the linear head and copies stale data for the 
remainder.
In practice, all AX.25 lower-layer drivers like mkiss and 6pack allocate 
fully linear skbs via dev_alloc_skb(), so this is not currently 
reachable, I think there should be a separated patch to fix this.

73s,
Mashiro Chen

On 4/13/26 05:05, David Laight wrote:
> On Sun, 12 Apr 2026 13:17:51 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
>
>> On Thu,  9 Apr 2026 10:50:26 +0800 Mashiro Chen wrote:
>>> Fix mirrors the identical bug fixed in NET/ROM (nr_in.c): check for
>>> overflow before adding skb->len to fraglen, and abort fragment
>>> reassembly cleanly if the limit would be exceeded.
>> Same problem as reported by Simon on the netrom patch applies here.
>>
>> nit: I don't think you need to cast ax25->fraglen to unsigned int
>> in the comparison. since it's added with skb->len it should get
>> auto-prompted to unsigned int.
> It wouldn't matter if that comparison were signed.
>
> Or change the type of ax25->fraglen to be 32bits and do the
> sanity check for overlong packets later in the code.
> I had a quick look at the header and the structure hasn't
> been size-optimised...
>
> 	David
>

[PATCH v4 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Mashiro Chen]

ax25_rx_fragment() accumulates fragment lengths into ax25_cb->fraglen,
which is an unsigned short. When the total exceeds 65535, fraglen wraps
around to a small value. The subsequent alloc_skb(fraglen) allocates a
too-small buffer, and skb_put() in the copy loop triggers skb_over_panic().

Add pskb_may_pull(skb, 1) at function entry to ensure the segmentation
header byte is in the linear data area before dereferencing skb->data.
This also rejects zero-length skbs, which the original code did not
check for.

Two issues in the overflow error path are also fixed:
First, the current skb, after skb_pull(skb, 1), is neither enqueued
nor freed before returning 1, leaking it. Add kfree_skb(skb) before
the return.
Second, ax25->fraglen is not reset after skb_queue_purge(). Add
ax25->fraglen = 0 to restore a consistent state.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 net/ax25/ax25_in.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index d75b3e9ed93de8..e1834e11bb0b6a 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -35,12 +35,22 @@ static int ax25_rx_fragment(ax25_cb *ax25, struct sk_buff *skb)
 {
 	struct sk_buff *skbn, *skbo;
 
+	if (!pskb_may_pull(skb, 1))
+		return 0;
+
 	if (ax25->fragno != 0) {
 		if (!(*skb->data & AX25_SEG_FIRST)) {
 			if ((ax25->fragno - 1) == (*skb->data & AX25_SEG_REM)) {
 				/* Enqueue fragment */
 				ax25->fragno = *skb->data & AX25_SEG_REM;
 				skb_pull(skb, 1);	/* skip fragno */
+				if (ax25->fraglen + skb->len > USHRT_MAX) {
+					kfree_skb(skb);
+					skb_queue_purge(&ax25->frag_queue);
+					ax25->fragno = 0;
+					ax25->fraglen = 0;
+					return 1;
+				}
 				ax25->fraglen += skb->len;
 				skb_queue_tail(&ax25->frag_queue, skb);
 
-- 
2.53.0

Re: [PATCH v4 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Paolo Abeni]

On 4/13/26 10:49 PM, Mashiro Chen wrote:
> ax25_rx_fragment() accumulates fragment lengths into ax25_cb->fraglen,
> which is an unsigned short. When the total exceeds 65535, fraglen wraps
> around to a small value. The subsequent alloc_skb(fraglen) allocates a
> too-small buffer, and skb_put() in the copy loop triggers skb_over_panic().
> 
> Add pskb_may_pull(skb, 1) at function entry to ensure the segmentation
> header byte is in the linear data area before dereferencing skb->data.
> This also rejects zero-length skbs, which the original code did not
> check for.
> 
> Two issues in the overflow error path are also fixed:
> First, the current skb, after skb_pull(skb, 1), is neither enqueued
> nor freed before returning 1, leaking it. Add kfree_skb(skb) before
> the return.
> Second, ax25->fraglen is not reset after skb_queue_purge(). Add
> ax25->fraglen = 0 to restore a consistent state.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>

we are moving ax25 out of tree:

https://lore.kernel.org/netdev/20260421021824.1293976-1-kuba@kernel.org/

please hold off until Thursday (after that our net PR will land into
mainline), and eventually resend if the code still exists in Linus's
tree at that point.

Thanks,

Paolo

Re: [PATCH v4 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Hugh Blemings]

Hi Paolo, All,

On 21/4/2026 17:29, Paolo Abeni wrote:
> On 4/13/26 10:49 PM, Mashiro Chen wrote:
>> ax25_rx_fragment() accumulates fragment lengths into ax25_cb->fraglen,
>> which is an unsigned short. When the total exceeds 65535, fraglen wraps
>> around to a small value. The subsequent alloc_skb(fraglen) allocates a
>> too-small buffer, and skb_put() in the copy loop triggers skb_over_panic().
>>
>> Add pskb_may_pull(skb, 1) at function entry to ensure the segmentation
>> header byte is in the linear data area before dereferencing skb->data.
>> This also rejects zero-length skbs, which the original code did not
>> check for.
>>
>> Two issues in the overflow error path are also fixed:
>> First, the current skb, after skb_pull(skb, 1), is neither enqueued
>> nor freed before returning 1, leaking it. Add kfree_skb(skb) before
>> the return.
>> Second, ax25->fraglen is not reset after skb_queue_purge(). Add
>> ax25->fraglen = 0 to restore a consistent state.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
> we are moving ax25 out of tree:
>
> https://lore.kernel.org/netdev/20260421021824.1293976-1-kuba@kernel.org/
>
> please hold off until Thursday (after that our net PR will land into
> mainline), and eventually resend if the code still exists in Linus's
> tree at that point.

Is there any flexibility here ?

Jakubs (CC'd) patches to remove unfortunately weren't cross posted to 
linux-hams and so I'm not able to directly reply in netdev

We've had a thread ongoing in linux-hams around the future of 
AX25/ROSE/NETROM for the last week or so and believe we've a path 
towards an orderly exit from the mainline tree, probably towards a 
userspace implementation. This includes a couple of folks who have 
indicated they would be open to overseeing the maintenance of the code 
in the meantime.

We'd hoped to have a period of a few months to do an orderly exit from 
the tree to minimise the impact on the (admittedly small, but non-zero) 
users that build trees/make use of the in kernel support.

Apologies for my lack of familiarity with the process here to deprecate etc.

Cheers/73
Hugh


-- 
I am slowly moving to hugh@blemings.id.au as my main email address.
If you're using hugh@blemings.org please update your address book accordingly.
Thank you :)

Re: [PATCH v4 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Andrew Lunn]

On Tue, Apr 21, 2026 at 06:45:39PM +1000, Hugh Blemings wrote:
> Hi Paolo, All,
> 
> On 21/4/2026 17:29, Paolo Abeni wrote:
> > On 4/13/26 10:49 PM, Mashiro Chen wrote:
> > > ax25_rx_fragment() accumulates fragment lengths into ax25_cb->fraglen,
> > > which is an unsigned short. When the total exceeds 65535, fraglen wraps
> > > around to a small value. The subsequent alloc_skb(fraglen) allocates a
> > > too-small buffer, and skb_put() in the copy loop triggers skb_over_panic().
> > > 
> > > Add pskb_may_pull(skb, 1) at function entry to ensure the segmentation
> > > header byte is in the linear data area before dereferencing skb->data.
> > > This also rejects zero-length skbs, which the original code did not
> > > check for.
> > > 
> > > Two issues in the overflow error path are also fixed:
> > > First, the current skb, after skb_pull(skb, 1), is neither enqueued
> > > nor freed before returning 1, leaking it. Add kfree_skb(skb) before
> > > the return.
> > > Second, ax25->fraglen is not reset after skb_queue_purge(). Add
> > > ax25->fraglen = 0 to restore a consistent state.
> > > 
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
> > we are moving ax25 out of tree:
> > 
> > https://lore.kernel.org/netdev/20260421021824.1293976-1-kuba@kernel.org/
> > 
> > please hold off until Thursday (after that our net PR will land into
> > mainline), and eventually resend if the code still exists in Linus's
> > tree at that point.
> 
> Is there any flexibility here ?
> 
> Jakubs (CC'd) patches to remove unfortunately weren't cross posted to
> linux-hams and so I'm not able to directly reply in netdev
> 
> We've had a thread ongoing in linux-hams around the future of
> AX25/ROSE/NETROM for the last week or so and believe we've a path towards an
> orderly exit from the mainline tree, probably towards a userspace
> implementation. This includes a couple of folks who have indicated they
> would be open to overseeing the maintenance of the code in the meantime.
> 
> We'd hoped to have a period of a few months to do an orderly exit from the
> tree to minimise the impact on the (admittedly small, but non-zero) users
> that build trees/make use of the in kernel support.
> 
> Apologies for my lack of familiarity with the process here to deprecate etc.

I know it is short notice, but there is a conference call today. Jakub
sent this yesterday:

  The bi-weekly call is scheduled for tomorrow at 8:30 am (PT) /
  5:30 pm (~EU), at https://bbb.lwn.net/rooms/ldm-chf-zxx-we7/join

  I'd like to discuss evolution of the process which would prepare
  us for the "AI age" (read: influx of plausibly looking yet entirely
  computer generated patches).

	Andrew

Re: [PATCH v4 net] net: ax25: fix integer overflow in ax25_rx_fragment()

[Jakub Kicinski]

On Tue, 21 Apr 2026 18:45:39 +1000 Hugh Blemings wrote:
> We'd hoped to have a period of a few months to do an orderly exit from 
> the tree to minimise the impact on the (admittedly small, but non-zero) 
> users that build trees/make use of the in kernel support.

Sorry about the CC, I trusted get_maintainer.pl too much, clearly :(

If you only need a few months I'd strongly prefer to delete now.
We are getting multiple "fixes" for this code a day, and if we don't
delete it now we have to process these fixes for 2.5 more months,
until the next merge window. IOW even if we delete now you have
2.5 months until 7.1 is actually released, and of course LTS is
available, too.