[PATCH v2 0/4] kasan: Fix ordering between MTE tag colouring and page->flags

[Catalin Marinas]

On Fri, Feb 10, 2023 at 11:03:45AM -0800, Peter Collingbourne wrote:
> On Fri, Feb 10, 2023 at 10:28 AM Catalin Marinas
> <catalin.marinas at arm.com> wrote:
> > On Thu, Feb 09, 2023 at 10:19:20PM -0800, Peter Collingbourne wrote:
> > > Thanks for the information. We encountered a similar issue internally
> > > with the Android 5.15 common kernel. We tracked it down to an issue
> > > with page migration, where the source page was a userspace page with
> > > MTE tags, and the target page was allocated using KASAN (i.e. having
> > > a non-zero KASAN tag). This caused tag check faults when the page was
> > > subsequently accessed by the kernel as a result of the mismatching tags
> > > from userspace. Given the number of different ways that page migration
> > > target pages can be allocated, the simplest fix that we could think of
> > > was to synchronize the KASAN tag in copy_highpage().
> > >
> > > Can you try the patch below and let us know whether it fixes the issue?
> > >
> > > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > > index 24913271e898c..87ed38e9747bd 100644
> > > --- a/arch/arm64/mm/copypage.c
> > > +++ b/arch/arm64/mm/copypage.c
> > > @@ -23,6 +23,8 @@ void copy_highpage(struct page *to, struct page *from)
> > >
> > >       if (system_supports_mte() && test_bit(PG_mte_tagged, &from->flags)) {
> > >               set_bit(PG_mte_tagged, &to->flags);
> > > +             if (kasan_hw_tags_enabled())
> > > +                     page_kasan_tag_set(to, page_kasan_tag(from));
> > >               mte_copy_page_tags(kto, kfrom);
> >
> > Why not just page_kasan_tag_reset(to)? If PG_mte_tagged is set on the
> > 'from' page, the tags are random anyway and page_kasan_tag(from) should
> > already be 0xff. It makes more sense to do the same for the 'to' page
> > rather than copying the tag from the 'from' page. IOW, we are copying
> > user-controlled tags into a page, the kernel should have a match-all tag
> > in page->flags.
> 
> That would also work, but I was thinking that if copy_highpage() were
> being used to copy a KASAN page we should keep the original tag in
> order to maintain tag checks for page accesses.

If PG_mte_tagged is set on the source, it means that the tags are no
longer trusted and we should reset to match-all. Otherwise if
copy_highpage() is called on a page that was never mapped as PROT_MTE in
user space, PG_mte_tagged would not be set on the source and no tags
copied. In such case, we should keep the original KASAN tag in the
destination. Unless I misunderstood what you meant.

-- 
Catalin