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@xxxxxxxxxx/ 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
