MTE false-positive with shared userspace/kernel mapping

[Catalin Marinas]

Hi Andrey,

On Thu, Jul 20, 2023 at 08:28:12PM +0200, Andrey Konovalov wrote:
> Syzbot reported an issue originating from the packet sockets code [1],
> but it seems to be an MTE false-positive with a shared
> userspace/kernel mapping.
> 
> The problem is that mmap_region calls arch_validate_flags to check
> VM_MTE_ALLOWED only after mapping memory for a non-anonymous mapping
> via call_mmap().

That was on purpose as we can have some specific mmap implementation
that can set VM_MTE_ALLOWED. We only do this currently for shmem_mmap().
But I haven't thought of the vm_insert_page() case.

> What happens in the reproducer [2] is:
> 
> 1. Userspace creates a packet socket and makes the kernel allocate the
> backing memory for a shared mapping via alloc_one_pg_vec_page.
> 2. Userspace calls mmap _with PROT_MTE_ on a packet socket file descriptor.
> 3. mmap code sets VM_MTE via calc_vm_prot_bits(), as PROT_MTE has been provided.
> 3. mmap code calls the packet socket mmap handler packet_mmap via
> call_mmap() (without checking VM_MTE_ALLOWED at this point).
> 4. Packet socket code uses vm_insert_page to map the memory allocated
> in step #1 to the userspace area.
> 5. arm64 code resets memory tags for the backing memory via
> vm_insert_page->...->__set_pte_at->mte_sync_tags(), as the memory is
> MT_NORMAL_TAGGED due to VM_MTE.
> 6. Only now the mmap code checks VM_MTE_ALLOWED via
> arch_validate_flags() and unmaps the area, but the memory tags have
> already been reset.
> 5. The packet socket code accesses the area through its tagged kernel
> address via __packet_get_status(), which leads to a tag mismatch.

Ah, so we end up rejecting the mmap() eventually but the damage was done
by clearing the tags on the kernel page via a brief set_pte_at(). I
assume the problem only triggers with kasan enabled, though even without
kasan, we shouldn't allow a set_pte_at(PROT_MTE) for a vma that does not
allow MTE.

> I'm not sure what would be the best fix here. Moving
> arch_validate_flags() before call_mmap() would be an option, but maybe
> you have a better suggestion.

This would break the shmem case (though not sure who's using that). Also
since many drivers do vm_flags_set() (unrelated to MTE), it makes more
sense for arch_validate_flags() to happen after call_mmap().

Not ideal but an easy fix is calling arch_validate_flags() in those
specific mmap functions that call vm_insert_page(). They create a
mapping before the core code had a chance to validate the flags. Unless
we find a different solution for shmem_mmap() so that we can move the
arch_validate_flags() earlier.

-- 
Catalin