From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752054AbbL1Lux (ORCPT ); Mon, 28 Dec 2015 06:50:53 -0500 Received: from mout.kundenserver.de ([212.227.17.10]:64852 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751584AbbL1Luu (ORCPT ); Mon, 28 Dec 2015 06:50:50 -0500 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Ard Biesheuvel , kernel-hardening@lists.openwall.com, will.deacon@arm.com, catalin.marinas@arm.com, mark.rutland@arm.com, leif.lindholm@linaro.org, keescook@chromium.org, linux-kernel@vger.kernel.org, bhupesh.sharma@freescale.com, stuart.yoder@freescale.com Subject: Re: [RFC PATCH 01/10] arm64: introduce KIMAGE_VADDR as the virtual base of the kernel region Date: Mon, 28 Dec 2015 12:50:03 +0100 Message-ID: <11539877.krH4qBJsp9@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1451301654-32019-2-git-send-email-ard.biesheuvel@linaro.org> References: <1451301654-32019-1-git-send-email-ard.biesheuvel@linaro.org> <1451301654-32019-2-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:18MqtAdWcBQUKyg1A5P9MMDlvCuElrz7qgJDJUODUcEWjcAVNO0 Pj4yklGbdCZbf1bp6zPpN13NQmRZb/rbrLSLWs+C9KOHiMkpY3LLXjaFbcpfqx9Mt1M4G1D 8MmfVDQD+S+B8c96QcnORHwf3hx1+a2xASFTcWAt0JFTKh8tZENZi9tuS2XmsKOj0/lHfvY X+Beg4VVZAvlzhQH1KXZg== X-UI-Out-Filterresults: notjunk:1;V01:K0:0sQYPZT5cQA=:9/yFpqx+h7KgJKLDwyUe3d jgL6sx8oW1q1Yb5MqKXj1aUeeMK9dUwpd/pYNqoW2z9t9qy0SiJMgKs5donJEkfM3OQ5ndEHy K9konN+nMRzqddY0JE0HqeHC1oBHZTs4Agd9kJiNOhYBUwhG6immXF8kMsgjP/20xsgkl7nnS fZVwuZqBTqKtZ95JOwhhUHaz+AOfJC5pnIAf01wR8ZW6LMRmxVJVhm7xM8717DQ44swjU0Dh6 qL5WkL/JV4pf4DRMQRZ0s5Kc9ilwsDaPpL8ypLE3D20tPFqlmTQ4T+P5vo3L9I/fphKdJwKuH auoW5h0EvLRzHkv4qTGF7sH3bubStjgnhSzJHTADMwdjPWbg7e/GPCAM7fie58apn8xujyfn3 h1Q9pXm5apXlHBX8KRDR8afu6U5Vgmnl8Bt6E39rqOwNZPTZpaUdcJXDZ9h/5SYnXmQyYyaVr lNEXxmV46c3VPjrsyPoisIR56/XQ4D4IEoU8fV0Wuu+g55/wpJGYJw0owQD+lyvg2dHM7JIKI EjfokZmYTYWvfFtbSGTdQ2Jxw+Wfl7EX+k20LhNSbuzCzPpQ0yPeq6b5ZsbEJdsTGDPPEOFbQ u0tvZlAGXcWiD49ayk35y/Mq+qjuNcBGRL7ced9wQAM5oK7suJI/a7Kx6jnYSbJAEOnAlcl3N riBmV3GiauleX9dhWgXBTTrAVdVkzI72LCUNz2PnHZ1fpeOP+ExHh7T1J+ER5ysKxVK3SOTD4 LdbVBiFzaemm6sMu Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 28 December 2015 12:20:45 Ard Biesheuvel wrote: > @@ -75,8 +76,13 @@ > * private definitions which should NOT be used outside memory.h > * files. Use virt_to_phys/phys_to_virt/__pa/__va instead. > */ > -#define __virt_to_phys(x) (((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET)) > +#define __virt_to_phys(x) ({ \ > + phys_addr_t __x = (phys_addr_t)(x); \ > + __x >= PAGE_OFFSET ? (__x - PAGE_OFFSET + PHYS_OFFSET) : \ > + (__x - KIMAGE_VADDR + PHYS_OFFSET); }) > + > #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET)) > +#define __phys_to_kimg(x) ((unsigned long)((x) - PHYS_OFFSET + KIMAGE_VADDR)) Having a conditional here is a bit unfortunate. IIRC KASLR means something different depending on the architecture, we either randomize the physical address, or the virtual address, or both, and that addresses different attack scenarios. You seem to leave the physical address unchanged, which means that an attacker that has gained access to a DMA master device can potentially still modify the kernel without knowing the virtual address. Similarly, you seem to leave the kernel mapped at the original virtual address and just add a second map (or your __phys_to_virt is wrong), so if someone has the ability to access a kernel virtual memory address from user space, they also don't need the relocated address because they can potentially access the kernel .text and .data through the linear mapping. How about a different approach that keeps the relocatable kernel, but moves it in physical memory with the same random offset as the virtual address? That way, both would be random, and you can keep the simple virt_to_phys() function. I suppose the downside of that is that the number of random bits is then limited by the size of the first memblock, which is smaller than the vmalloc area. Arnd