From: Hou Tao <[email protected]>
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/[email protected]/
Change Log:
v2:
* 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/[email protected]/
Hou Tao (3):
x86/mm: Move is_vsyscall_vaddr() into asm/vsyscall.h
x86/mm: Disallow vsyscall page read for copy_from_kernel_nofault()
selftest/bpf: Test the read of vsyscall page under x86-64
arch/x86/include/asm/vsyscall.h | 10 ++++
arch/x86/mm/fault.c | 9 ---
arch/x86/mm/maccess.c | 9 +++
.../selftests/bpf/prog_tests/read_vsyscall.c | 57 +++++++++++++++++++
.../selftests/bpf/progs/read_vsyscall.c | 45 +++++++++++++++
5 files changed, 121 insertions(+), 9 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
create mode 100644 tools/testing/selftests/bpf/progs/read_vsyscall.c
--
2.29.2
From: Hou Tao <[email protected]>
Moving is_vsyscall_vaddr() into asm/vsyscall.h to make it available for
copy_from_kernel_nofault_allowed() in arch/x86/mm/maccess.c.
Signed-off-by: Hou Tao <[email protected]>
---
arch/x86/include/asm/vsyscall.h | 10 ++++++++++
arch/x86/mm/fault.c | 9 ---------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/vsyscall.h b/arch/x86/include/asm/vsyscall.h
index ab60a71a8dcb9..472f0263dbc61 100644
--- a/arch/x86/include/asm/vsyscall.h
+++ b/arch/x86/include/asm/vsyscall.h
@@ -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 */
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 679b09cfe241c..d6375b3c633bc 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -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)
--
2.29.2
On 1/26/2024 3:54 AM, Hou Tao wrote:
> From: Hou Tao <[email protected]>
>
> Moving is_vsyscall_vaddr() into asm/vsyscall.h to make it available for
s/Moving/Move
> copy_from_kernel_nofault_allowed() in arch/x86/mm/maccess.c.
>
> Signed-off-by: Hou Tao <[email protected]>
> ---
> arch/x86/include/asm/vsyscall.h | 10 ++++++++++
> arch/x86/mm/fault.c | 9 ---------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
Reviewed-by: Sohil Mehta <[email protected]>
On 1/30/2024 7:56 AM, Sohil Mehta wrote:
> On 1/26/2024 3:54 AM, Hou Tao wrote:
>> From: Hou Tao <[email protected]>
>>
>> Moving is_vsyscall_vaddr() into asm/vsyscall.h to make it available for
> s/Moving/Move
Will update in v3.
>
>> copy_from_kernel_nofault_allowed() in arch/x86/mm/maccess.c.
>>
>> Signed-off-by: Hou Tao <[email protected]>
>> ---
>> arch/x86/include/asm/vsyscall.h | 10 ++++++++++
>> arch/x86/mm/fault.c | 9 ---------
>> 2 files changed, 10 insertions(+), 9 deletions(-)
>>
>
> Reviewed-by: Sohil Mehta <[email protected]>
Thank you for the review and all of your suggestions.