On Tue, Jan 17, 2023 at 09:14:42PM -0800, Alexei Starovoitov wrote: > From: Alexei Starovoitov <ast@xxxxxxxxxx> > > There are several issues with copy_from_user_nofault(): > > - access_ok() is designed for user context only and for that reason > it has WARN_ON_IN_IRQ() which triggers when bpf, kprobe, eprobe > and perf on ppc are calling it from irq. > > - it's missing nmi_uaccess_okay() which is a nop on all architectures > except x86 where it's required. > The comment in arch/x86/mm/tlb.c explains the details why it's necessary. > Calling copy_from_user_nofault() from bpf, [ke]probe without this check is not safe. > > - __copy_from_user_inatomic() under CONFIG_HARDENED_USERCOPY is calling > check_object_size()->__check_object_size()->check_heap_object()->find_vmap_area()->spin_lock() > which is not safe to do from bpf, [ke]probe and perf due to potential deadlock. Er, this drops check_object_size() -- that needs to stay. The vmap area test in check_object_size is likely what needs fixing. It was discussed before: https://lore.kernel.org/lkml/YySML2HfqaE%2FwXBU@xxxxxxxxxxxxxxxxxxxx/ The only reason it was ultimately tolerable to remove the check from the x86-only _nmi function was because it was being used on compile-time sized copies. We need to fix the vmap lookup so the checking doesn't regress -- especially for trace, bpf, etc, where we could have much more interested dest/source/size combinations. :) -Kees -- Kees Cook
