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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Thanks,
Daniel




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux