From: Yonghong Song <yhs@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>, <kernel-team@fb.com>,
<x86@kernel.org>, KP Singh <kpsingh@kernel.org>
Subject: [PATCH bpf] x86/bpf: handle bpf-program-triggered exceptions properly
Date: Mon, 25 Jan 2021 16:12:19 -0800 [thread overview]
Message-ID: <20210126001219.845816-1-yhs@fb.com> (raw)
When reviewing patch ([1]), which adds a script to run bpf selftest
through qemu at /sbin/init stage, I found the following kernel bug
warning:
[ 112.118892] BUG: sleeping function called from invalid context at arch/x86/mm/fault.c:1351
[ 112.119805] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 354, name: new_name
[ 112.120512] 3 locks held by new_name/354:
[ 112.120868] #0: ffff88800476e0a0 (&p->lock){+.+.}-{3:3}, at: bpf_seq_read+0x3a/0x3d0
[ 112.121573] #1: ffffffff82d69800 (rcu_read_lock){....}-{1:2}, at: bpf_iter_run_prog+0x5/0x160
[ 112.122348] #2: ffff8880061c2088 (&mm->mmap_lock#2){++++}-{3:3}, at: exc_page_fault+0x1a1/0x640
[ 112.123128] Preemption disabled at:
[ 112.123130] [<ffffffff8108f913>] migrate_disable+0x33/0x80
[ 112.123942] CPU: 0 PID: 354 Comm: new_name Tainted: G O 5.11.0-rc4-00524-g6e66fbb
10597-dirty #1249
[ 112.124822] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-1.el7.centos 04/01
/2014
[ 112.125614] Call Trace:
[ 112.125835] dump_stack+0x77/0x97
[ 112.126137] ___might_sleep.cold.119+0xf2/0x106
[ 112.126537] exc_page_fault+0x1c1/0x640
[ 112.126888] asm_exc_page_fault+0x1e/0x30
[ 112.127241] RIP: 0010:bpf_prog_0a182df2d34af188_dump_bpf_prog+0xf5/0xb3c
[ 112.127825] Code: 00 00 8b 7d f4 41 8b 76 44 48 39 f7 73 06 48 01 fb 49 89 df 4c 89 7d d8 49 8b
bd 20 01 00 00 48 89 7d e0 49 8b bd e0 00 00 00 <48> 8b 7f 20 48 01 d7 48 89 7d e8 48 89 e9 48 83 c
1 d0 48 8b 7d c8
[ 112.129433] RSP: 0018:ffffc9000035fdc8 EFLAGS: 00010282
[ 112.129895] RAX: 0000000000000000 RBX: ffff888005a49458 RCX: 0000000000000024
[ 112.130509] RDX: 00000000000002f0 RSI: 0000000000000509 RDI: 0000000000000000
[ 112.131126] RBP: ffffc9000035fe20 R08: 0000000000000001 R09: 0000000000000000
[ 112.131737] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000400
[ 112.132355] R13: ffff888006085800 R14: ffff888004718540 R15: ffff888005a49458
[ 112.132990] ? bpf_prog_0a182df2d34af188_dump_bpf_prog+0xc8/0xb3c
[ 112.133526] bpf_iter_run_prog+0x75/0x160
[ 112.133880] __bpf_prog_seq_show+0x39/0x40
[ 112.134258] bpf_seq_read+0xf6/0x3d0
[ 112.134582] vfs_read+0xa3/0x1b0
[ 112.134873] ksys_read+0x4f/0xc0
[ 112.135166] do_syscall_64+0x2d/0x40
[ 112.135482] entry_SYSCALL_64_after_hwframe+0x44/0xa9
To reproduce the issue, with patch [1] and use the following script:
tools/testing/selftests/bpf/run_in_vm.sh -- cat /sys/fs/bpf/progs.debug
The reason of the above kernel warning is due to bpf program
tries to dereference an address of 0 and which is not caught
by bpf exception handling logic.
...
SEC("iter/bpf_prog")
int dump_bpf_prog(struct bpf_iter__bpf_prog *ctx)
{
struct bpf_prog *prog = ctx->prog;
struct bpf_prog_aux *aux;
...
if (!prog)
return 0;
aux = prog->aux;
...
... aux->dst_prog->aux->name ...
return 0;
}
If the aux->dst_prog is NULL pointer, a fault will happen when trying
to access aux->dst_prog->aux.
In arch/x86/mm/fault.c function do_usr_addr_fault(), we have following code
if (unlikely(cpu_feature_enabled(X86_FEATURE_SMAP) &&
!(hw_error_code & X86_PF_USER) &&
!(regs->flags & X86_EFLAGS_AC)))
{
bad_area_nosemaphore(regs, hw_error_code, address);
return;
}
When the test is run normally after login prompt, cpu_feature_enabled(X86_FEATURE_SMAP)
is true and bad_area_nosemaphore() is called and then fixup_exception() is called,
where bpf specific handler is able to fixup the exception.
But when the test is run at /sbin/init time, cpu_feature_enabled(X86_FEATURE_SMAP) is
false, the control reaches
if (unlikely(!mmap_read_trylock(mm))) {
if (!user_mode(regs) && !search_exception_tables(regs->ip)) {
/*
* Fault from code in kernel from
* which we do not expect faults.
*/
bad_area_nosemaphore(regs, hw_error_code, address);
return;
}
retry:
mmap_read_lock(mm);
} else {
/*
* The above down_read_trylock() might have succeeded in
* which case we'll have missed the might_sleep() from
* down_read():
*/
might_sleep();
}
and might_sleep() is triggered and the above kernel warning is print.
To fix the issue, before the above mmap_read_trylock(), we will check
whether fault ip can be served by bpf exception handler or not, if
yes, the exception will be fixed up and return.
[1] https://lore.kernel.org/bpf/20210123004445.299149-1-kpsingh@kernel.org/
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: KP Singh <kpsingh@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
arch/x86/include/asm/extable.h | 3 +++
arch/x86/mm/extable.c | 14 ++++++++++++++
arch/x86/mm/fault.c | 9 +++++++++
3 files changed, 26 insertions(+)
diff --git a/arch/x86/include/asm/extable.h b/arch/x86/include/asm/extable.h
index 1f0cbc52937c..1e99da15336c 100644
--- a/arch/x86/include/asm/extable.h
+++ b/arch/x86/include/asm/extable.h
@@ -38,6 +38,9 @@ enum handler_type {
extern int fixup_exception(struct pt_regs *regs, int trapnr,
unsigned long error_code, unsigned long fault_addr);
+extern int fixup_bpf_exception(struct pt_regs *regs, int trapnr,
+ unsigned long error_code,
+ unsigned long fault_addr);
extern int fixup_bug(struct pt_regs *regs, int trapnr);
extern enum handler_type ex_get_fault_handler_type(unsigned long ip);
extern void early_fixup_exception(struct pt_regs *regs, int trapnr);
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index b93d6cd08a7f..311da42c0aec 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -155,6 +155,20 @@ enum handler_type ex_get_fault_handler_type(unsigned long ip)
return EX_HANDLER_OTHER;
}
+int fixup_bpf_exception(struct pt_regs *regs, int trapnr,
+ unsigned long error_code, unsigned long fault_addr)
+{
+ const struct exception_table_entry *e;
+ ex_handler_t handler;
+
+ e = search_bpf_extables(regs->ip);
+ if (!e)
+ return 0;
+
+ handler = ex_fixup_handler(e);
+ return handler(e, regs, trapnr, error_code, fault_addr);
+}
+
int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
unsigned long fault_addr)
{
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index f1f1b5a0956a..e8182d30bf67 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1317,6 +1317,15 @@ void do_user_addr_fault(struct pt_regs *regs,
if (emulate_vsyscall(hw_error_code, regs, address))
return;
}
+
+#ifdef CONFIG_BPF_JIT
+ /*
+ * Faults incurred by bpf program might need emulation, i.e.,
+ * clearing the dest register.
+ */
+ if (fixup_bpf_exception(regs, X86_TRAP_PF, hw_error_code, address))
+ return;
+#endif
#endif
/*
--
2.24.1
next reply other threads:[~2021-01-26 5:39 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-26 0:12 Yonghong Song [this message]
2021-01-26 9:15 ` [PATCH bpf] x86/bpf: handle bpf-program-triggered exceptions properly Peter Zijlstra
2021-01-26 19:21 ` Yonghong Song
2021-01-27 9:30 ` Peter Zijlstra
2021-01-27 21:07 ` Andrii Nakryiko
2021-01-27 21:47 ` Andy Lutomirski
2021-01-28 1:05 ` Yonghong Song
2021-01-28 23:51 ` Andy Lutomirski
2021-01-29 0:11 ` Alexei Starovoitov
2021-01-29 0:18 ` Andy Lutomirski
2021-01-29 0:26 ` Alexei Starovoitov
2021-01-29 0:29 ` Andy Lutomirski
2021-01-29 0:32 ` Andy Lutomirski
2021-01-29 0:41 ` Alexei Starovoitov
2021-01-29 0:45 ` Andy Lutomirski
2021-01-29 1:04 ` Alexei Starovoitov
2021-01-29 1:31 ` Andy Lutomirski
2021-01-29 1:53 ` Alexei Starovoitov
2021-01-29 2:18 ` Andy Lutomirski
2021-01-29 2:32 ` Alexei Starovoitov
2021-01-29 3:09 ` Andy Lutomirski
2021-01-29 3:26 ` Alexei Starovoitov
2021-01-29 0:35 ` Jann Horn
2021-01-29 0:43 ` Alexei Starovoitov
2021-01-29 1:03 ` Jann Horn
2021-01-29 1:08 ` Alexei Starovoitov
[not found] <YBPToXfWV1ekRo4q@hirez.programming.kicks-ass.net>
[not found] ` <97EF0157-F068-4234-B826-C08B7324F356@amacapital.net>
2021-01-29 23:11 ` Alexei Starovoitov
2021-01-29 23:30 ` Andy Lutomirski
2021-01-30 2:54 ` Alexei Starovoitov
2021-01-31 17:32 ` Andy Lutomirski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210126001219.845816-1-yhs@fb.com \
--to=yhs@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=kpsingh@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox