Received-SPF: pass (google.com: domain of linux-kernel+bounces-104607-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Subject: Re: [PATCH bpf-next v2] bpf: Check return from set_memory_rox() and
 friends
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
 "David S. Miller" <davem@davemloft.net>, David Ahern <dsahern@kernel.org>,
 Alexei Starovoitov <ast@kernel.org>, Andrii Nakryiko <andrii@kernel.org>,
 Martin KaFai Lau <martin.lau@linux.dev>, Eduard Zingerman
 <eddyz87@gmail.com>, Song Liu <song@kernel.org>,
 Yonghong Song <yonghong.song@linux.dev>,
 John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>,
 Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
 Jiri Olsa <jolsa@kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
 Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
 Dave Hansen <dave.hansen@linux.intel.com>, "x86@kernel.org"
 <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
 Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>
Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
 "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 Kees Cook <keescook@chromium.org>
References: <883c5a268483a89ab13ed630210328a926f16e5b.1708526584.git.christophe.leroy@csgroup.eu>
 <14b840d4-631f-2ad9-fb92-540aa150250b@iogearbox.net>
 <7848b9e8-2326-4046-8cec-bac2255c8602@csgroup.eu>
From: Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <d6159a82-284e-780d-b1b6-d583caf07fea@iogearbox.net>
Date: Fri, 15 Mar 2024 17:00:38 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <7848b9e8-2326-4046-8cec-bac2255c8602@csgroup.eu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit

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