From: Catalin Marinas <catalin.marinas@arm.com> To: Steven Price <steven.price@arm.com> Cc: "Qun-wei Lin (林群崴)" <Qun-wei.Lin@mediatek.com>, "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>, "Chinwen Chang (張錦文)" <chinwen.chang@mediatek.com>, "kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>, "Kuan-Ying Lee (李冠穎)" <Kuan-Ying.Lee@mediatek.com>, "Casper Li (李中榮)" <casper.li@mediatek.com>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> Subject: Re: [BUG] Usersapce MTE error with allocation tag 0 when low on memory Date: Thu, 30 Mar 2023 18:36:08 +0100 [thread overview] Message-ID: <ZCXIiCtjFt19wBAM@arm.com> (raw) In-Reply-To: <f468f934-40b6-3547-d3ea-88a0aac5bd6a@arm.com> On Thu, Mar 30, 2023 at 02:56:50PM +0100, Steven Price wrote: > > On Wed, Mar 29, 2023 at 02:55:49AM +0000, Qun-wei Lin (林群崴) wrote: > >> 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(). SPARC has a potential race here since the page is made visible to the user but the tags are not restored yet (I raised this before). But even ignoring this small window, arch_do_swap() needs to have the metadata available. > >> 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(). I think we should move arch_do_swap_page() earlier before swap_free() and in arm64 we copy the tags to pte_page(pte). I don't think SPARC would have any issues with this change (and it also fixes their race). -- Catalin
WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com> To: Steven Price <steven.price@arm.com> Cc: "Qun-wei Lin (林群崴)" <Qun-wei.Lin@mediatek.com>, "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>, "Chinwen Chang (張錦文)" <chinwen.chang@mediatek.com>, "kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>, "Kuan-Ying Lee (李冠穎)" <Kuan-Ying.Lee@mediatek.com>, "Casper Li (李中榮)" <casper.li@mediatek.com>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> Subject: Re: [BUG] Usersapce MTE error with allocation tag 0 when low on memory Date: Thu, 30 Mar 2023 18:36:08 +0100 [thread overview] Message-ID: <ZCXIiCtjFt19wBAM@arm.com> (raw) In-Reply-To: <f468f934-40b6-3547-d3ea-88a0aac5bd6a@arm.com> On Thu, Mar 30, 2023 at 02:56:50PM +0100, Steven Price wrote: > > On Wed, Mar 29, 2023 at 02:55:49AM +0000, Qun-wei Lin (林群崴) wrote: > >> 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(). SPARC has a potential race here since the page is made visible to the user but the tags are not restored yet (I raised this before). But even ignoring this small window, arch_do_swap() needs to have the metadata available. > >> 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(). I think we should move arch_do_swap_page() earlier before swap_free() and in arm64 we copy the tags to pte_page(pte). I don't think SPARC would have any issues with this change (and it also fixes their race). -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-03-30 17:36 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-29 2:55 [BUG] Usersapce MTE error with allocation tag 0 when low on memory Qun-wei Lin (林群崴) 2023-03-29 2:55 ` Qun-wei Lin (林群崴) 2023-03-29 2:55 ` Qun-wei Lin (林群崴) 2023-03-29 15:59 ` Andrey Konovalov 2023-03-29 15:59 ` Andrey Konovalov 2023-03-29 16:54 ` Catalin Marinas 2023-03-29 16:54 ` Catalin Marinas 2023-03-30 13:56 ` Steven Price 2023-03-30 13:56 ` Steven Price 2023-03-30 17:36 ` Catalin Marinas [this message] 2023-03-30 17:36 ` Catalin Marinas
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZCXIiCtjFt19wBAM@arm.com \ --to=catalin.marinas@arm.com \ --cc=Kuan-Ying.Lee@mediatek.com \ --cc=Qun-wei.Lin@mediatek.com \ --cc=casper.li@mediatek.com \ --cc=chinwen.chang@mediatek.com \ --cc=david@redhat.com \ --cc=gregkh@linuxfoundation.org \ --cc=kasan-dev@googlegroups.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=steven.price@arm.com \ --cc=surenb@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.