[PATCH v2] rose: fix OOB reads on short CLEAR REQUEST frames

[PATCH v2] rose: fix OOB reads on short CLEAR REQUEST frames

[Ashutosh Desai]

rose_process_rx_frame() calls rose_decode() which reads skb->data[2]
without any prior length check. For CLEAR REQUEST frames the state
machines then read skb->data[3] and skb->data[4] as the cause and
diagnostic bytes.

A crafted 3-byte ROSE CLEAR REQUEST frame passes the minimum length
gate in rose_route_frame() and reaches rose_process_rx_frame(), where
rose_decode() reads one byte past the header and the state machines
read two bytes past the valid buffer.

Add a pskb_may_pull(skb, 3) check before rose_decode() to cover its
skb->data[2] access, and a pskb_may_pull(skb, 5) check afterwards for
the CLEAR REQUEST path to cover the cause and diagnostic reads.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
V1 -> V2: switch skb->len check to pskb_may_pull; also add
          pskb_may_pull(skb, 3) before rose_decode() to cover its
          skb->data[2] access

v1: https://lore.kernel.org/netdev/20260409013246.2051746-1-ashutoshdesai993@gmail.com/

 net/rose/rose_in.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0276b393f0e5..b9f01a11e2df 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -269,8 +269,18 @@ int rose_process_rx_frame(struct sock *sk, struct sk_buff *skb)
 if (rose->state == ROSE_STATE_0)
  0;
 
+if (!pskb_may_pull(skb, 3)) {
+kfree_skb(skb);
+return 0;
+}
+
 frametype = rose_decode(skb, &ns, &nr, &q, &d, &m);
 
+if (frametype == ROSE_CLEAR_REQUEST && !pskb_may_pull(skb, 5)) {
+kfree_skb(skb);
+return 0;
+}
+
 switch (rose->state) {
 case ROSE_STATE_1:
 ueued = rose_state1_machine(sk, skb, frametype);
-- 
2.34.1

Re: [PATCH v2] rose: fix OOB reads on short CLEAR REQUEST frames

[Eric Dumazet]

On Mon, Apr 13, 2026 at 11:04 PM Ashutosh Desai
<ashutoshdesai993@gmail.com> wrote:
>
> rose_process_rx_frame() calls rose_decode() which reads skb->data[2]
> without any prior length check. For CLEAR REQUEST frames the state
> machines then read skb->data[3] and skb->data[4] as the cause and
> diagnostic bytes.
>
> A crafted 3-byte ROSE CLEAR REQUEST frame passes the minimum length
> gate in rose_route_frame() and reaches rose_process_rx_frame(), where
> rose_decode() reads one byte past the header and the state machines
> read two bytes past the valid buffer.
>
> Add a pskb_may_pull(skb, 3) check before rose_decode() to cover its
> skb->data[2] access, and a pskb_may_pull(skb, 5) check afterwards for
> the CLEAR REQUEST path to cover the cause and diagnostic reads.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
> V1 -> V2: switch skb->len check to pskb_may_pull; also add
>           pskb_may_pull(skb, 3) before rose_decode() to cover its
>           skb->data[2] access
>
> v1: https://lore.kernel.org/netdev/20260409013246.2051746-1-ashutoshdesai993@gmail.com/
>
>  net/rose/rose_in.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
> index 0276b393f0e5..b9f01a11e2df 100644
> --- a/net/rose/rose_in.c
> +++ b/net/rose/rose_in.c
> @@ -269,8 +269,18 @@ int rose_process_rx_frame(struct sock *sk, struct sk_buff *skb)
>  if (rose->state == ROSE_STATE_0)
>   0;
>
> +if (!pskb_may_pull(skb, 3)) {
> +kfree_skb(skb);
> +return 0;
> +}
> +
>  frametype = rose_decode(skb, &ns, &nr, &q, &d, &m);
>
> +if (frametype == ROSE_CLEAR_REQUEST && !pskb_may_pull(skb, 5)) {
> +kfree_skb(skb);
> +return 0;
> +}
> +
>  switch (rose->state) {
>  case ROSE_STATE_1:
>  ueued = rose_state1_machine(sk, skb, frametype);
> --
> 2.34.1

rose_process_rx_frame() callers already call kfree_skb(skb) if
rose_process_rx_frame()
returns a 0.
Your patch would add double-frees.


Your patch is white-space mangled.

Please take a look at Documentation/process/maintainer-netdev.rst

Preparing changes
-----------------

Attention to detail is important.  Re-read your own work as if you were the
reviewer.  You can start with using ``checkpatch.pl``, perhaps even with
the ``--strict`` flag.  But do not be mindlessly robotic in doing so.
If your change is a bug fix, make sure your commit log indicates the
end-user visible symptom, the underlying reason as to why it happens,
and then if necessary, explain why the fix proposed is the best way to
get things done.  Don't mangle whitespace, and as is common, don't
mis-indent function arguments that span multiple lines.  If it is your
first patch, mail it to yourself so you can test apply it to an
unpatched tree to confirm infrastructure didn't mangle it.

Finally, go back and read
:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
to be sure you are not repeating some common mistake documented there.

Also:

Indicating target tree
~~~~~~~~~~~~~~~~~~~~~~

To help maintainers and CI bots you should explicitly mark which tree
your patch is targeting. Assuming that you use git, use the prefix
flag::

  git format-patch --subject-prefix='PATCH net-next' start..finish

Use ``net`` instead of ``net-next`` (always lower case) in the above for
bug-fix ``net`` content.

Please

pw-bot: cr

Re: [PATCH v2] rose: fix OOB reads on short CLEAR REQUEST frames

[Ashutosh Desai]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

Hi Eric,

On Mon, Apr 13, 2026 at 11:11 PM Eric Dumazet wrote:
> rose_process_rx_frame() callers already call kfree_skb(skb) if
> rose_process_rx_frame() returns a 0. Your patch would add double-frees.
>
> Your patch is white-space mangled.

Thanks for the review. Sent v3 with all issues addressed.

Best regards,
Ashutosh

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

[Eric Dumazet]

On Wed, Apr 8, 2026 at 6:22 PM Ashutosh Desai
<ashutoshdesai993@gmail.com> wrote:
>
> ax25_rcv() calls skb_pull(skb, ax25_addr_size(&dp)) to strip the
> address header, then immediately reads skb->data[0] and skb->data[1]
> without verifying the buffer still contains at least 2 bytes.
>
> A crafted 15-byte KISS frame (1 KISS byte + 14 address bytes with
> EBIT set in the source address, no control/PID bytes) passes
> ax25_addr_parse() which only requires len >= 14, and passes the KISS
> byte check (low nibble == 0). After skb_pull(1) in ax25_kiss_rcv()
> and skb_pull(14) in ax25_rcv(), skb->len is 0 and the subsequent
> reads of skb->data[0] (control byte) and skb->data[1] (PID byte)
> are out of bounds.
>
> Add a check that at least 2 bytes remain after stripping the address
> header, freeing the skb and returning on malformed input.
>
> Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
> ---
>  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 d75b3e9ed..92baac77f 100644
> --- a/net/ax25/ax25_in.c
> +++ b/net/ax25/ax25_in.c
> @@ -217,6 +217,11 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
>          */
>         skb_pull(skb, ax25_addr_size(&dp));
>
> +       if (skb->len < 2) {
> +               kfree_skb(skb);
> +               return 0;
> +       }
> +

Are you aware of pskb_may_pull() ?

Testing skb->len is not enough.

I suspect all net/ax25 is expecting linear skbs and never considered fragments.

[PATCH v2] rose: fix OOB reads on short CLEAR REQUEST frames.

[Ashutosh Desai]

rose_process_rx_frame() passes skb directly to rose_decode(), which
reads skb->data[2] without any prior length check. For CLEAR REQUEST
frames the state machines then read skb->data[3] and skb->data[4] as
the cause and diagnostic bytes.

The original fix checked skb->len after the rose_decode() call, which
was wrong for two reasons: rose_decode() already accessed skb->data[2]
before the check ran, and skb->len counts bytes across non-linear
fragments while skb->data only covers the linear head - so even a
passing len check doesn't guarantee the bytes are safe to read directly.

Eric Dumazet pointed out that pskb_may_pull() is the right approach
here. Add a pskb_may_pull(skb, 3) check before rose_decode() to cover
its frame[2] access, and a pskb_may_pull(skb, 5) check afterwards for
the CLEAR REQUEST path to cover the cause and diagnostic reads.

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
---
 net/rose/rose_in.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0276b393f..b9f01a11e 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -269,8 +269,18 @@ int rose_process_rx_frame(struct sock *sk, struct sk_buff *skb)
 	if (rose->state == ROSE_STATE_0)
 		return 0;
 
+	if (!pskb_may_pull(skb, 3)) {
+		kfree_skb(skb);
+		return 0;
+	}
+
 	frametype = rose_decode(skb, &ns, &nr, &q, &d, &m);
 
+	if (frametype == ROSE_CLEAR_REQUEST && !pskb_may_pull(skb, 5)) {
+		kfree_skb(skb);
+		return 0;
+	}
+
 	switch (rose->state) {
 	case ROSE_STATE_1:
 		queued = rose_state1_machine(sk, skb, frametype);
-- 
2.34.1