Skip to content

Commit 8bb79eb

Browse files
edumazetgregkh
authored andcommitted
slip: make slhc_remember() more robust against malicious packets
[ Upstream commit 7d3fce8 ] syzbot found that slhc_remember() was missing checks against malicious packets [1]. slhc_remember() only checked the size of the packet was at least 20, which is not good enough. We need to make sure the packet includes the IPv4 and TCP header that are supposed to be carried. Add iph and th pointers to make the code more readable. [1] BUG: KMSAN: uninit-value in slhc_remember+0x2e8/0x7b0 drivers/net/slip/slhc.c:666 slhc_remember+0x2e8/0x7b0 drivers/net/slip/slhc.c:666 ppp_receive_nonmp_frame+0xe45/0x35e0 drivers/net/ppp/ppp_generic.c:2455 ppp_receive_frame drivers/net/ppp/ppp_generic.c:2372 [inline] ppp_do_recv+0x65f/0x40d0 drivers/net/ppp/ppp_generic.c:2212 ppp_input+0x7dc/0xe60 drivers/net/ppp/ppp_generic.c:2327 pppoe_rcv_core+0x1d3/0x720 drivers/net/ppp/pppoe.c:379 sk_backlog_rcv+0x13b/0x420 include/net/sock.h:1113 __release_sock+0x1da/0x330 net/core/sock.c:3072 release_sock+0x6b/0x250 net/core/sock.c:3626 pppoe_sendmsg+0x2b8/0xb90 drivers/net/ppp/pppoe.c:903 sock_sendmsg_nosec net/socket.c:729 [inline] __sock_sendmsg+0x30f/0x380 net/socket.c:744 ____sys_sendmsg+0x903/0xb60 net/socket.c:2602 ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2656 __sys_sendmmsg+0x3c1/0x960 net/socket.c:2742 __do_sys_sendmmsg net/socket.c:2771 [inline] __se_sys_sendmmsg net/socket.c:2768 [inline] __x64_sys_sendmmsg+0xbc/0x120 net/socket.c:2768 x64_sys_call+0xb6e/0x3ba0 arch/x86/include/generated/asm/syscalls_64.h:308 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f Uninit was created at: slab_post_alloc_hook mm/slub.c:4091 [inline] slab_alloc_node mm/slub.c:4134 [inline] kmem_cache_alloc_node_noprof+0x6bf/0xb80 mm/slub.c:4186 kmalloc_reserve+0x13d/0x4a0 net/core/skbuff.c:587 __alloc_skb+0x363/0x7b0 net/core/skbuff.c:678 alloc_skb include/linux/skbuff.h:1322 [inline] sock_wmalloc+0xfe/0x1a0 net/core/sock.c:2732 pppoe_sendmsg+0x3a7/0xb90 drivers/net/ppp/pppoe.c:867 sock_sendmsg_nosec net/socket.c:729 [inline] __sock_sendmsg+0x30f/0x380 net/socket.c:744 ____sys_sendmsg+0x903/0xb60 net/socket.c:2602 ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2656 __sys_sendmmsg+0x3c1/0x960 net/socket.c:2742 __do_sys_sendmmsg net/socket.c:2771 [inline] __se_sys_sendmmsg net/socket.c:2768 [inline] __x64_sys_sendmmsg+0xbc/0x120 net/socket.c:2768 x64_sys_call+0xb6e/0x3ba0 arch/x86/include/generated/asm/syscalls_64.h:308 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f CPU: 0 UID: 0 PID: 5460 Comm: syz.2.33 Not tainted 6.12.0-rc2-syzkaller-00006-g87d6aab2389e #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024 Fixes: b5451d7 ("slip: Move the SLIP drivers") Reported-by: [email protected] Closes: https://lore.kernel.org/netdev/[email protected]/T/#u Signed-off-by: Eric Dumazet <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 8fe992f commit 8bb79eb

File tree

1 file changed

+34
-23
lines changed

1 file changed

+34
-23
lines changed

drivers/net/slip/slhc.c

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -643,46 +643,57 @@ slhc_uncompress(struct slcompress *comp, unsigned char *icp, int isize)
643643
int
644644
slhc_remember(struct slcompress *comp, unsigned char *icp, int isize)
645645
{
646-
struct cstate *cs;
647-
unsigned ihl;
648-
646+
const struct tcphdr *th;
649647
unsigned char index;
648+
struct iphdr *iph;
649+
struct cstate *cs;
650+
unsigned int ihl;
650651

651-
if(isize < 20) {
652-
/* The packet is shorter than a legal IP header */
652+
/* The packet is shorter than a legal IP header.
653+
* Also make sure isize is positive.
654+
*/
655+
if (isize < (int)sizeof(struct iphdr)) {
656+
runt:
653657
comp->sls_i_runt++;
654-
return slhc_toss( comp );
658+
return slhc_toss(comp);
655659
}
660+
iph = (struct iphdr *)icp;
656661
/* Peek at the IP header's IHL field to find its length */
657-
ihl = icp[0] & 0xf;
658-
if(ihl < 20 / 4){
659-
/* The IP header length field is too small */
660-
comp->sls_i_runt++;
661-
return slhc_toss( comp );
662-
}
663-
index = icp[9];
664-
icp[9] = IPPROTO_TCP;
662+
ihl = iph->ihl;
663+
/* The IP header length field is too small,
664+
* or packet is shorter than the IP header followed
665+
* by minimal tcp header.
666+
*/
667+
if (ihl < 5 || isize < ihl * 4 + sizeof(struct tcphdr))
668+
goto runt;
669+
670+
index = iph->protocol;
671+
iph->protocol = IPPROTO_TCP;
665672

666673
if (ip_fast_csum(icp, ihl)) {
667674
/* Bad IP header checksum; discard */
668675
comp->sls_i_badcheck++;
669-
return slhc_toss( comp );
676+
return slhc_toss(comp);
670677
}
671-
if(index > comp->rslot_limit) {
678+
if (index > comp->rslot_limit) {
672679
comp->sls_i_error++;
673680
return slhc_toss(comp);
674681
}
675-
682+
th = (struct tcphdr *)(icp + ihl * 4);
683+
if (th->doff < sizeof(struct tcphdr) / 4)
684+
goto runt;
685+
if (isize < ihl * 4 + th->doff * 4)
686+
goto runt;
676687
/* Update local state */
677688
cs = &comp->rstate[comp->recv_current = index];
678689
comp->flags &=~ SLF_TOSS;
679-
memcpy(&cs->cs_ip,icp,20);
680-
memcpy(&cs->cs_tcp,icp + ihl*4,20);
690+
memcpy(&cs->cs_ip, iph, sizeof(*iph));
691+
memcpy(&cs->cs_tcp, th, sizeof(*th));
681692
if (ihl > 5)
682-
memcpy(cs->cs_ipopt, icp + sizeof(struct iphdr), (ihl - 5) * 4);
683-
if (cs->cs_tcp.doff > 5)
684-
memcpy(cs->cs_tcpopt, icp + ihl*4 + sizeof(struct tcphdr), (cs->cs_tcp.doff - 5) * 4);
685-
cs->cs_hsize = ihl*2 + cs->cs_tcp.doff*2;
693+
memcpy(cs->cs_ipopt, &iph[1], (ihl - 5) * 4);
694+
if (th->doff > 5)
695+
memcpy(cs->cs_tcpopt, &th[1], (th->doff - 5) * 4);
696+
cs->cs_hsize = ihl*2 + th->doff*2;
686697
cs->initialized = true;
687698
/* Put headers back on packet
688699
* Neither header checksum is recalculated

0 commit comments

Comments
 (0)