tmakin/ack-tegra
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'fix-the-read-of-vsyscall-page-through-bpf'

Browse Source 
Hou Tao says:

====================
Fix the read of vsyscall page through bpf

From: Hou Tao <houtao1@huawei.com>

Hi,

As reported by syzboot [1] and [2], when trying to read vsyscall page
by using bpf_probe_read_kernel() or bpf_probe_read(), oops may happen.

Thomas Gleixner had proposed a test patch [3], but it seems that no
formal patch is posted after about one month [4], so I post it instead
and add an Originally-by tag in patch #2.

Patch #1 makes is_vsyscall_vaddr() being a common helper. Patch #2 fixes
the problem by disallowing vsyscall page read for
copy_from_kernel_nofault(). Patch #3 adds one test case to ensure the
read of vsyscall page through bpf is rejected. Please see individual
patches for more details.

Comments are always welcome.

[1]: https://lore.kernel.org/bpf/CAG48ez06TZft=ATH1qh2c5mpS5BT8UakwNkzi6nvK5_djC-4Nw@mail.gmail.com/
[2]: https://lore.kernel.org/bpf/CABOYnLynjBoFZOf3Z4BhaZkc5hx_kHfsjiW+UWLoB=w33LvScw@mail.gmail.com/
[3]: https://lore.kernel.org/bpf/87r0jwquhv.ffs@tglx/
[4]: https://lore.kernel.org/bpf/e24b125c-8ff4-9031-6c53-67ff2e01f316@huaweicloud.com/

Change Log:
v3:
 * rephrase commit message for patch #1 & #2 (Sohil)
 * reword comments in copy_from_kernel_nofault_allowed() (Sohil)
 * add Rvb tag for patch #1 and Acked-by tag for patch #3 (Sohil, Yonghong)

v2: https://lore.kernel.org/bpf/20240126115423.3943360-1-houtao@huaweicloud.com/
  * move is_vsyscall_vaddr to asm/vsyscall.h instead (Sohil)
  * elaborate on the reason for disallowing of vsyscall page read in
    copy_from_kernel_nofault_allowed() (Sohil)
  * update the commit message of patch #2 to more clearly explain how
    the oops occurs. (Sohil)
  * update the commit message of patch #3 to explain the expected return
    values of various bpf helpers (Yonghong)

v1: https://lore.kernel.org/bpf/20240119073019.1528573-1-houtao@huaweicloud.com/
====================

Link: https://lore.kernel.org/r/20240202103935.3154011-1-houtao@huaweicloud.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Alexei Starovoitov
2024-02-15 19:21:39 -08:00
parent e37243b65d be66d79189
commit 54d46c9f58
5 changed files with 122 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
arch/x86/include/asm/vsyscall.h
+10
View File
@@ -4,6 +4,7 @@
#include <linux/seqlock.h>
#include <uapi/asm/vsyscall.h>
#include <asm/page_types.h>
#ifdef CONFIG_X86_VSYSCALL_EMULATION
extern void map_vsyscall(void);
@@ -24,4 +25,13 @@ static inline bool emulate_vsyscall(unsigned long error_code,
}
#endif
/*
* The (legacy) vsyscall page is the long page in the kernel portion
* of the address space that has user-accessible permissions.
*/
static inline bool is_vsyscall_vaddr(unsigned long vaddr)
{
return unlikely((vaddr & PAGE_MASK) == VSYSCALL_ADDR);
}
#endif /* _ASM_X86_VSYSCALL_H */
arch/x86/mm/fault.c
-9
View File
@@ -798,15 +798,6 @@ show_signal_msg(struct pt_regs *regs, unsigned long error_code,
show_opcodes(regs, loglvl);
}
/*
* The (legacy) vsyscall page is the long page in the kernel portion
* of the address space that has user-accessible permissions.
*/
static bool is_vsyscall_vaddr(unsigned long vaddr)
{
return unlikely((vaddr & PAGE_MASK) == VSYSCALL_ADDR);
}
static void
__bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code,
unsigned long address, u32 pkey, int si_code)
arch/x86/mm/maccess.c
+10
View File
@@ -3,6 +3,8 @@
#include <linux/uaccess.h>
#include <linux/kernel.h>
#include <asm/vsyscall.h>
#ifdef CONFIG_X86_64
bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
{
@@ -15,6 +17,14 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
if (vaddr < TASK_SIZE_MAX + PAGE_SIZE)
return false;
/*
* Reading from the vsyscall page may cause an unhandled fault in
* certain cases. Though it is at an address above TASK_SIZE_MAX, it is
* usually considered as a user space address.
*/
if (is_vsyscall_vaddr(vaddr))
return false;
/*
* Allow everything during early boot before 'x86_virt_bits'
* is initialized. Needed for instruction decoding in early
tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
+57
View File
@@ -0,0 +1,57 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (C) 2024. Huawei Technologies Co., Ltd */
#include "test_progs.h"
#include "read_vsyscall.skel.h"
#if defined(__x86_64__)
/* For VSYSCALL_ADDR */
#include <asm/vsyscall.h>
#else
/* To prevent build failure on non-x86 arch */
#define VSYSCALL_ADDR 0UL
#endif
struct read_ret_desc {
const char *name;
int ret;
} all_read[] = {
{ .name = "probe_read_kernel", .ret = -ERANGE },
{ .name = "probe_read_kernel_str", .ret = -ERANGE },
{ .name = "probe_read", .ret = -ERANGE },
{ .name = "probe_read_str", .ret = -ERANGE },
{ .name = "probe_read_user", .ret = -EFAULT },
{ .name = "probe_read_user_str", .ret = -EFAULT },
{ .name = "copy_from_user", .ret = -EFAULT },
{ .name = "copy_from_user_task", .ret = -EFAULT },
};
void test_read_vsyscall(void)
{
struct read_vsyscall *skel;
unsigned int i;
int err;
#if !defined(__x86_64__)
test__skip();
return;
#endif
skel = read_vsyscall__open_and_load();
if (!ASSERT_OK_PTR(skel, "read_vsyscall open_load"))
return;
skel->bss->target_pid = getpid();
err = read_vsyscall__attach(skel);
if (!ASSERT_EQ(err, 0, "read_vsyscall attach"))
goto out;
/* userspace may don't have vsyscall page due to LEGACY_VSYSCALL_NONE,
* but it doesn't affect the returned error codes.
*/
skel->bss->user_ptr = (void *)VSYSCALL_ADDR;
usleep(1);
for (i = 0; i < ARRAY_SIZE(all_read); i++)
ASSERT_EQ(skel->bss->read_ret[i], all_read[i].ret, all_read[i].name);
out:
read_vsyscall__destroy(skel);
}
tools/testing/selftests/bpf/progs/read_vsyscall.c
+45
View File
@@ -0,0 +1,45 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (C) 2024. Huawei Technologies Co., Ltd */
#include <linux/types.h>
#include <bpf/bpf_helpers.h>
#include "bpf_misc.h"
int target_pid = 0;
void *user_ptr = 0;
int read_ret[8];
char _license[] SEC("license") = "GPL";
SEC("fentry/" SYS_PREFIX "sys_nanosleep")
int do_probe_read(void *ctx)
{
char buf[8];
if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
return 0;
read_ret[0] = bpf_probe_read_kernel(buf, sizeof(buf), user_ptr);
read_ret[1] = bpf_probe_read_kernel_str(buf, sizeof(buf), user_ptr);
read_ret[2] = bpf_probe_read(buf, sizeof(buf), user_ptr);
read_ret[3] = bpf_probe_read_str(buf, sizeof(buf), user_ptr);
read_ret[4] = bpf_probe_read_user(buf, sizeof(buf), user_ptr);
read_ret[5] = bpf_probe_read_user_str(buf, sizeof(buf), user_ptr);
return 0;
}
SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
int do_copy_from_user(void *ctx)
{
char buf[8];
if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
return 0;
read_ret[6] = bpf_copy_from_user(buf, sizeof(buf), user_ptr);
read_ret[7] = bpf_copy_from_user_task(buf, sizeof(buf), user_ptr,
bpf_get_current_task_btf(), 0);
return 0;
}