From patchwork Wed Nov 11 22:45:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Xu X-Patchwork-Id: 11898861 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 559F1C56201 for ; Thu, 12 Nov 2020 01:52:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 13FE92076E for ; Thu, 12 Nov 2020 01:52:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=dxuuu.xyz header.i=@dxuuu.xyz header.b="FZqM56GK"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Y7iH9vFS" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728262AbgKLBcE (ORCPT ); Wed, 11 Nov 2020 20:32:04 -0500 Received: from wout2-smtp.messagingengine.com ([64.147.123.25]:41073 "EHLO wout2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726970AbgKKWqh (ORCPT ); Wed, 11 Nov 2020 17:46:37 -0500 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 198FB638; Wed, 11 Nov 2020 17:46:36 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Wed, 11 Nov 2020 17:46:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dxuuu.xyz; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm2; bh=JB1VBqITSKhPX sfUHzxUXkMo7GYuI0H/W3InHGhrcyk=; b=FZqM56GKkg2GCPlbMg3q8u+T9Az+t gpirqbGJBKtYgwu1V51EYkZcjDMQalXcufXU1zD8S2GJbbv06JXWvv4OrEwMgxpu cJlX5oxhpLzBrB2FC+WEfP+uYww3RhFcUX5oh4LZxXbBADpMuOfF1l3/H9fhhkYU RSg6RHDK3yVM659FSxEeh6S7KfyzvfHeooS3UvEXE9nwM3WugIwYxrjDqjFxTrKF 1bMQs6/Zel4vDnD4d5iOHWnADyxwnB46nPN+0Xa9POFKoDOg5MomazLhVUx9YXFm aq7xjaowxPB66KrEyYCiuRcT3rThngwNaTJnKmqhfUdbFufR2OcyqrQsQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; bh=JB1VBqITSKhPXsfUHzxUXkMo7GYuI0H/W3InHGhrcyk=; b=Y7iH9vFS tpwfKsdEayadu45on3j5hG/YDoswnVmEGzG2NXWEEdxhLUb52mhgNXlCFBgpiZE/ Fa5wVnmgZiFo339m/EVuubuOjtNFmbz29Z2pUFYF4+VyGUMDJ//3KwV+mtkKpa2C xLnnqcI+cg90pblb2vfqwUxf1SJwelAig/jTlYAzjkynkzNa1TpeuLT9RV0K8gkn EIhCEJ725OobPC/siONBU77D6rcXLpbnJuVaGWYJj6E0DIlwW+k8n3B6t+acnqgj HXZNXvLVXg87QI5Wd4OhxtwPTGLdivk5/KEBfZea4GsYALbwDR1FMTCCuF0SVt+L whya2eBzv6fObQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedruddvuddgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucgfrhhlucfvnfffucdljedtmdenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepffgrnhhivghlucgiuhcuoegu gihusegugihuuhhurdighiiiqeenucggtffrrghtthgvrhhnpefgkeduleekhfetvefhge fgvdegfeejfefguedvuddthffggffhhedtueeuteefieenucfkphepieelrddukedurddu tdehrdeigeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpegugihusegugihuuhhurdighiii X-ME-Proxy: Received: from localhost.localdomain (c-69-181-105-64.hsd1.ca.comcast.net [69.181.105.64]) by mail.messagingengine.com (Postfix) with ESMTPA id B61CC3063080; Wed, 11 Nov 2020 17:46:34 -0500 (EST) From: Daniel Xu To: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, songliubraving@fb.com, andrii.nakryiko@gmail.com Cc: Daniel Xu , kernel-team@fb.com Subject: [PATCH bpf v5 1/2] lib/strncpy_from_user.c: Don't overcopy bytes after NUL terminator Date: Wed, 11 Nov 2020 14:45:54 -0800 Message-Id: X-Mailer: git-send-email 2.29.2 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net do_strncpy_from_user() may copy some extra bytes after the NUL terminator into the destination buffer. This usually does not matter for normal string operations. However, when BPF programs key BPF maps with strings, this matters a lot. A BPF program may read strings from user memory by calling the bpf_probe_read_user_str() helper which eventually calls do_strncpy_from_user(). The program can then key a map with the resulting string. BPF map keys are fixed-width and string-agnostic, meaning that map keys are treated as a set of bytes. The issue is when do_strncpy_from_user() overcopies bytes after the NUL terminator, it can result in seemingly identical strings occupying multiple slots in a BPF map. This behavior is subtle and totally unexpected by the user. This commit has strncpy start copying a byte at a time if a NUL is spotted. Fixes: 6ae08ae3dea2 ("bpf: Add probe_read_{user, kernel} and probe_read_{user, kernel}_str helpers") Signed-off-by: Daniel Xu --- lib/strncpy_from_user.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c index e6d5fcc2cdf3..83180742e729 100644 --- a/lib/strncpy_from_user.c +++ b/lib/strncpy_from_user.c @@ -40,12 +40,11 @@ static inline long do_strncpy_from_user(char *dst, const char __user *src, /* Fall back to byte-at-a-time if we get a page fault */ unsafe_get_user(c, (unsigned long __user *)(src+res), byte_at_a_time); + if (has_zero(c, &data, &constants)) + goto byte_at_a_time; + *(unsigned long *)(dst+res) = c; - if (has_zero(c, &data, &constants)) { - data = prep_zero_mask(c, data, &constants); - data = create_zero_mask(data); - return res + find_zero(data); - } + res += sizeof(unsigned long); max -= sizeof(unsigned long); } From patchwork Wed Nov 11 22:45:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Xu X-Patchwork-Id: 11898859 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2721CC4742C for ; Thu, 12 Nov 2020 01:51:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BCD1D2076E for ; Thu, 12 Nov 2020 01:51:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=dxuuu.xyz header.i=@dxuuu.xyz header.b="wRAitTtI"; dkim=temperror (0-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="OgQn8tFz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728268AbgKLBcF (ORCPT ); Wed, 11 Nov 2020 20:32:05 -0500 Received: from wout2-smtp.messagingengine.com ([64.147.123.25]:42209 "EHLO wout2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727010AbgKKWqi (ORCPT ); Wed, 11 Nov 2020 17:46:38 -0500 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 1EC889F4; Wed, 11 Nov 2020 17:46:37 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Wed, 11 Nov 2020 17:46:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dxuuu.xyz; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm2; bh=e+edu/6Q4wBbY YEXH8t+dOvgpbiWbWHOudleytAG4Gc=; b=wRAitTtIJLZuVX0UUBkgTz3WmQexj QHFccGR2MaKPp6jHmBkRqw3NZdocg14ROBnUNYwaHfMfSX5qt+YoKjMu007arqrW 7g7PNKJ8QJo6I2oWbUlFWmU9JIvxmM4eX7JioYfwldjdUXQ0JqFMlZnxP8Yql8i/ K/uywSbjbl4zTJC+2VrB1oWoDGaPQgkgtzAfcjs4opTKZ5OjBWSATi/Px0Wvm172 luMptdwsE/RmWE93wOUkt+gitBgiaVTeJV6/PVH5iyjj8MLW6hRVsNzf37LSm1Fk nGkCtblAmkClRMzvlrgt2IatK0UotHdhAH3p/Zars7zzW2cZpck+LzqJw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; bh=e+edu/6Q4wBbYYEXH8t+dOvgpbiWbWHOudleytAG4Gc=; b=OgQn8tFz 3IRVTygna6cMt3a7jB2fpoSLEH4heXyUZUHfHYz8g+n7+HJi3vthXHIetOxW+dJX eXC5lOY6JoG4biEsvLAHQuyGm8uSJ/OKutlfpmy4JXLZxziawxVSmXu8QqgpIP2H s1uY5WtHHfV8pqHahdnMZA12rvangT6cVYCY6xSHPp8SRRBb2BFYR22UsOuZRnzS KgPJkRcJgtXvGIDpXW1I60o48N0IbYmCF7y6PL3I3T2f/yRBxMrTPMhc7RdtdUhc okcDJV3bHCMTsddT+0OQ1nscWt7+XRxprUINF3NEPb890e4WqgW5H8ER8sAcUe2f 9X26n+DdbUI25g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedruddvuddgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne gfrhhlucfvnfffucdlvdefmdenucfjughrpefhvffufffkofgjfhgggfestdekredtredt tdenucfhrhhomhepffgrnhhivghlucgiuhcuoegugihusegugihuuhhurdighiiiqeenuc ggtffrrghtthgvrhhnpefgkeduleekhfetvefhgefgvdegfeejfefguedvuddthffggffh hedtueeuteefieenucfkphepieelrddukedurddutdehrdeigeenucevlhhushhtvghruf hiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegugihusegugihuuhhurdighiii X-ME-Proxy: Received: from localhost.localdomain (c-69-181-105-64.hsd1.ca.comcast.net [69.181.105.64]) by mail.messagingengine.com (Postfix) with ESMTPA id AA98C3063067; Wed, 11 Nov 2020 17:46:35 -0500 (EST) From: Daniel Xu To: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, songliubraving@fb.com, andrii.nakryiko@gmail.com Cc: Daniel Xu , kernel-team@fb.com, Andrii Nakryiko Subject: [PATCH bpf v5 2/2] selftest/bpf: Test bpf_probe_read_user_str() strips trailing bytes after NUL Date: Wed, 11 Nov 2020 14:45:55 -0800 Message-Id: <76d760cf4013887b9238c965564ae7c2415720db.1605134506.git.dxu@dxuuu.xyz> X-Mailer: git-send-email 2.29.2 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Previously, bpf_probe_read_user_str() could potentially overcopy the trailing bytes after the NUL due to how do_strncpy_from_user() does the copy in long-sized strides. The issue has been fixed in the previous commit. This commit adds a selftest that ensures we don't regress bpf_probe_read_user_str() again. Acked-by: Song Liu Acked-by: Andrii Nakryiko Signed-off-by: Daniel Xu --- .../bpf/prog_tests/probe_read_user_str.c | 71 +++++++++++++++++++ .../bpf/progs/test_probe_read_user_str.c | 25 +++++++ 2 files changed, 96 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c create mode 100644 tools/testing/selftests/bpf/progs/test_probe_read_user_str.c diff --git a/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c new file mode 100644 index 000000000000..e419298132b5 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/probe_read_user_str.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include "test_probe_read_user_str.skel.h" + +static const char str1[] = "mestring"; +static const char str2[] = "mestringalittlebigger"; +static const char str3[] = "mestringblubblubblubblubblub"; + +static int test_one_str(struct test_probe_read_user_str *skel, const char *str, + size_t len) +{ + int err, duration = 0; + char buf[256]; + + /* Ensure bytes after string are ones */ + memset(buf, 1, sizeof(buf)); + memcpy(buf, str, len); + + /* Give prog our userspace pointer */ + skel->bss->user_ptr = buf; + + /* Trigger tracepoint */ + usleep(1); + + /* Did helper fail? */ + if (CHECK(skel->bss->ret < 0, "prog_ret", "prog returned: %ld\n", + skel->bss->ret)) + return 1; + + /* Check that string was copied correctly */ + err = memcmp(skel->bss->buf, str, len); + if (CHECK(err, "memcmp", "prog copied wrong string")) + return 1; + + /* Now check that no extra trailing bytes were copied */ + memset(buf, 0, sizeof(buf)); + err = memcmp(skel->bss->buf + len, buf, sizeof(buf) - len); + if (CHECK(err, "memcmp", "trailing bytes were not stripped")) + return 1; + + return 0; +} + +void test_probe_read_user_str(void) +{ + struct test_probe_read_user_str *skel; + int err, duration = 0; + + skel = test_probe_read_user_str__open_and_load(); + if (CHECK(!skel, "test_probe_read_user_str__open_and_load", + "skeleton open and load failed\n")) + return; + + /* Give pid to bpf prog so it doesn't read from anyone else */ + skel->bss->pid = getpid(); + + err = test_probe_read_user_str__attach(skel); + if (CHECK(err, "test_probe_read_user_str__attach", + "skeleton attach failed: %d\n", err)) + goto out; + + if (test_one_str(skel, str1, sizeof(str1))) + goto out; + if (test_one_str(skel, str2, sizeof(str2))) + goto out; + if (test_one_str(skel, str3, sizeof(str3))) + goto out; + +out: + test_probe_read_user_str__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c new file mode 100644 index 000000000000..3ae398b75dcd --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_probe_read_user_str.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include + +#include + +pid_t pid = 0; +long ret = 0; +void *user_ptr = 0; +char buf[256] = {}; + +SEC("tracepoint/syscalls/sys_enter_nanosleep") +int on_write(void *ctx) +{ + if (pid != (bpf_get_current_pid_tgid() >> 32)) + return 0; + + ret = bpf_probe_read_user_str(buf, sizeof(buf), user_ptr); + + return 0; +} + +char _license[] SEC("license") = "GPL";