Re: [PATCH net v2] slip: reject VJ receive packets on instances with no rstate array

[Simon Horman <horms@kernel.org>]

On Thu, Apr 16, 2026 at 04:41:31AM +0800, Weiming Shi wrote:
> slhc_init() accepts rslots == 0 as a valid configuration, with the
> documented meaning of 'no receive compression'. In that case the
> allocation loop in slhc_init() is skipped, so comp->rstate stays
> NULL and comp->rslot_limit stays 0 (from the kzalloc of struct
> slcompress).
> 
> The receive helpers do not defend against that configuration.
> slhc_uncompress() dereferences comp->rstate[x] when the VJ header
> carries an explicit connection ID, and slhc_remember() later assigns
> cs = &comp->rstate[...] after only comparing the packet's slot number
> to comp->rslot_limit. Because rslot_limit is 0, slot 0 passes the
> range check, and the code dereferences a NULL rstate.
> 
> The configuration is reachable in-tree through PPP. PPPIOCSMAXCID
> stores its argument in a signed int, and (val >> 16) uses arithmetic
> shift. Passing 0xffff0000 therefore sign-extends to -1, so val2 + 1
> is 0 and ppp_generic.c ends up calling slhc_init(0, 1). Because
> /dev/ppp open is gated by ns_capable(CAP_NET_ADMIN), the whole path
> is reachable from an unprivileged user namespace. Once the malformed
> VJ state is installed, any inbound VJ-compressed or VJ-uncompressed
> frame that selects slot 0 crashes the kernel in softirq context:
> 
>  Oops: general protection fault, probably for non-canonical
>        address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
>  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>  RIP: 0010:slhc_uncompress (drivers/net/slip/slhc.c:519)
>  Call Trace:
>   <TASK>
>   ppp_receive_nonmp_frame (drivers/net/ppp/ppp_generic.c:2466)
>   ppp_input (drivers/net/ppp/ppp_generic.c:2359)
>   ppp_async_process (drivers/net/ppp/ppp_async.c:492)
>   tasklet_action_common (kernel/softirq.c:926)
>   handle_softirqs (kernel/softirq.c:623)
>   run_ksoftirqd (kernel/softirq.c:1055)
>   smpboot_thread_fn (kernel/smpboot.c:160)
>   kthread (kernel/kthread.c:436)
>   ret_from_fork (arch/x86/kernel/process.c:164)
>   </TASK>
> 
> Reject the receive side on such instances instead of touching rstate.
> slhc_uncompress() falls through to its existing 'bad' label, which
> bumps sls_i_error and enters the toss state. slhc_remember() mirrors
> that with an explicit sls_i_error increment followed by slhc_toss();
> the sls_i_runt counter is not used here because a missing rstate is
> an internal configuration state, not a runt packet.
> 
> The transmit path is unaffected: the only in-tree caller that picks
> rslots from userspace (ppp_generic.c) still supplies tslots >= 1, and
> slip.c always calls slhc_init(16, 16), so comp->tstate remains valid
> and slhc_compress() continues to work.
> 
> Fixes: b5451d783ade ("slip: Move the SLIP drivers")

AI review points out that the cited commit moves code but doesn't
add this bug.

It seems to me that this bug has existed since the beginning of git
history. If so, the Fixes tag should be:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> v2:
> - slhc_remember(): use sls_i_error instead of sls_i_runt for the
>   missing-rstate case; it is a configuration error, not a runt packet
>   (Simon).
> - slhc_uncompress(): goto bad instead of returning 0, so the instance
>   also enters SLF_TOSS on the first rejected frame.

Otherwise this looks good to me:

Reviewed-by: Simon Horman <horms@kernel.org>


I do note that Sashiko flags some other problems in this code.
I do not think that needs to delay progress of this patch.
But you may wish to look into them as follow-up work.