# Re: \[PATCH v23 14/28\] x86/mm: Shadow Stack page fault error checking

- From: Yu, Yu-cheng \<hidden\>
- Date: 2021-03-22 17:28:52 
- List: linux-doc
- Page: https://ratatoskr.run/linux-doc/2021/03/6311044
- Thread: https://ratatoskr.run/linux-doc/2021/03/6303369/t.md

```text
On 3/22/2021 3:38 AM, Kirill A. Shutemov wrote:
> On Tue, Mar 16, 2021 at 08:10:40AM -0700, Yu-cheng Yu wrote:
[...]
>>   
>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
>> index a73347e2cdfc..4316732a18c6 100644
>> --- a/arch/x86/mm/fault.c
>> +++ b/arch/x86/mm/fault.c
>> @@ -1100,6 +1100,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma)
>>   				       (error_code & X86_PF_INSTR), foreign))
>>   		return 1;
>>   
>> +	/*
>> +	 * Verify a shadow stack access is within a shadow stack VMA.
>> +	 * It is always an error otherwise.  Normal data access to a
>> +	 * shadow stack area is checked in the case followed.
>> +	 */
>> +	if (error_code & X86_PF_SHSTK) {
>> +		if (!(vma->vm_flags & VM_SHSTK))
>> +			return 1;
>> +		return 0;
>
> Any reason to return 0 here? I would rather keep the single return 0 in
> the function, after all checks are done.
>

For shadow stack fault, X86_PF_SHSTK and X86_PF_WRITE both can be set. So for shadow stack fault, return from here and don't go into the normal write fault case.

Thanks,
Yu-cheng

>> +	}
>> +
>>   	if (error_code & X86_PF_WRITE) {
>>   		/* write, present and write, not present: */
>>   		if (unlikely(!(vma->vm_flags & VM_WRITE)))
>> @@ -1293,6 +1304,14 @@ void do_user_addr_fault(struct pt_regs *regs,
>>   
>>   	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
>>   
>> +	/*
>> +	 * Clearing _PAGE_DIRTY is used to detect shadow stack access.
>> +	 * This method cannot distinguish shadow stack read vs. write.
>> +	 * For valid shadow stack accesses, set FAULT_FLAG_WRITE to effect
>> +	 * copy-on-write.
>> +	 */
>> +	if (error_code & X86_PF_SHSTK)
>> +		flags |= FAULT_FLAG_WRITE;
>>   	if (error_code & X86_PF_WRITE)
>>   		flags |= FAULT_FLAG_WRITE;
>>   	if (error_code & X86_PF_INSTR)
>> -- 
>> 2.21.0
>>
>
```