MTE false-positive with shared userspace/kernel mapping

MTE false-positive with shared userspace/kernel mapping

[Andrey Konovalov]

Hi Catalin,

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().

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.

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.

On a side note, I think the packet sockets code ought to use GFP_USER
and vmalloc_user for allocating the backing memory in
alloc_one_pg_vec_page, but that shouldn't make any difference to MTE.

Thanks!

[1] https://syzkaller.appspot.com/bug?extid=64b0f633159fde08e1f1
[2] https://syzkaller.appspot.com/text?tag=ReproC&x=17fd0aee280000

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: MTE false-positive with shared userspace/kernel mapping

[Andrey Konovalov]

On Fri, Aug 4, 2023 at 7:52 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> 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.

Just FTR: filed a KASAN bug to not forget about this issue:

https://bugzilla.kernel.org/show_bug.cgi?id=218295

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel