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

On Mon, Apr 13, 2026 at 11:04=E2=80=AFPM Ashutosh Desai
<[email protected]> 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: [email protected]
> Signed-off-by: Ashutosh Desai <[email protected]>
> ---
> 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-ashutoshdesai=
[email protected]/
>
>  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 =3D=3D ROSE_STATE_0)
>   0;
>
> +if (!pskb_may_pull(skb, 3)) {
> +kfree_skb(skb);
> +return 0;
> +}
> +
>  frametype =3D rose_decode(skb, &ns, &nr, &q, &d, &m);
>
> +if (frametype =3D=3D ROSE_CLEAR_REQUEST && !pskb_may_pull(skb, 5)) {
> +kfree_skb(skb);
> +return 0;
> +}
> +
>  switch (rose->state) {
>  case ROSE_STATE_1:
>  ueued =3D 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=3D'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