Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f468f934-40b6-3547-d3ea-88a0aac5bd6a@arm.com>
Date:   Thu, 30 Mar 2023 14:56:50 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [BUG] Usersapce MTE error with allocation tag 0 when low on
 memory
To:     Catalin Marinas <catalin.marinas@arm.com>,
        =?UTF-8?B?UXVuLXdlaSBMaW4gKOael+e+pOW0tCk=?= 
        <Qun-wei.Lin@mediatek.com>
Cc:     "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "surenb@google.com" <surenb@google.com>,
        "david@redhat.com" <david@redhat.com>,
        =?UTF-8?B?Q2hpbndlbiBDaGFuZyAo5by16Yym5paHKQ==?= 
        <chinwen.chang@mediatek.com>,
        "kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>,
        =?UTF-8?B?S3Vhbi1ZaW5nIExlZSAo5p2O5Yag56mOKQ==?= 
        <Kuan-Ying.Lee@mediatek.com>,
        =?UTF-8?B?Q2FzcGVyIExpICjmnY7kuK3mpq4p?= <casper.li@mediatek.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
References: <5050805753ac469e8d727c797c2218a9d780d434.camel@mediatek.com>
 <ZCRtVW9Q0WOKEQVX@arm.com>
Content-Language: en-GB
From:   Steven Price <steven.price@arm.com>
In-Reply-To: <ZCRtVW9Q0WOKEQVX@arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 29/03/2023 17:54, Catalin Marinas wrote:
> + Steven Price who added the MTE swap support.
> 
> On Wed, Mar 29, 2023 at 02:55:49AM +0000, Qun-wei Lin (林群崴) wrote:

<snip>

>>
>> Having compared the differences between Kernel-5.15 and Kernel-6.1,
>> We found the order of swap_free() and set_pte_at() is changed in
>> do_swap_page().
>>
>> When fault in, do_swap_page() will call swap_free() first:
>> do_swap_page() -> swap_free() -> __swap_entry_free() ->
>> free_swap_slot() -> swapcache_free_entries() -> swap_entry_free() ->
>> swap_range_free() -> arch_swap_invalidate_page() ->
>> mte_invalidate_tags_area() ->  mte_invalidate_tags() -> xa_erase()
>>
>> and then call set_pte_at():
>> do_swap_page() -> set_pte_at() -> __set_pte_at() -> mte_sync_tags() ->
>> mte_sync_page_tags() -> mte_restore_tags() -> xa_load()
>>
>> This means that the swap slot is invalidated before pte mapping, and
>> this will cause the mte tag in XArray to be released before tag
>> restore.

This analysis looks correct to me. The MTE swap code works on the
assumption that the set_pte_at() will restore the tags to the page
before the swap entry is removed. The reordering which has happened
since has broken this assumption and as you observed can cause the tags
to be unavailable by the time set_pte_at() is called.

>> After I moved swap_free() to the next line of set_pte_at(), the problem
>> is disappeared.
>>
>> We suspect that the following patches, which have changed the order, do
>> not consider the mte tag restoring in page fault flow:
>> https://lore.kernel.org/all/20220131162940.210846-5-david@redhat.com/

I'm not sure I entirely follow the reasoning in this patch, so I'm not
sure whether it's safe to just move swap_free() down to below
set_pte_at() or if that reintroduces the information leak.

I also wonder if sparc has a similar issue as the arch_do_swap()
callback is located next to set_pte_at().

>> Any suggestion is appreciated.

The other possibility is to add a(nother) callback for MTE in
arch_do_swap() that calls mte_restore_tags() on the page before the
swap_free() call rather than depending on the hook in set_pte_at().

Steve