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

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

[Weiming Shi]

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")
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.

 drivers/net/slip/slhc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/slip/slhc.c b/drivers/net/slip/slhc.c
index e3c785da3eef3..e18a4213d10ce 100644
--- a/drivers/net/slip/slhc.c
+++ b/drivers/net/slip/slhc.c
@@ -506,6 +506,8 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
 		comp->sls_i_error++;
 		return 0;
 	}
+	if (!comp->rstate)
+		goto bad;
 	changes = *cp++;
 	if(changes & NEW_C){
 		/* Make sure the state index is in range, then grab the state.
@@ -649,6 +651,10 @@ slhc_remember(struct slcompress *comp, unsigned char *icp, int isize)
 	struct cstate *cs;
 	unsigned int ihl;
 
+	if (!comp->rstate) {
+		comp->sls_i_error++;
+		return slhc_toss(comp);
+	}
 	/* The packet is shorter than a legal IP header.
 	 * Also make sure isize is positive.
 	 */
-- 
2.43.0

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

[Simon Horman]

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.

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

[Weiming Shi]

On 26-04-18 13:39, Simon Horman wrote:
> 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.

Thanks for your review. 

I've already sent two follow-up patches for the decode()/pull16() 
bounds-checking issues:

    [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()
    https://lore.kernel.org/netdev/20260415213359.335657-2-bestswngs@gmail.com/

    [PATCH net] slip: bound decode() reads against the compressed packet length
    https://lore.kernel.org/netdev/20260416100147.531855-5-bestswngs@gmail.com/

Best regards,
Weiming Shi

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

[Simon Horman]

On Sat, Apr 18, 2026 at 10:37:26PM +0800, Weiming Shi wrote:
> On 26-04-18 13:39, Simon Horman wrote:
> > On Thu, Apr 16, 2026 at 04:41:31AM +0800, Weiming Shi wrote:

...

> > 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.
> 
> Thanks for your review. 
> 
> I've already sent two follow-up patches for the decode()/pull16() 
> bounds-checking issues:
> 
>     [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()
>     https://lore.kernel.org/netdev/20260415213359.335657-2-bestswngs@gmail.com/
> 
>     [PATCH net] slip: bound decode() reads against the compressed packet length
>     https://lore.kernel.org/netdev/20260416100147.531855-5-bestswngs@gmail.com/

Great, thanks!

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 16 Apr 2026 04:41:31 +0800 you 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.
> 
> [...]

Here is the summary with links:
  - [net,v2] slip: reject VJ receive packets on instances with no rstate array
    https://git.kernel.org/netdev/net/c/e76607442d5b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html