Re: [PATCH] arm64: mm: force write fault for atomic RMW instructions

Yang Shi
2024-05-14T03:19:47Z

>> +
>> + if (get_user(insn, (unsigned int __user *) instruction_pointer(regs))) {
>> + pagefault_enable();
>> + goto continue_fault;
>> + }
>> +
>> + if (aarch64_insn_is_class_atomic(insn)) {
>> + vm_flags = VM_WRITE;
>> + mm_flags |= FAULT_FLAG_WRITE;
>> + }
> The above would need to check if the fault is coming from a 64-bit user
> mode, otherwise the decoding wouldn't make sense:
>
> if (!user_mode(regs) || compat_user_mode(regs))
> return false;
>
> (assuming a separate function that checks the above and returns a bool;
> you'd need to re-enable the page faults)
>
> You also need to take care of endianness since the instructions are
> always little-endian. We use a similar pattern in user_insn_read():
>
> u32 instr;
> __le32 instr_le;
> if (get_user(instr_le, (__le32 __user *)instruction_pointer(regs)))
> return false;
> instr = le32_to_cpu(instr_le);
> ...
>
> That said, I'm not keen on this kernel workaround. If openjdk decides to
> improve some security and goes for PROT_EXEC-only mappings of its text
> sections, the above trick will no longer work.

I noticed futex does replace insns. IIUC, the below sequence should can
do the trick for exec-only, right?

disable privileged
read insn with ldxr
enable privileged

>