From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965248AbbHKOSv (ORCPT ); Tue, 11 Aug 2015 10:18:51 -0400 Received: from mail-wi0-f169.google.com ([209.85.212.169]:35983 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964890AbbHKOSu (ORCPT ); Tue, 11 Aug 2015 10:18:50 -0400 Date: Tue, 11 Aug 2015 16:18:45 +0200 From: Michal Hocko To: Suresh Siddha Cc: LKML , Jiri Kosina Subject: set_memory_rw on the kernel text Message-ID: <20150811141845.GF18998@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Subject, I was debugging an issue that the kernel text didn't get remapped RW after set_memory_rw and generated a #PF even though set_memory_rw returned with success (0). I am completely unfamiliar with the code but it become clear from the code inspection that static_protections() will drop _PAGE_RW from the protection flags with CONFIG_DEBUG_RODATA for the large mappings. try_preserve_large_page will then interpret this as no change is needed and return with 0 all the way up to the caller. I can see the point that set_memory_rw doesn't allow remapping after certain moment (kernel_set_to_readonly is non-zero) but the current semantic with returning success even though the operation was ignored is strange. Shouldn't the function return -EPERM instead? So that the caller doesn't try to write to the address and #PF? Something like a completely untested. diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c index 89af288ec674..c1fcb02f9662 100644 --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -540,6 +540,10 @@ try_preserve_large_page(pte_t *kpte, unsigned long address, cpa->pfn = pfn; new_prot = static_protections(req_prot, address, pfn); + if (pgprot_val(new_prot) ^ pgprot_val(req_prot)) { + do_split = -EPERM; + goto out_unlock; + } /* * We need to check the full range, whether I am also trying to understand why the semantic is different for 4k pages. I can see that pmd prot change might influence different sections in the same pmd range or something like that but why don't we simply split the pmd then and make the 4k page RW? -- Michal Hocko SUSE Labs