linux-hardening - Re: [PATCH] lkdtm: Add CFI_BACKWARD to test ROP mitigations

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 15 Apr 2022 01:39:49 -0700
From:   Dan Li <ashimida@...ux.alibaba.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] lkdtm: Add CFI_BACKWARD to test ROP mitigations



On 4/14/22 10:22, Kees Cook wrote:
> On Thu, Apr 14, 2022 at 03:19:02AM -0700, Dan Li wrote:
>> Hi, Kees,
>> Thanks for the rewrite. I tested this patch, and it works fine for
>> me except for a few minor comments below :)
>> We may need to ignore the pac high bits of return address according
>> to TCR.T1SZ (or simply remove the high 16 bits before comparing).
> 
> Oh! Hah, yes, I totally forgot that. Thanks for testing -- getting PAC
> emulation working in QEMU has eluded me. I think untagged_addr() will
> work yes? i.e.:
> 
> 	if((untagged_addr(*ret_addr) == expected)
> 

untagged_addr might not clear enough bits, the following code works
fine for me:

+#define no_pac_addr(addr)      \
+       ((__force __typeof__(addr))((__force u64)(addr) | PAGE_OFFSET))
        
-       if(*ret_addr == expected)
+       if(no_pac_addr(*ret_addr) == expected)
                 *ret_addr = (addr);

I re-checked the arm manual and found that the pac bits in an address is:
- Xn[54:bottom_PAC_bit]		 //When address tagging is used
- Xn[63:56, 54:bottom_PAC_bit]	 //When address tagging is not used

bottom_PAC_bit = 64-TCR_ELx.TnSZ //For kernel is VA_BITS

The pac bits could be at most [63:56, 54:VA_BITS], untagged_addr
clears [63:56] (and clearing the high 16 bits doesn't seem to be
enough either :) ).

For example, when CONFIG_ARM64_VA_BITS_39 enabled, i get a case:
- lr         : 0xffffffc0088d04f8
- lr with pac: 0x5681a740088d04f8
- PAGE_OFFSET: 0xffffff8000000000

"lr with pac|PAGE_OFFSET" seems to meet the need.

>>
>>> +	else
>>> +		/* Check architecture, stack layout, or compiler behavior... */
>>> +		pr_warn("Eek: return address mismatch! %px != %px\n",
>>> +			*ret_addr, addr);
>>
>> According to the context, it might be "expected" here?
>>
>> 		pr_warn("Eek: return address mismatch! %px != %px\n",
>> 			*ret_addr, expected);
>>
>> I simply ignored the upper 16 bits, and tested it separately
>> in gcc/llvm 12 with SCS/PAC and all the four cases worked fine for me.
> 
> Great! Do you have the PAC "Oops" text handy so I can include it in the
> commit log as an example of what should be expected?
> 

Yeah, in my test environment I get the following output:

/kselftest_install/lkdtm # ./CFI_BACKWARD.sh
[   58.333529] lkdtm: Performing direct entry CFI_BACKWARD
[   58.333927] lkdtm: Attempting unchecked stack return address redirection ...
[   58.334230] lkdtm: ok: redirected stack return address.
[   58.334870] lkdtm: Attempting checked stack return address redirection ...
[   58.336287] Unable to handle kernel paging request at virtual address bfffffc0088d0514
[   58.336633] Mem abort info:
[   58.336789]   ESR = 0x86000004
[   58.336992]   EC = 0x21: IABT (current EL), IL = 32 bits
[   58.337234]   SET = 0, FnV = 0
[   58.337429]   EA = 0, S1PTW = 0
[   58.337611]   FSC = 0x04: level 0 translation fault
[   58.337874] [bfffffc0088d0514] address between user and kernel address ranges
[   58.338304] Internal error: Oops: 86000004 [#1] PREEMPT SMP
[   58.339209] Modules linked in:
[   58.340384] CPU: 1 PID: 131 Comm: cat Not tainted 5.18.0-rc2-29474-gb8dcca8f6a13-dirty #393
[   58.340842] Hardware name: linux,dummy-virt (DT)
[   58.341231] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   58.341593] pc : 0xbfffffc0088d0514
[   58.342255] lr : lkdtm_CFI_BACKWARD+0x108/0x130
[   58.342716] sp : ffffffc00a73bc60
[   58.342906] x29: ffffffc00a73bc60 x28: ffffff8003320000 x27: 0000000000000002
[   58.343462] x26: ffffffc00a204d00 x25: 0000000000000002 x24: ffffffc0092e72f0
[   58.343863] x23: 0000000000000006 x22: ffffffc00a204d10 x21: ffffff80188d2000
[   58.344264] x20: ffffffc00a73bde0 x19: ffffffc00a302000 x18: ffffffffffffffff
[   58.344732] x17: 663a72646461202c x16: 3866343064383830 x15: 0000000000000004
[   58.345112] x14: 0000000000000fff x13: ffffffc009f8a3e0 x12: 0000000000000003
[   58.345492] x11: 00000000ffffefff x10: c0000000ffffefff x9 : 90af2887c07d9500
[   58.345926] x8 : ffffffc0088d04f8 x7 : 205d323234353333 x6 : 332e38352020205b
[   58.346288] x5 : ffffffc00a2c313c x4 : 0000000000000001 x3 : 0000000000000000
[   58.346670] x2 : 0000000000000000 x1 : ffffffc00968626d x0 : 000000000000006d
[   58.347154] Call trace:
[   58.347419]  0xbfffffc0088d0514
[   58.347806]  lkdtm_do_action+0x1c/0x30
[   58.348085]  direct_entry+0x178/0x1b0
[   58.348291]  full_proxy_write+0x6c/0xe8
[   58.348523]  vfs_write+0x174/0x3b0
[   58.348721]  ksys_write+0x78/0xe4
[   58.348914]  __arm64_sys_write+0x1c/0x28
[   58.349125]  el0_svc_common+0xa4/0x134
[   58.349354]  do_el0_svc+0x24/0x7c
[   58.349551]  el0_svc+0x28/0xa4
[   58.349745]  el0t_64_sync_handler+0x84/0xe4
[   58.349995]  el0t_64_sync+0x170/0x174
[   58.350659] Code: bad PC value
[   58.351182] ---[ end trace 0000000000000000 ]---
CFI_BACKWARD: saw 'call trace:|ok: control flow unchanged': ok

Thanks,
Dan.