From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga03.intel.com ([134.134.136.65]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1fBTae-0001JX-OA for speck@linutronix.de; Thu, 26 Apr 2018 01:12:45 +0200 Date: Wed, 25 Apr 2018 16:12:38 -0700 From: Andi Kleen Subject: [MODERATED] Re: [PATCH 1/6] Patch 1 Message-ID: <20180425231238.GO14273@tassilo.jf.intel.com> References: <20180425160654.GH14273@tassilo.jf.intel.com> <20180425173619.GJ14273@tassilo.jf.intel.com> <20180425181152.GK14273@tassilo.jf.intel.com> <20180425185140.GL14273@tassilo.jf.intel.com> <20180425211901.GN14273@tassilo.jf.intel.com> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: > So first off, split that up, and do the TRIVIAL TWO-LINER that was already > posted (by Hocko?) for swap entry mitigation. Nothing else. I haven't see Hocko's patch, but I assume it just inverts the complete swap entry except P while it is stored. Two more corner cases: - How about the case when you have more than MAX_PA/2 physical memory? For this case we would need a flag to disable the invert (and clear the reporting), otherwise it will be attackable. Frankly this is fairly unlikely because usually systems have far more MAX_PA than DIMM slots, except possibly for SGI UV. It would be still better to handle it for that case. - And how about someone adding a swap file that is larger than MAX_PA/2? This would be also attackable with the invert approach. I can readd a check to swapon for that. My patch kit also made sure that swapon() with a too large file gets rejected, otherwise it would be attackable if inverted. This also needs to be readded. This will require checking phys_bits again. Is that ok for you? > > Then, start looking at PROT_NONE. There are two cases: > > - actual real file mappings > > We can use the same trick for PROT_NONE as we did for swap entries: > just invert all the high bits. None of this "how many bits do I have". Ok. > > - the /dev/mem kind of mappings (not necessarily through /dev/mem, they > may well be remap_pfn_range() other ways > > These are always shared. It's not entirely clear that anybody even uses > PROT_NONE on them. We may be able to just disallow PROT_NONE entirely, > or we can just zap the mapping instead of trying to "save" it, since > there really isn't anythign to save. Ok. So disallow PROT_NONE until someone complains. I assume we want to disallow PROT_NONE for any VM_PFNMAP vma. I don't think it makes sense for any other shared mapping? BTW there's also the case that if you have a device driver that does not use lazy faulting, but remap_pfn_range in the ->mmap function and the mmap is called with PROT_NONE, and it remaps memory, not MMIO. I still need to audit the tree to see if that can happen. It may be safer to forbid that case somewhere in the VM also. -Andi