From patchwork Tue Feb 28 13:35:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Potapenko X-Patchwork-Id: 733531 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 3vXfkT5L7Mz9s7c for ; Wed, 1 Mar 2017 00:36:49 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="vjh9EY00"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752694AbdB1Ngl (ORCPT ); Tue, 28 Feb 2017 08:36:41 -0500 Received: from mail-wm0-f42.google.com ([74.125.82.42]:38092 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751660AbdB1Nf6 (ORCPT ); Tue, 28 Feb 2017 08:35:58 -0500 Received: by mail-wm0-f42.google.com with SMTP id u199so11872465wmd.1 for ; Tue, 28 Feb 2017 05:35: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=WBcnHwOcx0qzolCtbQG7wXwH5kHh9EEDSQikvmxOX2c=; b=vjh9EY002ckdv6vao2Og7+G2lA8SpxMCW5jqeD/kINJADVE1AlUnUFZDwwI/9VDnKz xaTsvc35LCQEXFaCZo5C69Yq5DwbMg8LT6JZ8eOYLGPBBqFy1+G7MjkZ4cX5EaYseu1e MGeIyZTXiP0qcLptTSx2tehK6cVJAG4ruJ1m4KUgIr2xc3go7548XaT97shuTJYskO6B cIO86HANLRkxRdm/hRdECWuJgcrCb1n/eEXXzYSaZ5BesD6Tbc/y/3g7v4aSeFXVx672 v/58mKXlu91I41nFkjRwbFCEY8ff48VKv3bTxCkw+9gETP3EImJwuLgQP8GP/P/YdDmL Sk1Q== 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=WBcnHwOcx0qzolCtbQG7wXwH5kHh9EEDSQikvmxOX2c=; b=aDaZzj5WM2c9oBimtH1W//CMnAg0rnYQ6Y2R1vdk1i9to4t0vaV0DMplKObljwXpv1 piYdcqtcr32TMnGb2AkSpG1g89B2zN8Zqga4VD3RizWdJ/ozf+e5eWeUwCKr+g1dFlAZ cKSieEIaHDwFCkiDkQ2VS9Syx82DOs0KVKSx0gzdXHmgIKCdjDeV6p0ZzawLXV9tE2l7 ELhD9sFV6yix7MQI1oSDoqfxqiWRFQV7/RujN5ny+Cwqb7NVG8RpDJf6U7oQU2yJBgJq QkJp1RPvBT+B26yLVbScXeRAFiF46oKe58aEIAm/VGv2GMHcTDDHbZe/FIjS6JHnASmQ 1tGg== X-Gm-Message-State: AMke39mMRQ1zOVYAODw/5mmxJnTK096IsA6JNQ0MUoYiNwU4JvdTkVHNLleD9ua+9b16bTQc X-Received: by 10.28.175.138 with SMTP id y132mr2615346wme.86.1488288933152; Tue, 28 Feb 2017 05:35:33 -0800 (PST) Received: from glider0.muc.corp.google.com ([100.105.28.21]) by smtp.gmail.com with ESMTPSA id 10sm18648334wmk.26.2017.02.28.05.35.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 28 Feb 2017 05:35:32 -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 v3] net: don't call strlen() on the user buffer in packet_bind_spkt() Date: Tue, 28 Feb 2017 14:35:29 +0100 Message-Id: <20170228133529.128491-1-glider@google.com> X-Mailer: git-send-email 2.11.0.483.g087da7b7c-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(): ================================================================== 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 v2: - per offline comment from Dmitry Vyukov use sizeof(name) - 1 instead of a constant. net/packet/af_packet.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 2bd0d1949312..d40c8e9ff364 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -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. + */ + name[sizeof(name) - 1] = '\0'; + strncpy(name, uaddr->sa_data, sizeof(name) - 1); return packet_do_bind(sk, name, 0, pkt_sk(sk)->num); }