[PATCH v4 net] ax25: fix OOB read after address header strip in ax25_rcv()

[PATCH v4 net] ax25: fix OOB read after address header strip in ax25_rcv()

[Ashutosh Desai]

A crafted AX.25 frame with a valid address header but no control or PID
bytes causes skb->len to drop to zero after skb_pull() strips the
address header. The subsequent reads of skb->data[0] and skb->data[1]
are then out of bounds.

Linearize the skb at entry to ax25_rcv() so all subsequent accesses to
skb->data are safe. Then check skb->len before reading the control and
PID bytes, discarding frames that are too short.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
v4: linearize skb at entry to ax25_rcv(); replace pskb_may_pull() with
    skb->len check
v3: remove incorrect Suggested-by; add symptom, Fixes, Cc stable
v2: use pskb_may_pull(skb, 2) instead of skb->len < 2

Link to v3: https://lore.kernel.org/netdev/20260415063654.3831353-1-ashutoshdesai993@gmail.com/
Link to v2: https://lore.kernel.org/netdev/20260409152400.2219716-1-ashutoshdesai993@gmail.com/
Link to v1: https://lore.kernel.org/netdev/20260409012235.2049389-1-ashutoshdesai993@gmail.com/

 net/ax25/ax25_in.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index d75b3e9ed93d..d14ccebf9cdd 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -190,6 +190,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
 	ax25_cb *ax25;
 	ax25_dev *ax25_dev;
 
+	if (skb_linearize(skb))
+		goto free;
+
 	/*
 	 *	Process the AX.25/LAPB frame.
 	 */
@@ -217,6 +220,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
 	 */
 	skb_pull(skb, ax25_addr_size(&dp));
 
+	if (skb->len < 2)
+		goto free;
+
 	/* For our port addresses ? */
 	if (ax25cmp(&dest, dev_addr) == 0 && dp.lastrepeat + 1 == dp.ndigi)
 		mine = 1;
-- 
2.34.1

Re: [PATCH v4 net] ax25: fix OOB read after address header strip in ax25_rcv()

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ax25: fix OOB read after address header strip in ax25_rcv()
    
A crafted AX.25 frame with a valid address header but no control or PID
bytes causes skb->len to drop to zero after skb_pull() strips the
address header. The subsequent reads of skb->data[0] and skb->data[1]
are then out of bounds.

> diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
> index d75b3e9ed93de..d14ccebf9cdd5 100644
> --- a/net/ax25/ax25_in.c
> +++ b/net/ax25/ax25_in.c
> @@ -190,6 +190,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>  	ax25_cb *ax25;
>  	ax25_dev *ax25_dev;
>  
> +	if (skb_linearize(skb))
> +		goto free;

Could calling skb_linearize() before verifying if the device is configured for
AX.25 introduce a regression?

Since ax25_packet_type is registered globally, ax25_rcv() receives all
ETH_P_AX25 packets from any network interface. skb_linearize() performs
memory allocations to coalesce fragmented packets. By performing this before
checking if ax25_dev is NULL, could an attacker send fragmented ETH_P_AX25
packets to an unconfigured interface to force unnecessary memory allocation
and CPU cycles?

Should the skb_linearize() call be moved after the device configuration check?

> +
>  	/*
>  	 *	Process the AX.25/LAPB frame.
>  	 */
[ ... ]
> @@ -217,6 +220,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>  	 */
>  	skb_pull(skb, ax25_addr_size(&dp));
>  
> +	if (skb->len < 2)
> +		goto free;

Does this unconditionally drop all AX.25 LAPB control frames?

Standard AX.25 LAPB control frames (such as SABM, DISC, UA, DM, and RR)
consist solely of a 1-byte Control field and no PID byte. For these connection
management frames, skb->len is exactly 1 after the address header is pulled.
By discarding all frames shorter than 2 bytes, could this break the ability
to establish, acknowledge, or disconnect AX.25 connections?

Should the initial check only verify skb->len >= 1, and any requirement for
skb->len >= 2 be strictly scoped to the code paths that actually parse a PID
byte?

Also, if this skb->len < 2 check is relaxed, there is a pre-existing
out-of-bounds read in ax25_decode() for extended modulus frames.

If a malformed 1-byte frame is received on an extended modulus (AX25_EMODULUS)
connection, ax25_decode() unconditionally reads skb->data[1] for I-frames and
S-frames. It then calls skb_pull(skb, 2) but ignores the return value. Should
ax25_decode() validate that skb->len >= 2 before parsing the second control
byte for extended modulus frames?

Finally, there appears to be a pre-existing use-after-free and list corruption
in the ax25_rcv() error path when allocating memory for ax25->digipeat.

When handling an incoming connection request for a listener socket, the code
physically links the skb into the listener socket's receive queue via
skb_queue_head(&sk->sk_receive_queue, skb). Shortly after, it attempts to
allocate memory. If this allocation fails, the error path explicitly calls
kfree_skb(skb).

However, it fails to unlink the skb from sk->sk_receive_queue. (The subsequent
call to ax25_destroy_socket() flushes the newly created socket's queue, not
the listener socket's queue). This leaves a freed skb in the listener socket's
queue. When a user process later calls accept(), will it dequeue and
dereference the freed skb, resulting in a use-after-free?

[PATCH v5 net] ax25: fix OOB read after address header strip in ax25_rcv()

[Ashutosh Desai]

A crafted AX.25 frame with a valid address header but no control byte
causes skb->len to reach zero after skb_pull() strips the header.
The subsequent reads of skb->data[0] (control) and skb->data[1] (PID)
are then out of bounds.

Linearize the skb after confirming the device is an AX.25 interface.
Guard with skb->len < 1 after the pull - one byte suffices for LAPB
control frames which have no PID byte. Add a separate skb->len < 2
check inside the UI branch before accessing the PID byte.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
v5:
- Move skb_linearize() to after ax25_dev_ax25dev() check; avoids
  unnecessary allocation for frames on non-AX.25 interfaces
- Lower general guard from skb->len < 2 to skb->len < 1; the stricter
  limit incorrectly dropped valid 1-byte LAPB control frames (SABM,
  DISC, UA, DM, RR) which carry no PID byte
- Add explicit skb->len < 2 check inside UI branch before the PID
  byte (skb->data[1]) access
v4:
- Linearize skb at entry to ax25_rcv(); replace pskb_may_pull() with
  skb->len < 2 check (per David Laight review)
v3:
- Remove incorrect Suggested-by; add Fixes:, Cc: stable@
v2:
- Replace skb->len check with pskb_may_pull(skb, 2)

Link to v4: https://lore.kernel.org/netdev/20260417065407.206499-1-ashutoshdesai993@gmail.com/
Link to v3: https://lore.kernel.org/netdev/20260415063654.3831353-1-ashutoshdesai993@gmail.com/
Link to v2: https://lore.kernel.org/netdev/20260409152400.2219716-1-ashutoshdesai993@gmail.com/
Link to v1: https://lore.kernel.org/netdev/20260409012235.2049389-1-ashutoshdesai993@gmail.com/

 net/ax25/ax25_in.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index d75b3e9ed93d..c81d6830af48 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -199,6 +199,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
 	if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL)
 		goto free;
 
+	if (skb_linearize(skb))
+		goto free;
+
 	/*
 	 *	Parse the address header.
 	 */
@@ -217,6 +220,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
 	 */
 	skb_pull(skb, ax25_addr_size(&dp));
 
+	if (skb->len < 1)
+		goto free;
+
 	/* For our port addresses ? */
 	if (ax25cmp(&dest, dev_addr) == 0 && dp.lastrepeat + 1 == dp.ndigi)
 		mine = 1;
@@ -227,6 +233,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	/* UI frame - bypass LAPB processing */
 	if ((*skb->data & ~0x10) == AX25_UI && dp.lastrepeat + 1 == dp.ndigi) {
+		if (skb->len < 2)
+			goto free;
+
 		skb_set_transport_header(skb, 2); /* skip control and pid */
 
 		ax25_send_to_raw(&dest, skb, skb->data[1]);
-- 
2.34.1

Re: [PATCH v5 net] ax25: fix OOB read after address header strip in ax25_rcv()

[David Laight]

On Tue, 21 Apr 2026 05:46:26 +0000
Ashutosh Desai <ashutoshdesai993@gmail.com> wrote:

> A crafted AX.25 frame with a valid address header but no control byte
> causes skb->len to reach zero after skb_pull() strips the header.
> The subsequent reads of skb->data[0] (control) and skb->data[1] (PID)
> are then out of bounds.
> 
> Linearize the skb after confirming the device is an AX.25 interface.
> Guard with skb->len < 1 after the pull - one byte suffices for LAPB
> control frames which have no PID byte. Add a separate skb->len < 2
> check inside the UI branch before accessing the PID byte.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
> v5:
> - Move skb_linearize() to after ax25_dev_ax25dev() check; avoids
>   unnecessary allocation for frames on non-AX.25 interfaces

Nitpick: 'on interfaces where AX.25 isn't enabled'
They still have to be AX.25 frames and get discarded.
So they won't really be expected and any allocated memory is
immediately freed.
More relevant would be linearizing before the ax25_addr_parse() call.
In any case I suspect this code never sees non-linear packets.
The packets will all be short, I don't know ax25, but X.25 (which I've
implemented most of in the past) originally had an mtu of 128 bytes
(and real links running at 2400 baud).

	David


> - Lower general guard from skb->len < 2 to skb->len < 1; the stricter
>   limit incorrectly dropped valid 1-byte LAPB control frames (SABM,
>   DISC, UA, DM, RR) which carry no PID byte
> - Add explicit skb->len < 2 check inside UI branch before the PID
>   byte (skb->data[1]) access
> v4:
> - Linearize skb at entry to ax25_rcv(); replace pskb_may_pull() with
>   skb->len < 2 check (per David Laight review)
> v3:
> - Remove incorrect Suggested-by; add Fixes:, Cc: stable@
> v2:
> - Replace skb->len check with pskb_may_pull(skb, 2)
> 
> Link to v4: https://lore.kernel.org/netdev/20260417065407.206499-1-ashutoshdesai993@gmail.com/
> Link to v3: https://lore.kernel.org/netdev/20260415063654.3831353-1-ashutoshdesai993@gmail.com/
> Link to v2: https://lore.kernel.org/netdev/20260409152400.2219716-1-ashutoshdesai993@gmail.com/
> Link to v1: https://lore.kernel.org/netdev/20260409012235.2049389-1-ashutoshdesai993@gmail.com/
> 
>  net/ax25/ax25_in.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
> index d75b3e9ed93d..c81d6830af48 100644
> --- a/net/ax25/ax25_in.c
> +++ b/net/ax25/ax25_in.c
> @@ -199,6 +199,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL)
>  		goto free;
>  
> +	if (skb_linearize(skb))
> +		goto free;
> +
>  	/*
>  	 *	Parse the address header.
>  	 */
> @@ -217,6 +220,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>  	 */
>  	skb_pull(skb, ax25_addr_size(&dp));
>  
> +	if (skb->len < 1)
> +		goto free;
> +
>  	/* For our port addresses ? */
>  	if (ax25cmp(&dest, dev_addr) == 0 && dp.lastrepeat + 1 == dp.ndigi)
>  		mine = 1;
> @@ -227,6 +233,9 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>  
>  	/* UI frame - bypass LAPB processing */
>  	if ((*skb->data & ~0x10) == AX25_UI && dp.lastrepeat + 1 == dp.ndigi) {
> +		if (skb->len < 2)
> +			goto free;
> +
>  		skb_set_transport_header(skb, 2); /* skip control and pid */
>  
>  		ax25_send_to_raw(&dest, skb, skb->data[1]);