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@xxxxxxxxx. >>> 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@xxxxxxxxxx> 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@xxxxxxxxxx/ 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.
