In the scenario where livepatch and kretfunc coexist, the pageattr of
im->image is rox after arch_prepare_bpf_trampoline in
bpf_trampoline_update, and then modify_fentry or register_fentry returns
-EAGAIN from bpf_tramp_ftrace_ops_func, the BPF_TRAMP_F_ORIG_STACK flag
will be configured, and arch_prepare_bpf_trampoline will be re-executed.
At this time, because the pageattr of im->image is rox,
arch_prepare_bpf_trampoline will read and write im->image, which causes
a fault. as follows:
insmod livepatch-sample.ko # samples/livepatch/livepatch-sample.c
bpftrace -e 'kretfunc:cmdline_proc_show {}'
BUG: unable to handle page fault for address: ffffffffa0206000
PGD 322d067 P4D 322d067 PUD 322e063 PMD 1297e067 PTE d428061
Oops: 0003 [#1] PREEMPT SMP PTI
CPU: 2 PID: 270 Comm: bpftrace Tainted: G E K 6.1.0 #5
RIP: 0010:arch_prepare_bpf_trampoline+0xed/0x8c0
RSP: 0018:ffffc90001083ad8 EFLAGS: 00010202
RAX: ffffffffa0206000 RBX: 0000000000000020 RCX: 0000000000000000
RDX: ffffffffa0206001 RSI: ffffffffa0206000 RDI: 0000000000000030
RBP: ffffc90001083b70 R08: 0000000000000066 R09: ffff88800f51b400
R10: 000000002e72c6e5 R11: 00000000d0a15080 R12: ffff8880110a68c8
R13: 0000000000000000 R14: ffff88800f51b400 R15: ffffffff814fec10
FS: 00007f87bc0dc780(0000) GS:ffff88803e600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffa0206000 CR3: 0000000010b70000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
bpf_trampoline_update+0x25a/0x6b0
__bpf_trampoline_link_prog+0x101/0x240
bpf_trampoline_link_prog+0x2d/0x50
bpf_tracing_prog_attach+0x24c/0x530
bpf_raw_tp_link_attach+0x73/0x1d0
__sys_bpf+0x100e/0x2570
__x64_sys_bpf+0x1c/0x30
do_syscall_64+0x5b/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
With this patch, when modify_fentry or register_fentry returns -EAGAIN
from bpf_tramp_ftrace_ops_func, the pageattr of im->image will be reset
to nx+rw.
Cc: [email protected]
Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
Signed-off-by: Chuang Wang <[email protected]>
---
kernel/bpf/trampoline.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 11f5ec0b8016..d0ed7d6f5eec 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -488,6 +488,10 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
/* reset fops->func and fops->trampoline for re-register */
tr->fops->func = NULL;
tr->fops->trampoline = 0;
+
+ /* reset im->image memory attr for arch_prepare_bpf_trampoline */
+ set_memory_nx((long)im->image, 1);
+ set_memory_rw((long)im->image, 1);
goto again;
}
#endif
--
2.37.2
On Sat, Dec 24, 2022 at 09:31:46PM +0800, Chuang Wang wrote:
> In the scenario where livepatch and kretfunc coexist, the pageattr of
> im->image is rox after arch_prepare_bpf_trampoline in
> bpf_trampoline_update, and then modify_fentry or register_fentry returns
> -EAGAIN from bpf_tramp_ftrace_ops_func, the BPF_TRAMP_F_ORIG_STACK flag
> will be configured, and arch_prepare_bpf_trampoline will be re-executed.
>
> At this time, because the pageattr of im->image is rox,
> arch_prepare_bpf_trampoline will read and write im->image, which causes
> a fault. as follows:
>
> insmod livepatch-sample.ko # samples/livepatch/livepatch-sample.c
> bpftrace -e 'kretfunc:cmdline_proc_show {}'
>
> BUG: unable to handle page fault for address: ffffffffa0206000
> PGD 322d067 P4D 322d067 PUD 322e063 PMD 1297e067 PTE d428061
> Oops: 0003 [#1] PREEMPT SMP PTI
> CPU: 2 PID: 270 Comm: bpftrace Tainted: G E K 6.1.0 #5
> RIP: 0010:arch_prepare_bpf_trampoline+0xed/0x8c0
> RSP: 0018:ffffc90001083ad8 EFLAGS: 00010202
> RAX: ffffffffa0206000 RBX: 0000000000000020 RCX: 0000000000000000
> RDX: ffffffffa0206001 RSI: ffffffffa0206000 RDI: 0000000000000030
> RBP: ffffc90001083b70 R08: 0000000000000066 R09: ffff88800f51b400
> R10: 000000002e72c6e5 R11: 00000000d0a15080 R12: ffff8880110a68c8
> R13: 0000000000000000 R14: ffff88800f51b400 R15: ffffffff814fec10
> FS: 00007f87bc0dc780(0000) GS:ffff88803e600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffa0206000 CR3: 0000000010b70000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> bpf_trampoline_update+0x25a/0x6b0
> __bpf_trampoline_link_prog+0x101/0x240
> bpf_trampoline_link_prog+0x2d/0x50
> bpf_tracing_prog_attach+0x24c/0x530
> bpf_raw_tp_link_attach+0x73/0x1d0
> __sys_bpf+0x100e/0x2570
> __x64_sys_bpf+0x1c/0x30
> do_syscall_64+0x5b/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> With this patch, when modify_fentry or register_fentry returns -EAGAIN
> from bpf_tramp_ftrace_ops_func, the pageattr of im->image will be reset
> to nx+rw.
>
> Cc: [email protected]
> Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
> Signed-off-by: Chuang Wang <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
jirka
> ---
> kernel/bpf/trampoline.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 11f5ec0b8016..d0ed7d6f5eec 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -488,6 +488,10 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
> /* reset fops->func and fops->trampoline for re-register */
> tr->fops->func = NULL;
> tr->fops->trampoline = 0;
> +
> + /* reset im->image memory attr for arch_prepare_bpf_trampoline */
> + set_memory_nx((long)im->image, 1);
> + set_memory_rw((long)im->image, 1);
> goto again;
> }
> #endif
> --
> 2.37.2
>
On Mon, Dec 26, 2022 at 2:25 PM Jiri Olsa <[email protected]> wrote:
>
> On Sat, Dec 24, 2022 at 09:31:46PM +0800, Chuang Wang wrote:
> > In the scenario where livepatch and kretfunc coexist, the pageattr of
> > im->image is rox after arch_prepare_bpf_trampoline in
> > bpf_trampoline_update, and then modify_fentry or register_fentry returns
> > -EAGAIN from bpf_tramp_ftrace_ops_func, the BPF_TRAMP_F_ORIG_STACK flag
> > will be configured, and arch_prepare_bpf_trampoline will be re-executed.
> >
> > At this time, because the pageattr of im->image is rox,
> > arch_prepare_bpf_trampoline will read and write im->image, which causes
> > a fault. as follows:
> >
> > insmod livepatch-sample.ko # samples/livepatch/livepatch-sample.c
> > bpftrace -e 'kretfunc:cmdline_proc_show {}'
> >
> > BUG: unable to handle page fault for address: ffffffffa0206000
> > PGD 322d067 P4D 322d067 PUD 322e063 PMD 1297e067 PTE d428061
> > Oops: 0003 [#1] PREEMPT SMP PTI
> > CPU: 2 PID: 270 Comm: bpftrace Tainted: G E K 6.1.0 #5
> > RIP: 0010:arch_prepare_bpf_trampoline+0xed/0x8c0
> > RSP: 0018:ffffc90001083ad8 EFLAGS: 00010202
> > RAX: ffffffffa0206000 RBX: 0000000000000020 RCX: 0000000000000000
> > RDX: ffffffffa0206001 RSI: ffffffffa0206000 RDI: 0000000000000030
> > RBP: ffffc90001083b70 R08: 0000000000000066 R09: ffff88800f51b400
> > R10: 000000002e72c6e5 R11: 00000000d0a15080 R12: ffff8880110a68c8
> > R13: 0000000000000000 R14: ffff88800f51b400 R15: ffffffff814fec10
> > FS: 00007f87bc0dc780(0000) GS:ffff88803e600000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: ffffffffa0206000 CR3: 0000000010b70000 CR4: 00000000000006e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > <TASK>
> > bpf_trampoline_update+0x25a/0x6b0
> > __bpf_trampoline_link_prog+0x101/0x240
> > bpf_trampoline_link_prog+0x2d/0x50
> > bpf_tracing_prog_attach+0x24c/0x530
> > bpf_raw_tp_link_attach+0x73/0x1d0
> > __sys_bpf+0x100e/0x2570
> > __x64_sys_bpf+0x1c/0x30
> > do_syscall_64+0x5b/0x80
> > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> >
> > With this patch, when modify_fentry or register_fentry returns -EAGAIN
> > from bpf_tramp_ftrace_ops_func, the pageattr of im->image will be reset
> > to nx+rw.
> >
> > Cc: [email protected]
> > Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
Good catch! Thanks for the fix!
> > Signed-off-by: Chuang Wang <[email protected]>
>
> Acked-by: Jiri Olsa <[email protected]>
Acked-by: Song Liu <[email protected]>
>
> jirka
>
> > ---
> > kernel/bpf/trampoline.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index 11f5ec0b8016..d0ed7d6f5eec 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -488,6 +488,10 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
> > /* reset fops->func and fops->trampoline for re-register */
> > tr->fops->func = NULL;
> > tr->fops->trampoline = 0;
> > +
> > + /* reset im->image memory attr for arch_prepare_bpf_trampoline */
> > + set_memory_nx((long)im->image, 1);
> > + set_memory_rw((long)im->image, 1);
> > goto again;
> > }
> > #endif
> > --
> > 2.37.2
> >
Hello:
This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <[email protected]>:
On Sat, 24 Dec 2022 21:31:46 +0800 you wrote:
> In the scenario where livepatch and kretfunc coexist, the pageattr of
> im->image is rox after arch_prepare_bpf_trampoline in
> bpf_trampoline_update, and then modify_fentry or register_fentry returns
> -EAGAIN from bpf_tramp_ftrace_ops_func, the BPF_TRAMP_F_ORIG_STACK flag
> will be configured, and arch_prepare_bpf_trampoline will be re-executed.
>
> At this time, because the pageattr of im->image is rox,
> arch_prepare_bpf_trampoline will read and write im->image, which causes
> a fault. as follows:
>
> [...]
Here is the summary with links:
- bpf: Fix panic due to wrong pageattr of im->image
https://git.kernel.org/bpf/bpf/c/9ed1d9aeef58
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html