Re: [PATCH 6/6] kselftest/arm64: Check mte tagged user address in kernel

On Wed, Sep 23, 2020 at 12:36:59PM +0530, Amit Kachhap wrote:

On 9/22/20 4:11 PM, Catalin Marinas wrote:

...

On Tue, Sep 01, 2020 at 02:57:19PM +0530, Amit Daniel Kachhap wrote:

...

+static int check_usermem_access_fault(int mem_type, int mode, int mapping) +{

• int fd, ret, i, err;
• char val = 'A';
• size_t len, read_len;
• void *ptr, *ptr_next;
• bool fault;
• len = 2 * page_sz;
• err = KSFT_FAIL;
• /*

• * Accessing user memory in kernel with invalid tag should fault in sync
  

• * mode but may not fault in async mode as per the implemented MTE
  

• * support in Arm64 kernel.
  

• */
  

• if (mode == MTE_ASYNC_ERR)

• fault = false;
  

• else

• fault = true;
  

• mte_switch_mode(mode, MTE_ALLOW_NON_ZERO_TAG);
• fd = create_temp_file();
• if (fd == -1)

• return KSFT_FAIL;
  

• for (i = 0; i < len; i++)

• write(fd, &val, sizeof(val));
  

• lseek(fd, 0, 0);
• ptr = mte_allocate_memory(len, mem_type, mapping, true);
• if (check_allocated_memory(ptr, len, mem_type, true) != KSFT_PASS) {

• close(fd);
  

• return KSFT_FAIL;
  

• }
• mte_initialize_current_context(mode, (uintptr_t)ptr, len);
• /* Copy from file into buffer with valid tag */
• read_len = read(fd, ptr, len);
• ret = errno;

My reading of the man page is that errno is set only if read() returns -1.

Yes. The checks should be optimized here.

It's not about optimisation but correctness. The errno man page states that errno is only relevant if the syscall returns -1. So it may potentially hold a stale value (e.g. EFAULT) in case of read() success but the check below fails anyway:

...

...

• mte_wait_after_trig();
• if ((cur_mte_cxt.fault_valid == true) || ret == EFAULT || read_len < len)

• goto usermem_acc_err;