Skip to content

Commit 4cefe32

Browse files
winmingregkh
authored andcommitted
slip: bound decode() reads against the compressed packet length
[ Upstream commit 4c1367a ] 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: 1da177e ("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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent de42f86 commit 4cefe32

1 file changed

Lines changed: 25 additions & 18 deletions

File tree

drivers/net/slip/slhc.c

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@
8080
#include <linux/unaligned.h>
8181

8282
static unsigned char *encode(unsigned char *cp, unsigned short n);
83-
static long decode(unsigned char **cpp);
83+
static long decode(unsigned char **cpp, const unsigned char *end);
8484
static unsigned char * put16(unsigned char *cp, unsigned short x);
85-
static unsigned short pull16(unsigned char **cpp);
85+
static long pull16(unsigned char **cpp, const unsigned char *end);
8686

8787
/* Allocate compression data structure
8888
* slots must be in range 0 to 255 (zero meaning no compression)
@@ -190,30 +190,34 @@ encode(unsigned char *cp, unsigned short n)
190190
return cp;
191191
}
192192

193-
/* Pull a 16-bit integer in host order from buffer in network byte order */
194-
static unsigned short
195-
pull16(unsigned char **cpp)
193+
/* Pull a 16-bit integer in host order from buffer in network byte order.
194+
* Returns -1 if the buffer is exhausted, otherwise the 16-bit value.
195+
*/
196+
static long
197+
pull16(unsigned char **cpp, const unsigned char *end)
196198
{
197-
short rval;
199+
long rval;
198200

201+
if (*cpp + 2 > end)
202+
return -1;
199203
rval = *(*cpp)++;
200204
rval <<= 8;
201205
rval |= *(*cpp)++;
202206
return rval;
203207
}
204208

205-
/* Decode a number */
209+
/* Decode a number. Returns -1 if the buffer is exhausted. */
206210
static long
207-
decode(unsigned char **cpp)
211+
decode(unsigned char **cpp, const unsigned char *end)
208212
{
209213
int x;
210214

215+
if (*cpp >= end)
216+
return -1;
211217
x = *(*cpp)++;
212-
if(x == 0){
213-
return pull16(cpp) & 0xffff; /* pull16 returns -1 on error */
214-
} else {
215-
return x & 0xff; /* -1 if PULLCHAR returned error */
216-
}
218+
if (x == 0)
219+
return pull16(cpp, end);
220+
return x & 0xff;
217221
}
218222

219223
/*
@@ -499,6 +503,7 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
499503
struct cstate *cs;
500504
int len, hdrlen;
501505
unsigned char *cp = icp;
506+
const unsigned char *end = icp + isize;
502507

503508
/* We've got a compressed packet; read the change byte */
504509
comp->sls_i_compressed++;
@@ -536,6 +541,8 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
536541
thp = &cs->cs_tcp;
537542
ip = &cs->cs_ip;
538543

544+
if (cp + 2 > end)
545+
goto bad;
539546
thp->check = *(__sum16 *)cp;
540547
cp += 2;
541548

@@ -566,34 +573,34 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
566573
default:
567574
if(changes & NEW_U){
568575
thp->urg = 1;
569-
if((x = decode(&cp)) == -1) {
576+
if((x = decode(&cp, end)) == -1) {
570577
goto bad;
571578
}
572579
thp->urg_ptr = htons(x);
573580
} else
574581
thp->urg = 0;
575582
if(changes & NEW_W){
576-
if((x = decode(&cp)) == -1) {
583+
if((x = decode(&cp, end)) == -1) {
577584
goto bad;
578585
}
579586
thp->window = htons( ntohs(thp->window) + x);
580587
}
581588
if(changes & NEW_A){
582-
if((x = decode(&cp)) == -1) {
589+
if((x = decode(&cp, end)) == -1) {
583590
goto bad;
584591
}
585592
thp->ack_seq = htonl( ntohl(thp->ack_seq) + x);
586593
}
587594
if(changes & NEW_S){
588-
if((x = decode(&cp)) == -1) {
595+
if((x = decode(&cp, end)) == -1) {
589596
goto bad;
590597
}
591598
thp->seq = htonl( ntohl(thp->seq) + x);
592599
}
593600
break;
594601
}
595602
if(changes & NEW_I){
596-
if((x = decode(&cp)) == -1) {
603+
if((x = decode(&cp, end)) == -1) {
597604
goto bad;
598605
}
599606
ip->id = htons (ntohs (ip->id) + x);

0 commit comments

Comments
 (0)