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

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 for Android: free password hash cracker in your pocket

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

Message-ID: <f1049f13-53e4-470e-89e5-d99d7e171d39@os.amperecomputing.com>
Date: Mon, 13 May 2024 21:19:39 -0600
From: Yang Shi <yang@...amperecomputing.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: will@...nel.org, scott@...amperecomputing.com, cl@...two.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: mm: force write fault for atomic RMW instructions

>> +
>> +	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

>