netdev - Re: [PATCH bpf-next 1/2] bpf: Take return from set_memory_ro() into account with bpf_prog_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d163fc82-7942-4a6f-a09e-59562e3f9d01@csgroup.eu>
Date: Thu, 22 Feb 2024 08:53:28 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Daniel Borkmann <daniel@...earbox.net>, Hengqi Chen
	<hengqi.chen@...il.com>
CC: 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>, "bpf@...r.kernel.org"
	<bpf@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, Kees Cook <keescook@...omium.org>, "linux-hardening
 @ vger . kernel . org" <linux-hardening@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Take return from set_memory_ro() into
 account with bpf_prog_lock_ro()



Le 21/02/2024 à 18:30, Daniel Borkmann a écrit :
> On 2/19/24 7:39 AM, Christophe Leroy wrote:
>>
>>
>> Le 19/02/2024 à 02:40, Hengqi Chen a écrit :
>>> [Vous ne recevez pas souvent de courriers de hengqi.chen@...il.com. 
>>> Découvrez pourquoi ceci est important à 
>>> https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Hello Christophe,
>>>
>>> On Sun, Feb 18, 2024 at 6:55 PM Christophe Leroy
>>> <christophe.leroy@...roup.eu> wrote:
>>>>
>>>> set_memory_ro() can fail, leaving memory unprotected.
>>>>
>>>> Check its return and take it into account as an error.
>>>>
>>>
>>> I don't see a cover letter for this series, could you describe how
>>> set_memory_ro() could fail.
>>> (Most callsites of set_memory_ro() didn't check the return values)
>>
>> Yeah, there is no cover letter because as explained in patch 2 the two
>> patches are autonomous. The only reason why I sent it as a series is
>> because the patches both modify include/linux/filter.h in two places
>> that are too close to each other.
>>
>> I should have added a link to https://github.com/KSPP/linux/issues/7
>> See that link for detailed explanation.
>>
>> If we take powerpc as an exemple, set_memory_ro() is a frontend to
>> change_memory_attr(). When you look at change_memory_attr() you see it
>> can return -EINVAL in two cases. Then it calls
>> apply_to_existing_page_range(). When you go down the road you see you
>> can get -EINVAL or -ENOMEM from that function or its callees.
> 
> By that logic, don't you have the same issue when undoing all of this?
> E.g. take arch_protect_bpf_trampoline() / arch_unprotect_bpf_trampoline()
> which is not covered in here, but what happens if you set it first to ro
> and later the setting back to rw fails? How would the error path there
> look like? It's something you cannot recover.
> 

arch_protect_bpf_trampoline() is handled there 
https://patchwork.kernel.org/project/netdevbpf/patch/883c5a268483a89ab13ed630210328a926f16e5b.1708526584.git.christophe.leroy@csgroup.eu/

In case setting back to RW fails there is not security issue, the things 
will likely blow up later with a write access to write protected memory 
but in terms of security that's not a problem.