From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [RFC PATCH v9 03/13] mm: Add support for eXclusive Page Frame Ownership (XPFO) Date: Thu, 4 Apr 2019 19:01:39 +0200 Message-ID: <20190404170139.GA4038@hirez.programming.kicks-ass.net> References: <20190404074323.GO4038@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Khalid Aziz Cc: catalin.marinas-5wv7dgnIgG8@public.gmane.org, will.deacon-5wv7dgnIgG8@public.gmane.org, steven.sistare-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, hch-jcswGhMUV9g@public.gmane.org, tycho-E0fblnxP3wo@public.gmane.org, ak-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, aneesh.kumar-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org, jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org, rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, anthony.yznaga-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, riel-ebMLmSuQjDVBDgjK7y7TUQ@public.gmane.org, npiggin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org, luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, jrdr.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, jkosina-AlSwsSmVLrQ@public.gmane.org, joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org, arunks-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, dwmw-vV1OtcyAfmbQXOPxS62xeg@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, keith.busch-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, joao.m.martins-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org, brgl-ARrdPY/1zhM@public.gmane.org, konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, jsteckli-ebkRAfMGSJGzQB+pC5nmwQ@public.gmane.org, jroedel-l3A5Bk7waGM@public.gmane.org, keescook-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, zhangshaokun-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, chris.hyser-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Khalid Aziz List-Id: iommu@lists.linux-foundation.org On Thu, Apr 04, 2019 at 09:15:46AM -0600, Khalid Aziz wrote: > Thanks Peter. I really appreciate your review. Your feedback helps make > this code better and closer to where I can feel comfortable not calling > it RFC any more. > > The more I look at xpfo_kmap()/xpfo_kunmap() code, the more I get > uncomfortable with it. As you pointed out about calling kmap_atomic from > NMI context, that just makes the kmap_atomic code look even worse. I > pointed out one problem with this code in cover letter and suggested a > rewrite. I see these problems with this code: Well, I no longer use it from NMI context, but I did do that for a while. We now have a giant heap of magic in the NMI path that allows us to take faults from NMI context (please don't ask), this means we can mostly do copy_from_user_inatomic() now. > 1. When xpfo_kmap maps a page back in physmap, it opens up the ret2dir > attack security hole again even if just for the duration of kmap. A kmap > can stay around for some time if the page is being used for I/O. Correct. > 2. This code uses spinlock which leads to problems. If it does not > disable IRQ, it is exposed to deadlock around xpfo_lock. If it disables > IRQ, I think it can still deadlock around pgd_lock. I've not spotted that inversion yet, but then I didn't look at the lock usage outside of k{,un}map_xpfo(). > I think a better implementation of xpfo_kmap()/xpfo_kunmap() would map > the page at a new virtual address similar to what kmap_high for i386 > does. This avoids re-opening the ret2dir security hole. We can also > possibly do away with xpfo_lock saving bytes in page-frame and the not > so sane code sequence can go away. Right, the TLB invalidation issues are still tricky, even there :/