Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: KXvY1rhjTLmFjc4ha9vowdTixgoN79Tt4P5vIld0lpQVPMDYqv3MkzFASO7fgySbCqy2OTt5uU
 HvJ3dke+obUdib0jpHM7YgshgxClYQcfKmbQx/4cg4SWJD1cLAFPPr79U43ch7YjqX9QcE+wC+
 pSLLggM9ntfPzSa86FVMYlc5y3af8F6bJqkfi+ioRdLOcKIvl1+2x8EbsJ4rDrRxVR4DYyP3ep
 BvJsRnvPYRIBXCjpqKmklz5dxNZK/Ph2r9VyQ2ftLEixYVeE8mqLFKv0lBhsi3LorVDJW9FLsm
 KwY=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Palmer Dabbelt <palmer@dabbelt.com>,
        "zong.li@sifive.com" <zong.li@sifive.com>
CC:     "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "mhiramat@kernel.org" <mhiramat@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>
Subject: Re: [PATCH v5 9/9] riscv: add STRICT_KERNEL_RWX support
Thread-Topic: [PATCH v5 9/9] riscv: add STRICT_KERNEL_RWX support
Thread-Index: AQHWDXtlba0LR/X2T0mj7jnY6FovDQ==
Date:   Sun, 19 Apr 2020 23:50:53 +0000
Message-ID: <BY5PR04MB69001DE1B20907D2BA8F0BEDE7D70@BY5PR04MB6900.namprd04.prod.outlook.com>
References: <mhng-09f91ec8-5821-41ad-a743-3842ca10f9e2@palmerdabbelt-glaptop1>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: ddc42d53-3916-4e2c-9425-08d7e4bc7f47
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Apr 2020 23:50:53.4396
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: pfOJBgEnVnoFgvDrdXQbNGxCxzJBoG152NvcWtgEuye8QaxKXqeN9vClxbL+5froR+04aV7+Rp5tx1Jr7yppTQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR04MB6932
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/04/18 9:30, Palmer Dabbelt wrote:=0A=
> On Wed, 08 Apr 2020 00:57:04 PDT (-0700), zong.li@sifive.com wrote:=0A=
>> The commit contains that make text section as non-writable, rodata=0A=
>> section as read-only, and data section as non-executable.=0A=
>>=0A=
>> The init section should be changed to non-executable.=0A=
>>=0A=
>> Signed-off-by: Zong Li <zong.li@sifive.com>=0A=
>> ---=0A=
>>  arch/riscv/Kconfig                  |  1 +=0A=
>>  arch/riscv/include/asm/set_memory.h |  8 ++++++=0A=
>>  arch/riscv/mm/init.c                | 44 +++++++++++++++++++++++++++++=
=0A=
>>  3 files changed, 53 insertions(+)=0A=
>>=0A=
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig=0A=
>> index 1e1efc998baf..58b556167d59 100644=0A=
>> --- a/arch/riscv/Kconfig=0A=
>> +++ b/arch/riscv/Kconfig=0A=
>> @@ -61,6 +61,7 @@ config RISCV=0A=
>>  	select ARCH_HAS_GIGANTIC_PAGE=0A=
>>  	select ARCH_HAS_SET_DIRECT_MAP=0A=
>>  	select ARCH_HAS_SET_MEMORY=0A=
>> +	select ARCH_HAS_STRICT_KERNEL_RWX=0A=
=0A=
This should be:=0A=
=0A=
	select ARCH_HAS_STRICT_KERNEL_RWX if !MMU=0A=
=0A=
This option does not make sense for the no MMU case and more importantly, i=
t=0A=
ends up generating gigantic binary images which breaks things like the K210=
 support.=0A=
=0A=
Palmer,=0A=
=0A=
I sent a patch to fix that. You did not reply/comment on it.=0A=
=0A=
=0A=
>>  	select ARCH_WANT_HUGE_PMD_SHARE if 64BIT=0A=
>>  	select SPARSEMEM_STATIC if 32BIT=0A=
>>  	select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU=0A=
>> diff --git a/arch/riscv/include/asm/set_memory.h b/arch/riscv/include/as=
m/set_memory.h=0A=
>> index 4c5bae7ca01c..c38df4771c09 100644=0A=
>> --- a/arch/riscv/include/asm/set_memory.h=0A=
>> +++ b/arch/riscv/include/asm/set_memory.h=0A=
>> @@ -22,6 +22,14 @@ static inline int set_memory_x(unsigned long addr, in=
t numpages) { return 0; }=0A=
>>  static inline int set_memory_nx(unsigned long addr, int numpages) { ret=
urn 0; }=0A=
>>  #endif=0A=
>>=0A=
>> +#ifdef CONFIG_STRICT_KERNEL_RWX=0A=
>> +void set_kernel_text_ro(void);=0A=
>> +void set_kernel_text_rw(void);=0A=
>> +#else=0A=
>> +static inline void set_kernel_text_ro(void) { }=0A=
>> +static inline void set_kernel_text_rw(void) { }=0A=
>> +#endif=0A=
>> +=0A=
>>  int set_direct_map_invalid_noflush(struct page *page);=0A=
>>  int set_direct_map_default_noflush(struct page *page);=0A=
>>=0A=
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c=0A=
>> index fab855963c73..b55be44ff9bd 100644=0A=
>> --- a/arch/riscv/mm/init.c=0A=
>> +++ b/arch/riscv/mm/init.c=0A=
>> @@ -12,6 +12,7 @@=0A=
>>  #include <linux/sizes.h>=0A=
>>  #include <linux/of_fdt.h>=0A=
>>  #include <linux/libfdt.h>=0A=
>> +#include <linux/set_memory.h>=0A=
>>=0A=
>>  #include <asm/fixmap.h>=0A=
>>  #include <asm/tlbflush.h>=0A=
>> @@ -477,6 +478,17 @@ static void __init setup_vm_final(void)=0A=
>>  	csr_write(CSR_SATP, PFN_DOWN(__pa_symbol(swapper_pg_dir)) | SATP_MODE)=
;=0A=
>>  	local_flush_tlb_all();=0A=
>>  }=0A=
>> +=0A=
>> +void free_initmem(void)=0A=
>> +{=0A=
>> +	unsigned long init_begin =3D (unsigned long)__init_begin;=0A=
>> +	unsigned long init_end =3D (unsigned long)__init_end;=0A=
>> +=0A=
>> +	/* Make the region as non-execuatble. */=0A=
>> +	set_memory_nx(init_begin, (init_end - init_begin) >> PAGE_SHIFT);=0A=
>> +	free_initmem_default(POISON_FREE_INITMEM);=0A=
>> +}=0A=
>> +=0A=
>>  #else=0A=
>>  asmlinkage void __init setup_vm(uintptr_t dtb_pa)=0A=
>>  {=0A=
>> @@ -488,6 +500,38 @@ static inline void setup_vm_final(void)=0A=
>>  }=0A=
>>  #endif /* CONFIG_MMU */=0A=
>>=0A=
>> +#ifdef CONFIG_STRICT_KERNEL_RWX=0A=
>> +void set_kernel_text_rw(void)=0A=
>> +{=0A=
>> +	unsigned long text_start =3D (unsigned long)_text;=0A=
>> +	unsigned long text_end =3D (unsigned long)_etext;=0A=
>> +=0A=
>> +	set_memory_rw(text_start, (text_end - text_start) >> PAGE_SHIFT);=0A=
>> +}=0A=
>> +=0A=
>> +void set_kernel_text_ro(void)=0A=
>> +{=0A=
>> +	unsigned long text_start =3D (unsigned long)_text;=0A=
>> +	unsigned long text_end =3D (unsigned long)_etext;=0A=
>> +=0A=
>> +	set_memory_ro(text_start, (text_end - text_start) >> PAGE_SHIFT);=0A=
>> +}=0A=
>> +=0A=
>> +void mark_rodata_ro(void)=0A=
>> +{=0A=
>> +	unsigned long text_start =3D (unsigned long)_text;=0A=
>> +	unsigned long text_end =3D (unsigned long)_etext;=0A=
>> +	unsigned long rodata_start =3D (unsigned long)__start_rodata;=0A=
>> +	unsigned long data_start =3D (unsigned long)_data;=0A=
>> +	unsigned long max_low =3D (unsigned long)(__va(PFN_PHYS(max_low_pfn)))=
;=0A=
>> +=0A=
>> +	set_memory_ro(text_start, (text_end - text_start) >> PAGE_SHIFT);=0A=
>> +	set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT)=
;=0A=
>> +	set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_SHIFT)=
;=0A=
>> +	set_memory_nx(data_start, (max_low - data_start) >> PAGE_SHIFT);=0A=
>> +}=0A=
>> +#endif=0A=
>> +=0A=
>>  void __init paging_init(void)=0A=
>>  {=0A=
>>  	setup_vm_final();=0A=
> =0A=
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>=0A=
> =0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=