From patchwork Thu Aug 24 21:38:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 805679 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="WAIG+CHZ"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3xdd7j1Rs9z9sRV for ; Fri, 25 Aug 2017 07:42:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753593AbdHXVml (ORCPT ); Thu, 24 Aug 2017 17:42:41 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36754 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753142AbdHXVmk (ORCPT ); Thu, 24 Aug 2017 17:42:40 -0400 Received: by mail-pg0-f66.google.com with SMTP id 83so996752pgb.3; Thu, 24 Aug 2017 14:42:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=ocX8QPzxM33mUCEoHgjmnorxMZp6ncAmqWan3aqOxpQ=; b=WAIG+CHZo6SyaEvaH65OQt4NQZ0v5kjbpqdDHOcdcOgflbh+ur2PMMo07P0SxDcFDQ sYnPlD8gPIy2TurJErGwtJrEZL/L0o0KirrnSJemJWQJIX0YEEAv227FimkTW9xANcAN mqi7qJVweJaVBM7jLDhUIEGygH+1FhP3nTsd+it8pdFNgT3veuWz67BL6l2KWL2ZSokP TSUsPbBhZBI1midWmGMhm8rDdsw7gsOJLXrYV3p6T7XhkKWMxd/o7LQAoAlm2a1Y7j9i yukvyveRH75EZZUzFEJfmTVhmcWD2TG6dHrDC0wwd4QusycVCDJs91aKZ+NV9om2t0ix DiDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=ocX8QPzxM33mUCEoHgjmnorxMZp6ncAmqWan3aqOxpQ=; b=NnTHl8Wf6AMvr2hams6zHyxEb6eQLxXlJ19uUXo+d6wGgqJm6ejZURufxVEiZIF7ib 6IXfJO2djRdIKt+bIUHYHPPmHfAp5paA4JaXCYzl1qIfLrQX78hjHPWt20W2Re+ckblB ywJ1PNcus83/AJd51Qu0BQqnIsp2Qx4gU3tnmeMfMgjK+JmhyEJ4YnJMrtoZ+o4S9suA Rh6T73PPu2YyddrJPVM0sa5ysPmngjJ6/u50g8RvkF0EJl878eq9LnSQxBzB7NrvcTYk /0NqoVsu5iXFawDgy80OciDkHt9lEuTN2s3G+CExmDbErMpJGgk4UE5GzhkISgPvYqL/ 88yw== X-Gm-Message-State: AHYfb5gINsBxAZEIbLUftu9suN3ZhaPYmc7sJbWUmxLrfORhJehzuWPv FHJgwHdf6/cH5gRSAC0= X-Received: by 10.99.3.212 with SMTP id 203mr7314021pgd.409.1503610959138; Thu, 24 Aug 2017 14:42:39 -0700 (PDT) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.66.174.81]) by smtp.gmail.com with ESMTPSA id u186sm8855853pfu.82.2017.08.24.14.42.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 24 Aug 2017 14:42:38 -0700 (PDT) From: Eric Biggers To: netdev@vger.kernel.org Cc: "David S . Miller" , linux-kernel@vger.kernel.org, Eric Biggers , Dmitry Vyukov , Tom Herbert Subject: [PATCH] strparser: initialize all callbacks Date: Thu, 24 Aug 2017 14:38:51 -0700 Message-Id: <20170824213851.57601-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.14.1.342.g6490525c54-goog Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Biggers commit bbb03029a899 ("strparser: Generalize strparser") added more function pointers to 'struct strp_callbacks'; however, kcm_attach() was not updated to initialize them. This could cause the ->lock() and/or ->unlock() function pointers to be set to garbage values, causing a crash in strp_work(). Fix the bug by moving the callback structs into static memory, so unspecified members are zeroed. Also constify them while we're at it. This bug was found by syzkaller, which encountered the following splat: IP: 0x55 PGD 3b1ca067 P4D 3b1ca067 PUD 3b12f067 PMD 0 Oops: 0010 [#1] SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) Modules linked in: CPU: 2 PID: 1194 Comm: kworker/u8:1 Not tainted 4.13.0-rc4-next-20170811 #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Workqueue: kstrp strp_work task: ffff88006bb0e480 task.stack: ffff88006bb10000 RIP: 0010:0x55 RSP: 0018:ffff88006bb17540 EFLAGS: 00010246 RAX: dffffc0000000000 RBX: ffff88006ce4bd60 RCX: 0000000000000000 RDX: 1ffff1000d9c97bd RSI: 0000000000000000 RDI: ffff88006ce4bc48 RBP: ffff88006bb17558 R08: ffffffff81467ab2 R09: 0000000000000000 R10: ffff88006bb17438 R11: ffff88006bb17940 R12: ffff88006ce4bc48 R13: ffff88003c683018 R14: ffff88006bb17980 R15: ffff88003c683000 FS: 0000000000000000(0000) GS:ffff88006de00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000055 CR3: 000000003c145000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: process_one_work+0xbf3/0x1bc0 kernel/workqueue.c:2098 worker_thread+0x223/0x1860 kernel/workqueue.c:2233 kthread+0x35e/0x430 kernel/kthread.c:231 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431 Code: Bad RIP value. RIP: 0x55 RSP: ffff88006bb17540 CR2: 0000000000000055 ---[ end trace f0e4920047069cee ]--- Here is a C reproducer (requires CONFIG_BPF_SYSCALL=y and CONFIG_AF_KCM=y): #include #include #include #include #include #include #include #include static const struct bpf_insn bpf_insns[3] = { { .code = 0xb7 }, /* BPF_MOV64_IMM(0, 0) */ { .code = 0x95 }, /* BPF_EXIT_INSN() */ }; static const union bpf_attr bpf_attr = { .prog_type = 1, .insn_cnt = 2, .insns = (uintptr_t)&bpf_insns, .license = (uintptr_t)"", }; int main(void) { int bpf_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &bpf_attr, sizeof(bpf_attr)); int inet_fd = socket(AF_INET, SOCK_STREAM, 0); int kcm_fd = socket(AF_KCM, SOCK_DGRAM, 0); ioctl(kcm_fd, SIOCKCMATTACH, &(struct kcm_attach) { .fd = inet_fd, .bpf_fd = bpf_fd }); } Fixes: bbb03029a899 ("strparser: Generalize strparser") Cc: Dmitry Vyukov Cc: Tom Herbert Signed-off-by: Eric Biggers --- Documentation/networking/strparser.txt | 2 +- include/net/strparser.h | 2 +- kernel/bpf/sockmap.c | 10 +++++----- net/kcm/kcmsock.c | 11 +++++------ net/strparser/strparser.c | 2 +- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Documentation/networking/strparser.txt b/Documentation/networking/strparser.txt index fe01302471ae..13081b3decef 100644 --- a/Documentation/networking/strparser.txt +++ b/Documentation/networking/strparser.txt @@ -35,7 +35,7 @@ Functions ========= strp_init(struct strparser *strp, struct sock *sk, - struct strp_callbacks *cb) + const struct strp_callbacks *cb) Called to initialize a stream parser. strp is a struct of type strparser that is allocated by the upper layer. sk is the TCP diff --git a/include/net/strparser.h b/include/net/strparser.h index 4fe966a0ad92..7dc131d62ad5 100644 --- a/include/net/strparser.h +++ b/include/net/strparser.h @@ -138,7 +138,7 @@ void strp_done(struct strparser *strp); void strp_stop(struct strparser *strp); void strp_check_rcv(struct strparser *strp); int strp_init(struct strparser *strp, struct sock *sk, - struct strp_callbacks *cb); + const struct strp_callbacks *cb); void strp_data_ready(struct strparser *strp); int strp_process(struct strparser *strp, struct sk_buff *orig_skb, unsigned int orig_offset, size_t orig_len, diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 78b2bb9370ac..617c239590c2 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -368,12 +368,12 @@ static int smap_read_sock_done(struct strparser *strp, int err) static int smap_init_sock(struct smap_psock *psock, struct sock *sk) { - struct strp_callbacks cb; + static const struct strp_callbacks cb = { + .rcv_msg = smap_read_sock_strparser, + .parse_msg = smap_parse_func_strparser, + .read_sock_done = smap_read_sock_done, + }; - memset(&cb, 0, sizeof(cb)); - cb.rcv_msg = smap_read_sock_strparser; - cb.parse_msg = smap_parse_func_strparser; - cb.read_sock_done = smap_read_sock_done; return strp_init(&psock->strp, sk, &cb); } diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index 88ce73288247..48e993b2dbcf 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -1376,7 +1376,11 @@ static int kcm_attach(struct socket *sock, struct socket *csock, struct kcm_psock *psock = NULL, *tpsock; struct list_head *head; int index = 0; - struct strp_callbacks cb; + static const struct strp_callbacks cb = { + .rcv_msg = kcm_rcv_strparser, + .parse_msg = kcm_parse_func_strparser, + .read_sock_done = kcm_read_sock_done, + }; int err; csk = csock->sk; @@ -1391,11 +1395,6 @@ static int kcm_attach(struct socket *sock, struct socket *csock, psock->sk = csk; psock->bpf_prog = prog; - cb.rcv_msg = kcm_rcv_strparser; - cb.abort_parser = NULL; - cb.parse_msg = kcm_parse_func_strparser; - cb.read_sock_done = kcm_read_sock_done; - err = strp_init(&psock->strp, csk, &cb); if (err) { kmem_cache_free(kcm_psockp, psock); diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c index 434aa6637a52..d4ea46a5f233 100644 --- a/net/strparser/strparser.c +++ b/net/strparser/strparser.c @@ -472,7 +472,7 @@ static void strp_sock_unlock(struct strparser *strp) } int strp_init(struct strparser *strp, struct sock *sk, - struct strp_callbacks *cb) + const struct strp_callbacks *cb) { if (!cb || !cb->rcv_msg || !cb->parse_msg)