[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

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 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

[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