Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jiri Olsa <olsajiri@gmail.com>
Date:   Mon, 26 Dec 2022 23:25:23 +0100
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
Message-ID: <Y6ofUzLHZKraIDre@krava>
References: <20221224133146.780578-1-nashuiliang@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221224133146.780578-1-nashuiliang@gmail.com>
Precedence: bulk

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
>