[PATCH net v2] netrom: fix out-of-bounds read in nr_rx_frame()

[PATCH net v2] netrom: fix out-of-bounds read in nr_rx_frame()

[Stanislav Fort]

Add early pskb_may_pull() validation in nr_rx_frame() to prevent
out-of-bounds reads when processing malformed NET/ROM frames.

The vulnerability occurs when nr_route_frame() accepts frames as
short as NR_NETWORK_LEN (15 bytes) but nr_rx_frame() immediately
accesses the 5-byte transport header at bytes 15-19 without validation.
For CONNREQ frames, additional fields are accessed (window at byte 20,
user address at bytes 21-27, optional BPQ timeout at bytes 35-36).

Attack vector: External AX.25 I-frames with PID=0xCF (NET/ROM) can
reach nr_route_frame() via the AX.25 protocol dispatch mechanism:
  ax25_rcv() -> ax25_rx_iframe() -> ax25_protocol_function(0xCF)
  -> nr_route_frame()

For frames destined to local NET/ROM devices, nr_route_frame() calls
nr_rx_frame() which immediately dereferences unvalidated offsets,
causing out-of-bounds reads that can crash the kernel or leak memory.

Fix by using pskb_may_pull() early to linearize the maximum required
packet size (37 bytes) before any pointer assignments. This prevents
use-after-free issues when pskb_may_pull() reallocates skb->head and
ensures all subsequent accesses are within bounds.

Reported-by: Stanislav Fort <disclosure@aisle.com>
Signed-off-by: Stanislav Fort <disclosure@aisle.com>
---
 net/netrom/af_netrom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 3331669d8e33..3056229dcd20 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -883,7 +883,11 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 
 	/*
 	 *	skb->data points to the netrom frame start
+	 *	Linearize the packet early to avoid use-after-free issues
+	 *	when pskb_may_pull() reallocates skb->head later
 	 */
+	if (!pskb_may_pull(skb, max(NR_NETWORK_LEN + NR_TRANSPORT_LEN + 1 + AX25_ADDR_LEN, 37)))
+		return 0;
 
 	src  = (ax25_address *)(skb->data + 0);
 	dest = (ax25_address *)(skb->data + 7);
-- 
2.39.3 (Apple Git-146)

Re: [PATCH net v2] netrom: fix out-of-bounds read in nr_rx_frame()

[Eric Dumazet]

On Tue, Sep 2, 2025 at 4:27 AM Stanislav Fort <stanislav.fort@aisle.com> wrote:
>
> Add early pskb_may_pull() validation in nr_rx_frame() to prevent
> out-of-bounds reads when processing malformed NET/ROM frames.
>
> The vulnerability occurs when nr_route_frame() accepts frames as
> short as NR_NETWORK_LEN (15 bytes) but nr_rx_frame() immediately
> accesses the 5-byte transport header at bytes 15-19 without validation.
> For CONNREQ frames, additional fields are accessed (window at byte 20,
> user address at bytes 21-27, optional BPQ timeout at bytes 35-36).
>
> Attack vector: External AX.25 I-frames with PID=0xCF (NET/ROM) can
> reach nr_route_frame() via the AX.25 protocol dispatch mechanism:
>   ax25_rcv() -> ax25_rx_iframe() -> ax25_protocol_function(0xCF)
>   -> nr_route_frame()
>
> For frames destined to local NET/ROM devices, nr_route_frame() calls
> nr_rx_frame() which immediately dereferences unvalidated offsets,
> causing out-of-bounds reads that can crash the kernel or leak memory.
>
> Fix by using pskb_may_pull() early to linearize the maximum required
> packet size (37 bytes) before any pointer assignments. This prevents
> use-after-free issues when pskb_may_pull() reallocates skb->head and
> ensures all subsequent accesses are within bounds.
>
> Reported-by: Stanislav Fort <disclosure@aisle.com>
> Signed-off-by: Stanislav Fort <disclosure@aisle.com>
> ---
>  net/netrom/af_netrom.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> index 3331669d8e33..3056229dcd20 100644
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -883,7 +883,11 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
>
>         /*
>          *      skb->data points to the netrom frame start
> +        *      Linearize the packet early to avoid use-after-free issues
> +        *      when pskb_may_pull() reallocates skb->head later
>          */
> +       if (!pskb_may_pull(skb, max(NR_NETWORK_LEN + NR_TRANSPORT_LEN + 1 + AX25_ADDR_LEN, 37)))
> +               return 0;

I am not sure about the minimal packet length we expect from this point.

I was suggesting to use skb_linearize() here, but then add the needed
skb->len checks
of various sizes depending on the context (different places you had
patched earlier)

Thank you.

Re: [PATCH net v2] netrom: fix out-of-bounds read in nr_rx_frame()

[Disclosure]

[-- Attachment #1.1: Type: text/plain, Size: 3563 bytes --]

Hi Eric,

Thanks a lot for the suggestions -- that's way better. I’ve sent v3, which 
follows your plan to linearize and add per-context length 
checks: https://lore.kernel.org/all/20250903181915.6359-1-disclosure@aisle.com/T/#u

- Use skb_linearize() at the start of nr_rx_frame().
- Require skb->len >= NR_NETWORK_LEN + NR_TRANSPORT_LEN before reading base 
fields (offsets 0-19).
- Existing-socket path: for NR_CONNACK, require one extra byte for the 
window (>=21). Keep the BPQ ext detection for len == 22.
- CONNREQ path: require window + user address present (>= 28) before 
reading offsets 20-27. Keep the optional timeout only when len == 37.
- IP-over-NET/ROM path unchanged.

This should avoid both OOB reads and potential UAF from earlier unvalidated 
accesses, and it answers the minimal length concern by checking the exact 
lengths needed at each access point. 

Please let me know if this is good enough or if anything else should be 
changed or improved. 

Thanks a lot!

Best wishes,
Stanislav Fort
Aisle Research

On Tuesday, September 2, 2025 at 3:17:14 PM UTC+3 Eric Dumazet wrote:

> On Tue, Sep 2, 2025 at 4:27 AM Stanislav Fort <stanislav.fort@aisle.com> 
> wrote:
> >
> > Add early pskb_may_pull() validation in nr_rx_frame() to prevent
> > out-of-bounds reads when processing malformed NET/ROM frames.
> >
> > The vulnerability occurs when nr_route_frame() accepts frames as
> > short as NR_NETWORK_LEN (15 bytes) but nr_rx_frame() immediately
> > accesses the 5-byte transport header at bytes 15-19 without validation.
> > For CONNREQ frames, additional fields are accessed (window at byte 20,
> > user address at bytes 21-27, optional BPQ timeout at bytes 35-36).
> >
> > Attack vector: External AX.25 I-frames with PID=0xCF (NET/ROM) can
> > reach nr_route_frame() via the AX.25 protocol dispatch mechanism:
> > ax25_rcv() -> ax25_rx_iframe() -> ax25_protocol_function(0xCF)
> > -> nr_route_frame()
> >
> > For frames destined to local NET/ROM devices, nr_route_frame() calls
> > nr_rx_frame() which immediately dereferences unvalidated offsets,
> > causing out-of-bounds reads that can crash the kernel or leak memory.
> >
> > Fix by using pskb_may_pull() early to linearize the maximum required
> > packet size (37 bytes) before any pointer assignments. This prevents
> > use-after-free issues when pskb_may_pull() reallocates skb->head and
> > ensures all subsequent accesses are within bounds.
> >
> > Reported-by: Stanislav Fort <disclosure@aisle.com>
> > Signed-off-by: Stanislav Fort <disclosure@aisle.com>
> > ---
> > net/netrom/af_netrom.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> > index 3331669d8e33..3056229dcd20 100644
> > --- a/net/netrom/af_netrom.c
> > +++ b/net/netrom/af_netrom.c
> > @@ -883,7 +883,11 @@ int nr_rx_frame(struct sk_buff *skb, struct 
> net_device *dev)
> >
> > /*
> > * skb->data points to the netrom frame start
> > + * Linearize the packet early to avoid use-after-free issues
> > + * when pskb_may_pull() reallocates skb->head later
> > */
> > + if (!pskb_may_pull(skb, max(NR_NETWORK_LEN + NR_TRANSPORT_LEN + 1 + 
> AX25_ADDR_LEN, 37)))
> > + return 0;
>
> I am not sure about the minimal packet length we expect from this point.
>
> I was suggesting to use skb_linearize() here, but then add the needed
> skb->len checks
> of various sizes depending on the context (different places you had
> patched earlier)
>
> Thank you.
>

[-- Attachment #1.2: Type: text/html, Size: 4480 bytes --]

[PATCH net v3] netrom: linearize and validate lengths in nr_rx_frame()

[Stanislav Fort]

Linearize skb and add targeted length checks in nr_rx_frame() to avoid out-of-bounds reads and potential use-after-free when processing malformed NET/ROM frames.

- Linearize skb and require at least NR_NETWORK_LEN + NR_TRANSPORT_LEN (20 bytes) before reading network/transport fields.
- For existing sockets path, ensure NR_CONNACK includes the window byte (>= 21 bytes).
- For CONNREQ handling, ensure window (byte 20) and user address (bytes 21-27) are present (>= 28 bytes).
- Maintain existing BPQ extension handling:
  - NR_CONNACK len == 22 implies 1 extra byte (TTL)
  - NR_CONNREQ len == 37 implies 2 extra bytes (timeout)

Suggested-by: Eric Dumazet <edumazet@google.com>
Reported-by: Stanislav Fort <disclosure@aisle.com>
Signed-off-by: Stanislav Fort <disclosure@aisle.com>
---
 net/netrom/af_netrom.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 3331669d8e33..f0660dd6d3b0 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -885,6 +885,11 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 	 *	skb->data points to the netrom frame start
 	 */
 
+	if (skb_linearize(skb))
+		return 0;
+	if (skb->len < NR_NETWORK_LEN + NR_TRANSPORT_LEN)
+		return 0;
+
 	src  = (ax25_address *)(skb->data + 0);
 	dest = (ax25_address *)(skb->data + 7);
 
@@ -927,6 +932,11 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	if (sk != NULL) {
+		if (frametype == NR_CONNACK &&
+		    skb->len < NR_NETWORK_LEN + NR_TRANSPORT_LEN + 1) {
+			sock_put(sk);
+			return 0;
+		}
 		bh_lock_sock(sk);
 		skb_reset_transport_header(skb);
 
@@ -961,10 +971,14 @@ int nr_rx_frame(struct sk_buff *skb, struct net_device *dev)
 		return 0;
 	}
 
-	sk = nr_find_listener(dest);
+	/* Need window (byte 20) and user address (bytes 21-27) */
+	if (skb->len < NR_NETWORK_LEN + NR_TRANSPORT_LEN + 1 + AX25_ADDR_LEN)
+		return 0;
 
 	user = (ax25_address *)(skb->data + 21);
 
+	sk = nr_find_listener(dest);
+
 	if (sk == NULL || sk_acceptq_is_full(sk) ||
 	    (make = nr_make_new(sk)) == NULL) {
 		nr_transmit_refusal(skb, 0);
-- 
2.39.3 (Apple Git-146)

Re: [PATCH net v3] netrom: linearize and validate lengths in nr_rx_frame()

[Eric Dumazet]

On Wed, Sep 3, 2025 at 11:19 AM Stanislav Fort <stanislav.fort@aisle.com> wrote:
>
> Linearize skb and add targeted length checks in nr_rx_frame() to avoid out-of-bounds reads and potential use-after-free when processing malformed NET/ROM frames.
>
> - Linearize skb and require at least NR_NETWORK_LEN + NR_TRANSPORT_LEN (20 bytes) before reading network/transport fields.
> - For existing sockets path, ensure NR_CONNACK includes the window byte (>= 21 bytes).
> - For CONNREQ handling, ensure window (byte 20) and user address (bytes 21-27) are present (>= 28 bytes).
> - Maintain existing BPQ extension handling:
>   - NR_CONNACK len == 22 implies 1 extra byte (TTL)
>   - NR_CONNREQ len == 37 implies 2 extra bytes (timeout)
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Stanislav Fort <disclosure@aisle.com>
> Signed-off-by: Stanislav Fort <disclosure@aisle.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net v3] netrom: linearize and validate lengths in nr_rx_frame()

[Jakub Kicinski]

On Wed,  3 Sep 2025 21:19:15 +0300 Stanislav Fort wrote:
> Linearize skb and add targeted length checks in nr_rx_frame() to avoid out-of-bounds reads and potential use-after-free when processing malformed NET/ROM frames.
> 
> - Linearize skb and require at least NR_NETWORK_LEN + NR_TRANSPORT_LEN (20 bytes) before reading network/transport fields.
> - For existing sockets path, ensure NR_CONNACK includes the window byte (>= 21 bytes).
> - For CONNREQ handling, ensure window (byte 20) and user address (bytes 21-27) are present (>= 28 bytes).
> - Maintain existing BPQ extension handling:
>   - NR_CONNACK len == 22 implies 1 extra byte (TTL)
>   - NR_CONNREQ len == 37 implies 2 extra bytes (timeout)
> 
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Stanislav Fort <disclosure@aisle.com>
> Signed-off-by: Stanislav Fort <disclosure@aisle.com>

Thanks for the fix! The commit message needs a bit of touching up..

Please wrap it at 74 chars.

Please add a Fixes tag pointing to the commit which introduced the
bugs. Quite possibly Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") in area
of the code base.

Please remove the Reported-by:, it's implicit that the author discovered
the bug if no explicit reported-by tag is provided.

Please add the review tag from Eric when reposting.
-- 
pw-bot: cr