From patchwork Tue Feb 28 13:29:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Potapenko X-Patchwork-Id: 733525 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 3vXfZW1WnQz9s0g for ; Wed, 1 Mar 2017 00:29:55 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="QJQw6RtI"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752690AbdB1N3w (ORCPT ); Tue, 28 Feb 2017 08:29:52 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:37363 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751441AbdB1N3r (ORCPT ); Tue, 28 Feb 2017 08:29:47 -0500 Received: by mail-wm0-f48.google.com with SMTP id v77so11687025wmv.0 for ; Tue, 28 Feb 2017 05:29:46 -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=bphP3lhVjjqsMzbt//EHQaH2tk6pno3wVimDgS5U3ww=; b=QJQw6RtI2EOpOgb8lpnMriBP2uWF9iLLOWKbHkzi+01a0MLhGf5XTT8IELzWu9P3pF X6LMn4lxGj0RfniVW8QerctOTTljIhd+0cv7e7dut0uKHm7zX0SPG96c+tbTtHBMwnqK KzocGfJzvZwu4pKHUp7fIJNcfzj1SPeRuxbbI9oSQWHQ16dCpcHv0hzZ5C94O4z1YMQ+ peMPE93it7uOTGuSaPoga5Ke44PVRf9vgHMF9y0Ac4RInbj3LQ6cszw8CvzmWfGrh69i TnxLxnzZHqPYS6wE53qUtv21i+qvwt71QZsOd9sTDKwU0n3eZwwyuoeuVNeJo8c76yaR mkYg== 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=bphP3lhVjjqsMzbt//EHQaH2tk6pno3wVimDgS5U3ww=; b=YSfmaiYVEtMqK6JY56i5kvIN5oM9GDIspZdEcDpIlLvHubEysxUaeQUjWv5la98CnA NpllCICVLHw3kYiqKNjaeRzGkaX+xNehzr5Gb/WhPkC2IEGtA8Lt73ESCeh1nGGcE70G L5SBta86rcAQogaV2+uUZuSLH3criY2H07+udaKGPkzrpUq1bx8Ut528Zx7eAGxU9Q8y MoPSo2/MQwep7mOJmYObykEUt7bz+imZKiCrQuHocyOSTRMn3K+L10cl6+5/QTpPpEBY DyumTBHJoq1RcIhylqQBfB5RTbixaGbaiY8O4zn35WYyfkb92lRpiE7zpcZg5jNoFi50 si2A== X-Gm-Message-State: AMke39m/AOnfeccwvENreep2TDAXVPkE6lQkXfj1Z3tpQFzisjxo2+jq4CMyQbZBbIgyH606 X-Received: by 10.28.170.4 with SMTP id t4mr2576632wme.89.1488288574509; Tue, 28 Feb 2017 05:29:34 -0800 (PST) Received: from glider0.muc.corp.google.com ([100.105.28.21]) by smtp.gmail.com with ESMTPSA id o2sm2681033wmb.28.2017.02.28.05.29.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 28 Feb 2017 05:29:33 -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 v2] net: don't call strlen() on the user buffer in packet_bind_spkt() Date: Tue, 28 Feb 2017 14:29:31 +0100 Message-Id: <20170228132931.123155-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 v1: - copy no more than 14 bytes --- 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..ac6b3c77a52e 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[14] = '\0'; + strncpy(name, uaddr->sa_data, sizeof(name) - 1); return packet_do_bind(sk, name, 0, pkt_sk(sk)->num); }