netdev - Re: [PATCH net] netrom: validate header lengths in nr_rx_frame() using pskb_may

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <41e965f4-6797-44fd-bb7d-490292bbfeaan@aisle.com>
Date: Tue, 2 Sep 2025 04:31:29 -0700 (PDT)
From: Disclosure <disclosure@...le.com>
To: Disclosure <disclosure@...le.com>
Cc: Eric Dumazet <edumazet@...gle.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"security@...nel.org" <security@...nel.org>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH net] netrom: validate header lengths in nr_rx_frame()
 using pskb_may_pull()

Hi Eric, (cc Kuba and others)

Thank you for the technical feedback on the NET/ROM patch. You were 
absolutely right about the use-after-free issue in the previous version. 
That would have made things worse, good point!

I've sent a v2 patch that addresses your concerns:
- Single upfront pskb_may_pull() before any pointer assignments
- Full linearization as you suggested
- Detailed attack vector documentation

The v2 is now on the list: 
https://lore.kernel.org/netdev/20250902112652.26293-1-disclosure@aisle.com/T/#u

I don't have a specific stack trace to show but the call graph flow looks 
convincing enough to me. Given how simple the patch is, could this be 
sufficient?

Thanks for taking the time to review this carefully. Please let me know if 
anything needs to be modified or resent.

Best wishes,
Stanislav Fort
Aisle Research

On Monday, September 1, 2025 at 10:38:05 AM UTC+3 Eric Dumazet wrote:

> On Wed, Aug 27, 2025 at 7:38 AM Stanislav Fort <disclosure@...le.com> 
> wrote:
> >
> > NET/ROM nr_rx_frame() dereferences the 5-byte transport header
> > unconditionally. nr_route_frame() currently accepts frames as short as
> > NR_NETWORK_LEN (15 bytes), which can lead to small out-of-bounds reads
> > on short frames.
> >
> > Fix by using pskb_may_pull() in nr_rx_frame() to ensure the full
> > NET/ROM network + transport header is present before accessing it, and
> > guard the extra fields used by NR_CONNREQ (window, user address, and the
> > optional BPQ timeout extension) with additional pskb_may_pull() checks.
> >
> > This aligns with recent fixes using pskb_may_pull() to validate header
> > availability.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: Stanislav Fort <disclosure@...le.com>
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Stanislav Fort <disclosure@...le.com>
> > ---
> > net/netrom/af_netrom.c | 12 +++++++++++-
> > net/netrom/nr_route.c | 2 +-
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
> > index 3331669d8e33..1fbaa161288a 100644
> > --- a/net/netrom/af_netrom.c
> > +++ b/net/netrom/af_netrom.c
> > @@ -885,6 +885,10 @@ int nr_rx_frame(struct sk_buff *skb, struct 
> net_device *dev)
> > * skb->data points to the netrom frame start
> > */
> >
> > + /* Ensure NET/ROM network + transport header are present */
> > + if (!pskb_may_pull(skb, NR_NETWORK_LEN + NR_TRANSPORT_LEN))
> > + return 0;
> > +
> > src = (ax25_address *)(skb->data + 0);
> > dest = (ax25_address *)(skb->data + 7);
> >
> > @@ -961,6 +965,12 @@ int nr_rx_frame(struct sk_buff *skb, struct 
> net_device *dev)
> > return 0;
> > }
> >
> > + /* Ensure NR_CONNREQ fields (window + user address) are present */
> > + if (!pskb_may_pull(skb, 21 + AX25_ADDR_LEN)) {
>
> If skb->head is reallocated by this pskb_may_pull(), dest variable
> might point to a freed piece of memory
>
> (old skb->head)
>
> As far as netrom is concerned, I would force a full linearization of
> the packet very early
>
> It is also unclear if the bug even exists in the first place.
>
> Can you show the stack trace leading to this function being called
> from an arbitrary
> provider (like a packet being fed by malicious user space)
>
> For instance nr_rx_frame() can be called from net/netrom/nr_loopback.c
> with non malicious packet.
>
> For the remaining caller (nr_route_frame()), it is unclear to me.
>

Content of type "text/html" skipped