[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d6159a82-284e-780d-b1b6-d583caf07fea@iogearbox.net>
Date: Fri, 15 Mar 2024 17:00:38 +0100
From: Daniel Borkmann <daniel@...earbox.net>
To: Christophe Leroy <christophe.leroy@...roup.eu>,
"David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>,
Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
<eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, "x86@...nel.org"
<x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH bpf-next v2] bpf: Check return from set_memory_rox() and
friends
On 3/15/24 3:55 PM, Christophe Leroy wrote:
[...]
>>> {
>>> WARN_ON_ONCE(size > PAGE_SIZE);
>>> - set_memory_rox((long)image, 1);
>>> + return set_memory_rox((long)image, 1);
>>> }
>>> -void __weak arch_unprotect_bpf_trampoline(void *image, unsigned int
>>> size)
>>> +int __weak arch_unprotect_bpf_trampoline(void *image, unsigned int size)
>>> {
>>> + int err;
>>> WARN_ON_ONCE(size > PAGE_SIZE);
>>> - set_memory_nx((long)image, 1);
>>> - set_memory_rw((long)image, 1);
>>> +
>>> + err = set_memory_nx((long)image, 1);
>>> + if (err)
>>> + return err;
>>> + return set_memory_rw((long)image, 1);
>>> }
>>
>> Do we still need this? It looks like this does not have an in-tree user
>> anymore.
>
> Looks like last user went away with commit 187e2af05abe ("bpf:
> struct_ops supports more than one page for trampolines.") but I'm having
> hard time figuring if it's valid or not.
>
> But as there is no user anymore it surely can go away. Will you drop it
> or do you want a proper patch from me ?
My understanding is that the VM_FLUSH_RESET_PERMS would take care of this
via arch_alloc_bpf_trampoline(). Anyway, gvien there is a merge conflict
with this patch, pls include it with a v3.
Thanks,
Daniel
Powered by blists - more mailing lists