From patchwork Wed Mar 1 11:57:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Potapenko X-Patchwork-Id: 734223 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 3vYFn86dslz9s7j for ; Wed, 1 Mar 2017 23:56:12 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="vLlNeclV"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751422AbdCAM4L (ORCPT ); Wed, 1 Mar 2017 07:56:11 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:38698 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751135AbdCAM4G (ORCPT ); Wed, 1 Mar 2017 07:56:06 -0500 Received: by mail-wm0-f44.google.com with SMTP id u199so35584086wmd.1 for ; Wed, 01 Mar 2017 04:54:34 -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=ONehkVDhEBGP6xprW7TxLIU9bYv8NrlnrjT4SusYHJ8=; b=vLlNeclVcmT5wcZzrWYeiES/0eJCJ1bo5cQ00rM/2XHnxC/AEJeBi2DpW4VGJys2zb PKETaAwvUwp+89kRvZgZ8HGb5t4jh4K3+4mSvqUMTdeuUTznTCRzUpa5cFI9AR6TRk4Z /dE7MM8iu5wNmafFCjqi6o4Tu5TR8QcGvgSKs7s3PFQIkqBBLTykboLX2Ho19xdrJN48 DNEhMj9QvJuIfkkGy4LRNhIYU/prnte20pOZNVRqGy9OXtpHh9oihK9L/IYIF4mPfhfU PG5vnv0lBRcazOhJ7LCP4WutDkC+VDM8o7ahGYbPU6o5dgbbAYFEqtohFuiidWvVFigp t28Q== 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=ONehkVDhEBGP6xprW7TxLIU9bYv8NrlnrjT4SusYHJ8=; b=MzaQlq+K4jx8ZHiVkgOYuO/eWeBZ/5DUrBcV5ZoQ9dqiwCuwAuniDp0CNbEdT4OMNc R+wtwrY2Lq+B0t8VM6HOMYid/pZEvJYOZ+3M+UVYv1nuOf8peeUSCGTGYfADSCNatAZ3 QkqGgyVQjzsGe4kzKGmRIRXHGWCmLlWeZ3i52b4Eis7TnTG+x+mYvf+O6FyE5zM7jP7e iMniq1xcShN/t8sIe04Yqgx0CGBvFvZke0MXeOzTnvwo12ioRHM38Nu/sLhkdhDWBtGU iZizXyEBGnpyJ69zhWo5nOQpnUJr5TC4LzJGp7/5x5P2gYJixifz0QXQU5YfC5H+igwZ w3JQ== X-Gm-Message-State: AMke39nx6+eb0b1msN7gjzkULD1lFflMc4VbdbvCD1viiCpr0tZiY3Qq3OcfVi2HgPxW40uD X-Received: by 10.28.206.15 with SMTP id e15mr3104903wmg.43.1488369444907; Wed, 01 Mar 2017 03:57:24 -0800 (PST) Received: from glider0.muc.corp.google.com ([100.105.28.21]) by smtp.gmail.com with ESMTPSA id b17sm6628627wma.33.2017.03.01.03.57.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 01 Mar 2017 03:57:24 -0800 (PST) From: Alexander Potapenko To: dvyukov@google.com, kcc@google.com, edumazet@google.com Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH v4] net: don't call strlen() on the user buffer in packet_bind_spkt() Date: Wed, 1 Mar 2017 12:57:20 +0100 Message-Id: <20170301115720.99985-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 packet_bind_spkt(): Acked-by: Eric Dumazet ================================================================== BUG: KMSAN: use of unitialized memory CPU: 0 PID: 1074 Comm: packet Not tainted 4.8.0-rc6+ #1891 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 0000000000000000 ffff88006b6dfc08 ffffffff82559ae8 ffff88006b6dfb48 ffffffff818a7c91 ffffffff85b9c870 0000000000000092 ffffffff85b9c550 0000000000000000 0000000000000092 00000000ec400911 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:1003 [] __msan_warning+0x5b/0xb0 mm/kmsan/kmsan_instr.c:424 [< inline >] strlen lib/string.c:484 [] strlcpy+0x9d/0x200 lib/string.c:144 [] packet_bind_spkt+0x144/0x230 net/packet/af_packet.c:3132 [] SYSC_bind+0x40d/0x5f0 net/socket.c:1370 [] SyS_bind+0x82/0xa0 net/socket.c:1356 [] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:? chained origin: 00000000eba00911 [] 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:334 [] kmsan_internal_chain_origin+0x118/0x1e0 mm/kmsan/kmsan.c:527 [] __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 [] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:? origin description: ----address@SYSC_bind (origin=00000000eb400911) ================================================================== (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 #include int main() { struct sockaddr addr; memset(&addr, 0xff, sizeof(addr)); addr.sa_family = AF_PACKET; int fd = socket(PF_PACKET, SOCK_PACKET, htons(ETH_P_ALL)); bind(fd, &addr, sizeof(addr)); return 0; } ===================================== This happens because addr.sa_data copied from the userspace is not zero-terminated, and copying it with strlcpy() in packet_bind_spkt() results in calling strlen() on the kernel copy of that non-terminated buffer. Signed-off-by: Alexander Potapenko --- Changes since v3: - addressed comments by Eric Dumazet (avoid using constants, use memcpy() instead of strncpy()) --- net/packet/af_packet.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 2bd0d1949312..a0dbe7ca8f72 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3103,7 +3103,7 @@ static int packet_bind_spkt(struct socket *sock, struct sockaddr *uaddr, int addr_len) { struct sock *sk = sock->sk; - char name[15]; + char name[sizeof(uaddr->sa_data) + 1]; /* * Check legality @@ -3111,7 +3111,11 @@ static int packet_bind_spkt(struct socket *sock, struct sockaddr *uaddr, if (addr_len != sizeof(struct sockaddr)) return -EINVAL; - strlcpy(name, uaddr->sa_data, sizeof(name)); + /* uaddr->sa_data comes from the userspace, it's not guaranteed to be + * zero-terminated. + */ + memcpy(name, uaddr->sa_data, sizeof(uaddr->sa_data)); + name[sizeof(uaddr->sa_data)] = 0; return packet_do_bind(sk, name, 0, pkt_sk(sk)->num); }