summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWeiming Shi <bestswngs@gmail.com>2026-04-16 18:01:51 +0800
committerPaolo Abeni <pabeni@redhat.com>2026-04-21 10:18:18 +0200
commit4c1367a2d7aad643a6f87c6931b13cc1a25e8ca7 (patch)
tree921b34ef7ffece807645d3e0de5967bf9f6ab5ed
parente76607442d5b73e1ba6768f501ef815bb58c2c0e (diff)
slip: bound decode() reads against the compressed packet length
slhc_uncompress() parses a VJ-compressed TCP header by advancing a pointer through the packet via decode() and pull16(). Neither helper bounds-checks against isize, and decode() masks its return with & 0xffff so it can never return the -1 that callers test for -- those error paths are dead code. A short compressed frame whose change byte requests optional fields lets decode() read past the end of the packet. The over-read bytes are folded into the cached cstate and reflected into subsequent reconstructed packets. Make decode() and pull16() take the packet end pointer and return -1 when exhausted. Add a bounds check before the TCP-checksum read. The existing == -1 tests now do what they were always meant to. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Simon Horman <horms@kernel.org> Closes: https://lore.kernel.org/netdev/20260414134126.758795-2-horms@kernel.org/ Signed-off-by: Weiming Shi <bestswngs@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20260416100147.531855-5-bestswngs@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
-rw-r--r--drivers/net/slip/slhc.c43
1 files changed, 25 insertions, 18 deletions
diff --git a/drivers/net/slip/slhc.c b/drivers/net/slip/slhc.c
index e18a4213d10c..1a9b27d5e256 100644
--- a/drivers/net/slip/slhc.c
+++ b/drivers/net/slip/slhc.c
@@ -80,9 +80,9 @@
#include <linux/unaligned.h>
static unsigned char *encode(unsigned char *cp, unsigned short n);
-static long decode(unsigned char **cpp);
+static long decode(unsigned char **cpp, const unsigned char *end);
static unsigned char * put16(unsigned char *cp, unsigned short x);
-static unsigned short pull16(unsigned char **cpp);
+static long pull16(unsigned char **cpp, const unsigned char *end);
/* Allocate compression data structure
* slots must be in range 0 to 255 (zero meaning no compression)
@@ -190,30 +190,34 @@ encode(unsigned char *cp, unsigned short n)
return cp;
}
-/* Pull a 16-bit integer in host order from buffer in network byte order */
-static unsigned short
-pull16(unsigned char **cpp)
+/* Pull a 16-bit integer in host order from buffer in network byte order.
+ * Returns -1 if the buffer is exhausted, otherwise the 16-bit value.
+ */
+static long
+pull16(unsigned char **cpp, const unsigned char *end)
{
- short rval;
+ long rval;
+ if (*cpp + 2 > end)
+ return -1;
rval = *(*cpp)++;
rval <<= 8;
rval |= *(*cpp)++;
return rval;
}
-/* Decode a number */
+/* Decode a number. Returns -1 if the buffer is exhausted. */
static long
-decode(unsigned char **cpp)
+decode(unsigned char **cpp, const unsigned char *end)
{
int x;
+ if (*cpp >= end)
+ return -1;
x = *(*cpp)++;
- if(x == 0){
- return pull16(cpp) & 0xffff; /* pull16 returns -1 on error */
- } else {
- return x & 0xff; /* -1 if PULLCHAR returned error */
- }
+ if (x == 0)
+ return pull16(cpp, end);
+ return x & 0xff;
}
/*
@@ -499,6 +503,7 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
struct cstate *cs;
int len, hdrlen;
unsigned char *cp = icp;
+ const unsigned char *end = icp + isize;
/* We've got a compressed packet; read the change byte */
comp->sls_i_compressed++;
@@ -536,6 +541,8 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
thp = &cs->cs_tcp;
ip = &cs->cs_ip;
+ if (cp + 2 > end)
+ goto bad;
thp->check = *(__sum16 *)cp;
cp += 2;
@@ -566,26 +573,26 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
default:
if(changes & NEW_U){
thp->urg = 1;
- if((x = decode(&cp)) == -1) {
+ if((x = decode(&cp, end)) == -1) {
goto bad;
}
thp->urg_ptr = htons(x);
} else
thp->urg = 0;
if(changes & NEW_W){
- if((x = decode(&cp)) == -1) {
+ if((x = decode(&cp, end)) == -1) {
goto bad;
}
thp->window = htons( ntohs(thp->window) + x);
}
if(changes & NEW_A){
- if((x = decode(&cp)) == -1) {
+ if((x = decode(&cp, end)) == -1) {
goto bad;
}
thp->ack_seq = htonl( ntohl(thp->ack_seq) + x);
}
if(changes & NEW_S){
- if((x = decode(&cp)) == -1) {
+ if((x = decode(&cp, end)) == -1) {
goto bad;
}
thp->seq = htonl( ntohl(thp->seq) + x);
@@ -593,7 +600,7 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
break;
}
if(changes & NEW_I){
- if((x = decode(&cp)) == -1) {
+ if((x = decode(&cp, end)) == -1) {
goto bad;
}
ip->id = htons (ntohs (ip->id) + x);