All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Chuang Wang <nashuiliang@gmail.com>
Cc: stable@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] bpf: Fix panic due to wrong pageattr of im->image
Date: Mon, 26 Dec 2022 23:25:23 +0100	[thread overview]
Message-ID: <Y6ofUzLHZKraIDre@krava> (raw)
In-Reply-To: <20221224133146.780578-1-nashuiliang@gmail.com>

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: stable@vger.kernel.org
> Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
> Signed-off-by: Chuang Wang <nashuiliang@gmail.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

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
> 

  reply	other threads:[~2022-12-26 22:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-24 13:31 [PATCH] bpf: Fix panic due to wrong pageattr of im->image Chuang Wang
2022-12-26 22:25 ` Jiri Olsa [this message]
2022-12-27 19:08   ` Song Liu
2022-12-28 22:10 ` patchwork-bot+netdevbpf

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=Y6ofUzLHZKraIDre@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=nashuiliang@gmail.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=yhs@fb.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.