From patchwork Mon Mar 6 18:46:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Potapenko X-Patchwork-Id: 735893 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vcTKp1nzlz9sN5 for ; Tue, 7 Mar 2017 05:47:10 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="BusfGG/e"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932206AbdCFSqa (ORCPT ); Mon, 6 Mar 2017 13:46:30 -0500 Received: from mail-wm0-f51.google.com ([74.125.82.51]:38439 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754115AbdCFSqW (ORCPT ); Mon, 6 Mar 2017 13:46:22 -0500 Received: by mail-wm0-f51.google.com with SMTP id t193so72546626wmt.1 for ; Mon, 06 Mar 2017 10:46:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=G1pdBfR4WVpM7JlPm12RRZZ332GtQ0w2VJwdC2oXCDI=; b=BusfGG/eM5Pq6GCP1f3rNa/pbM+S00uc/89kndzzRdhC5pZtqzmoZbaZLON/Zhjifk uuw71NiLgh/fQU0RxrLUImnqtlvFgp5e/9aYCmn4m//UOm5OiWJ+lOEHFwOrFoE/zW8c MFfAkfbx0wrSBV/DA5K46bsDlmQGvX4dcS+wYurQK6Vw+723olUAmTAEcYnZeFKPv2aX 0ch8hdxAZFjGQtzL4pjpZ7HWPcgz+aQyAzVn2oFlu/MLIXvtxEXxyg/ZRAbXyX8cSPIw h6bzpmfAZIMhpCSeO8h9J447vFe0UnLUI0XbgdJVBNPiIuaHr3YkZZ80Z3evkp2dDqVx K8fg== 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=G1pdBfR4WVpM7JlPm12RRZZ332GtQ0w2VJwdC2oXCDI=; b=UskBcPexRAtI+W+GTpTSNC79zq9/yz/YeeXNtxCHd+zYTEwmzupTzsF7oMq0JV6FnP tVZu+c4C/Jr8aruJEyfB7cr4ui8TbI10wUYATw3tlp7KCQhdf1wjrYm5soKpH/AEQYhZ mKi+ka7PuglQKCJ12ty3GHUMrW1V9QxnF7V054CTuOlTVXiB0UnbGYHxZebI/kMXADwp 6OmKZprtmLVstbN6PR6HiGGid4dBtHs7gfWfIZ/8XssV902+MqfziRf2b2ARQeT4o5b8 N2ln/JunMC5dgVoVudt1Oa0fzh8LtrSj9vmFctYWPueqRIzCQUbQTJE2Tgrld1CQV5Ta 6aJA== X-Gm-Message-State: AMke39mrLwrZB5T+wmBShINOX9VXBbNzTxvXWcZsHnUXGNmPcUh3JREoDleWWCn0aNcEecWR X-Received: by 10.28.16.70 with SMTP id 67mr15236119wmq.142.1488825978539; Mon, 06 Mar 2017 10:46:18 -0800 (PST) Received: from glider0.muc.corp.google.com ([100.105.28.21]) by smtp.gmail.com with ESMTPSA id c133sm15830746wmd.13.2017.03.06.10.46.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 06 Mar 2017 10:46:17 -0800 (PST) From: Alexander Potapenko To: dvyukov@google.com, kcc@google.com, keescook@chromium.org, edumazet@google.com, paul@paul-moore.com, sds@tycho.nsa.gov Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, selinux@tycho.nsa.gov Subject: [PATCH v2] selinux: check for address length in selinux_socket_bind() Date: Mon, 6 Mar 2017 19:46:14 +0100 Message-Id: <20170306184614.20056-1-glider@google.com> X-Mailer: git-send-email 2.12.0.rc1.440.g5b76565f74-goog Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of uninitialized memory in selinux_socket_bind(): Acked-by: Eric Dumazet ================================================================== BUG: KMSAN: use of unitialized memory inter: 0 CPU: 3 PID: 1074 Comm: packet2 Tainted: G B 4.8.0-rc6+ #1916 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 0000000000000000 ffff8800882ffb08 ffffffff825759c8 ffff8800882ffa48 ffffffff818bf551 ffffffff85bab870 0000000000000092 ffffffff85bab550 0000000000000000 0000000000000092 00000000bb0009bb 0000000000000002 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x238/0x290 lib/dump_stack.c:51 [] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1008 [] __msan_warning+0x5b/0xb0 mm/kmsan/kmsan_instr.c:424 [] selinux_socket_bind+0xf41/0x1080 security/selinux/hooks.c:4288 [] security_socket_bind+0x1ec/0x240 security/security.c:1240 [] SYSC_bind+0x358/0x5f0 net/socket.c:1366 [] SyS_bind+0x82/0xa0 net/socket.c:1356 [] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [] entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.o:? chained origin: 00000000ba6009bb [] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67 [< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322 [< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337 [] kmsan_internal_chain_origin+0x118/0x1e0 mm/kmsan/kmsan.c:530 [] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380 [] SYSC_bind+0x129/0x5f0 net/socket.c:1356 [] SyS_bind+0x82/0xa0 net/socket.c:1356 [] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [] return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.o:? origin description: ----address@SYSC_bind (origin=00000000b8c00900) ================================================================== (the line numbers are relative to 4.8-rc6, but the bug persists upstream) , when I run the following program as root: ======================================================= #include #include #include int main(int argc, char *argv[]) { struct sockaddr addr; int size = 0; if (argc > 1) { size = atoi(argv[1]); } memset(&addr, 0, sizeof(addr)); int fd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); bind(fd, &addr, size); return 0; } ======================================================= (for different values of |size| other error reports are printed). This happens because bind() unconditionally copies |size| bytes of |addr| to the kernel, leaving the rest uninitialized. Then security_socket_bind() reads the IP address bytes, including the uninitialized ones, to determine the port, or e.g. pass them further to sel_netnode_find(), which uses them to calculate a hash. Signed-off-by: Alexander Potapenko --- Changes since v1: - fixed patch description - fixed addrlen tests to match those in inet_bind() and inet6_bind() (per comment from Eric Dumazet) --- security/selinux/hooks.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0a4b4b040e0a..ddc4aca6c840 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4351,10 +4351,19 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in u32 sid, node_perm; if (family == PF_INET) { + if (addrlen < sizeof(struct sockaddr_in)) { + err = -EINVAL; + goto out; + } addr4 = (struct sockaddr_in *)address; snum = ntohs(addr4->sin_port); addrp = (char *)&addr4->sin_addr.s_addr; + } else { + if (addrlen < SIN6_LEN_RFC2133) { + err = -EINVAL; + goto out; + } addr6 = (struct sockaddr_in6 *)address; snum = ntohs(addr6->sin6_port); addrp = (char *)&addr6->sin6_addr.s6_addr;