[PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Weiming Shi]

sl_bump() reserves only 80 bytes of expansion headroom before calling
slhc_uncompress(), but the reconstructed IP + TCP header is up to
ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
both can legitimately reach 15, so the header can grow to 2*15*4 =
120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
a compressed frame of size buffsize - 80 therefore writes up to
33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
attacker-controlled content:

 BUG: KASAN: slab-out-of-bounds in slhc_uncompress
 Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
 Workqueue: events_unbound flush_to_ldisc
 Call Trace:
  __asan_memmove+0x3f/0x70
  slhc_uncompress (drivers/net/slip/slhc.c:614)
  slip_receive_buf (drivers/net/slip/slip.c:342)
  tty_ldisc_receive_buf
  flush_to_ldisc

Raise the reservation to match the real worst case. The ppp_generic
receive path already enforces skb_tailroom >= 124 and is unaffected.

Fixes: b5451d783ade ("slip: Move the SLIP drivers")
Reported-by: Simon Horman <horms@kernel.org>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/net/slip/slip.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 820e1a8fc9560..37af7cbe7f81d 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -333,9 +333,13 @@ static void sl_bump(struct slip *sl)
 				printk(KERN_WARNING "%s: compressed packet ignored\n", dev->name);
 				return;
 			}
-			/* make sure we've reserved enough space for uncompress
-			   to use */
-			if (count + 80 > sl->buffsize) {
+			/* slhc_uncompress() prepends up to
+			 * ip->ihl * 4 + thp->doff * 4 bytes of reconstructed
+			 * IPv4 + TCP header. IHL and doff are 4-bit fields
+			 * (max 15) counting 4-byte units, so the header is
+			 * at most 2 * 15 * 4 = 120 bytes.
+			 */
+			if (count + 2 * 15 * 4 > sl->buffsize) {
 				dev->stats.rx_over_errors++;
 				return;
 			}
-- 
2.43.0

Re: [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Simon Horman]

On Thu, Apr 16, 2026 at 05:34:00AM +0800, Weiming Shi wrote:
> sl_bump() reserves only 80 bytes of expansion headroom before calling
> slhc_uncompress(), but the reconstructed IP + TCP header is up to
> ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
> both can legitimately reach 15, so the header can grow to 2*15*4 =
> 120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
> a compressed frame of size buffsize - 80 therefore writes up to
> 33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
> attacker-controlled content:
> 
>  BUG: KASAN: slab-out-of-bounds in slhc_uncompress
>  Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
>  Workqueue: events_unbound flush_to_ldisc
>  Call Trace:
>   __asan_memmove+0x3f/0x70
>   slhc_uncompress (drivers/net/slip/slhc.c:614)
>   slip_receive_buf (drivers/net/slip/slip.c:342)
>   tty_ldisc_receive_buf
>   flush_to_ldisc
> 
> Raise the reservation to match the real worst case. The ppp_generic
> receive path already enforces skb_tailroom >= 124 and is unaffected.
> 
> Fixes: b5451d783ade ("slip: Move the SLIP drivers")
> Reported-by: Simon Horman <horms@kernel.org>

FTR, I was mainly passing on a review generated by Sashiko

> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

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

As usual I'll comment on the review of this patch by Sashiko.

TL;DR: I don't think it should block progress of this patch.

The review by Sashiko flags out of bounds errors. However,
these are addressed by one of your other patches:

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

As noted in my review of that patch, while it seems too late for these
patches, please consider bundling related patches in a patchset in future.

Re: [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Simon Horman]

On Thu, Apr 16, 2026 at 05:34:00AM +0800, Weiming Shi wrote:
> sl_bump() reserves only 80 bytes of expansion headroom before calling
> slhc_uncompress(), but the reconstructed IP + TCP header is up to
> ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
> both can legitimately reach 15, so the header can grow to 2*15*4 =
> 120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
> a compressed frame of size buffsize - 80 therefore writes up to
> 33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
> attacker-controlled content:
> 
>  BUG: KASAN: slab-out-of-bounds in slhc_uncompress
>  Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
>  Workqueue: events_unbound flush_to_ldisc
>  Call Trace:
>   __asan_memmove+0x3f/0x70
>   slhc_uncompress (drivers/net/slip/slhc.c:614)
>   slip_receive_buf (drivers/net/slip/slip.c:342)
>   tty_ldisc_receive_buf
>   flush_to_ldisc
> 
> Raise the reservation to match the real worst case. The ppp_generic
> receive path already enforces skb_tailroom >= 124 and is unaffected.
> 
> Fixes: b5451d783ade ("slip: Move the SLIP drivers")
> Reported-by: Simon Horman <horms@kernel.org>

FTR, I was mainly passing on information flagged by Sashiko.

> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

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

Let me summarise my understanding of Sashiko's review of this patch.

TL;DR: I don't think that review should block progress of this patch.

1. The issue wrt concurrent MTU changes appears to be a separate,
   pre-existing problem. Maybe you've looked into it already,
   if not, you may wish to.

2. The bounds checking problems are addressed by other patches in flight.

   - [PATCH net v2] slip: reject VJ receive packets on instances with no rstate array
     https://lore.kernel.org/netdev/20260415204130.258866-2-bestswngs@gmail.com/

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

   In future you might want to consider creating patch sets for related
   patches. But I think it's too late in the case of these patches.

...

Re: [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Simon Horman]

On Sun, Apr 19, 2026 at 03:27:26PM +0100, Simon Horman wrote:
> On Thu, Apr 16, 2026 at 05:34:00AM +0800, Weiming Shi wrote:
> > sl_bump() reserves only 80 bytes of expansion headroom before calling
> > slhc_uncompress(), but the reconstructed IP + TCP header is up to
> > ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
> > both can legitimately reach 15, so the header can grow to 2*15*4 =
> > 120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
> > a compressed frame of size buffsize - 80 therefore writes up to
> > 33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
> > attacker-controlled content:
> > 
> >  BUG: KASAN: slab-out-of-bounds in slhc_uncompress
> >  Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
> >  Workqueue: events_unbound flush_to_ldisc
> >  Call Trace:
> >   __asan_memmove+0x3f/0x70
> >   slhc_uncompress (drivers/net/slip/slhc.c:614)
> >   slip_receive_buf (drivers/net/slip/slip.c:342)
> >   tty_ldisc_receive_buf
> >   flush_to_ldisc
> > 
> > Raise the reservation to match the real worst case. The ppp_generic
> > receive path already enforces skb_tailroom >= 124 and is unaffected.
> > 
> > Fixes: b5451d783ade ("slip: Move the SLIP drivers")
> > Reported-by: Simon Horman <horms@kernel.org>
> 
> FTR, I was mainly passing on information flagged by Sashiko.
> 
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

I'm very sorry but the text below below was for a different,
albeit related, patch:

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

The corresponding text relating to this patch was posted as:

https://lore.kernel.org/netdev/20260419142710.GI280379@horms.kernel.org/

Sorry for the mix up!

> Let me summarise my understanding of Sashiko's review of this patch.
> 
> TL;DR: I don't think that review should block progress of this patch.
> 
> 1. The issue wrt concurrent MTU changes appears to be a separate,
>    pre-existing problem. Maybe you've looked into it already,
>    if not, you may wish to.
> 
> 2. The bounds checking problems are addressed by other patches in flight.
> 
>    - [PATCH net v2] slip: reject VJ receive packets on instances with no rstate array
>      https://lore.kernel.org/netdev/20260415204130.258866-2-bestswngs@gmail.com/
> 
>    - [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()
>      https://lore.kernel.org/netdev/20260415213359.335657-2-bestswngs@gmail.com/
> 
>    In future you might want to consider creating patch sets for related
>    patches. But I think it's too late in the case of these patches.
> 
> ...

Re: [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Simon Horman]

On Sun, Apr 19, 2026 at 03:27:10PM +0100, Simon Horman wrote:
> On Thu, Apr 16, 2026 at 05:34:00AM +0800, Weiming Shi wrote:
> > sl_bump() reserves only 80 bytes of expansion headroom before calling
> > slhc_uncompress(), but the reconstructed IP + TCP header is up to
> > ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
> > both can legitimately reach 15, so the header can grow to 2*15*4 =
> > 120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
> > a compressed frame of size buffsize - 80 therefore writes up to
> > 33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
> > attacker-controlled content:
> > 
> >  BUG: KASAN: slab-out-of-bounds in slhc_uncompress
> >  Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
> >  Workqueue: events_unbound flush_to_ldisc
> >  Call Trace:
> >   __asan_memmove+0x3f/0x70
> >   slhc_uncompress (drivers/net/slip/slhc.c:614)
> >   slip_receive_buf (drivers/net/slip/slip.c:342)
> >   tty_ldisc_receive_buf
> >   flush_to_ldisc
> > 
> > Raise the reservation to match the real worst case. The ppp_generic
> > receive path already enforces skb_tailroom >= 124 and is unaffected.
> > 
> > Fixes: b5451d783ade ("slip: Move the SLIP drivers")
> > Reported-by: Simon Horman <horms@kernel.org>
> 
> FTR, I was mainly passing on a review generated by Sashiko
> 
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> As usual I'll comment on the review of this patch by Sashiko.
> 
> TL;DR: I don't think it should block progress of this patch.
> 
> The review by Sashiko flags out of bounds errors. However,
> these are addressed by one of your other patches:
> 
> - [PATCH net] slip: bound decode() reads against the compressed packet length
>   https://lore.kernel.org/netdev/20260416100147.531855-5-bestswngs@gmail.com/
> 
> As noted in my review of that patch, while it seems too late for these
> patches, please consider bundling related patches in a patchset in future.

I'm very sorry but the text above results from me muddling
up my response to different slip patches.

I'll post the correct text for this patch elsewhere in this thread.

Re: [PATCH net] slip: fix slab-out-of-bounds write in slhc_uncompress()

[Simon Horman]

On Sun, Apr 19, 2026 at 03:32:33PM +0100, Simon Horman wrote:
> On Sun, Apr 19, 2026 at 03:27:26PM +0100, Simon Horman wrote:
> > On Thu, Apr 16, 2026 at 05:34:00AM +0800, Weiming Shi wrote:
> > > sl_bump() reserves only 80 bytes of expansion headroom before calling
> > > slhc_uncompress(), but the reconstructed IP + TCP header is up to
> > > ip->ihl*4 + thp->doff*4 bytes. IHL and TCP doff are 4-bit fields and
> > > both can legitimately reach 15, so the header can grow to 2*15*4 =
> > > 120 bytes. A VJ-uncompressed primer with ihl=15, doff=15 followed by
> > > a compressed frame of size buffsize - 80 therefore writes up to
> > > 33 bytes past the kmalloc(buffsize + 4) rbuff allocation, with
> > > attacker-controlled content:
> > > 
> > >  BUG: KASAN: slab-out-of-bounds in slhc_uncompress
> > >  Write of size 1069 at addr ffff88800ba93078 by task kworker/u8:1/32
> > >  Workqueue: events_unbound flush_to_ldisc
> > >  Call Trace:
> > >   __asan_memmove+0x3f/0x70
> > >   slhc_uncompress (drivers/net/slip/slhc.c:614)
> > >   slip_receive_buf (drivers/net/slip/slip.c:342)
> > >   tty_ldisc_receive_buf
> > >   flush_to_ldisc
> > > 
> > > Raise the reservation to match the real worst case. The ppp_generic
> > > receive path already enforces skb_tailroom >= 124 and is unaffected.
> > > 
> > > Fixes: b5451d783ade ("slip: Move the SLIP drivers")

AI review flags that this patch moved the code, rather than
adding the bug. It suggests the bug has been present since the
beginning of git history, so:

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

> > > Reported-by: Simon Horman <horms@kernel.org>
> > 
> > FTR, I was mainly passing on information flagged by Sashiko.
> > 
> > > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > 
> > Reviewed-by: Simon Horman <horms@kernel.org>
> 
> I'm very sorry but the text below below was for a different,
> albeit related, patch:
> 
> - [PATCH net] slip: bound decode() reads against the compressed packet length
>   https://lore.kernel.org/netdev/20260416100147.531855-5-bestswngs@gmail.com/
> 
> The corresponding text relating to this patch was posted as:
> 
> https://lore.kernel.org/netdev/20260419142710.GI280379@horms.kernel.org/
> 
> Sorry for the mix up!

Actually, that's not right either.
I will try one more time:

TL;DR: I don't think that review should block progress of this patch.

1. The issue wrt concurrent MTU changes appears to be a separate,
   pre-existing problem. Maybe you've looked into it already,
   if not, you may wish to.

2. The bounds checking problems are addressed by other patches in flight.

   - [PATCH net v2] slip: reject VJ receive packets on instances with no rstate array
     https://lore.kernel.org/netdev/20260415204130.258866-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/

   In future you might want to consider creating patch sets for related
   patches. But I think it's too late in the case of these patches.