Re: [PATCH v5 8/8] powerpc/mm: Disable set_memory() routines when strict RWX isn't enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Le 26/02/2020 à 07:35, Russell Currey a écrit :
There are a couple of reasons that the set_memory() functions are
problematic when STRICT_KERNEL_RWX isn't enabled:

  - The linear mapping is a different size and apply_to_page_range()
	may modify a giant section, breaking everything
  - patch_instruction() doesn't know to work around a page being marked
  	RO, and will subsequently crash

The latter can be replicated by building a kernel with the set_memory()
patches but with STRICT_KERNEL_RWX off and running ftracetest.

I agree with Andrew, those changes should go into patch 1.


Reported-by: Jordan Niethe <jniethe5@xxxxxxxxx>
Signed-off-by: Russell Currey <ruscur@xxxxxxxxxx>
---
v5: Apply to both set_memory_attr() and change_memory_attr()

See my comments of v4, additional comments below

v4: New

  arch/powerpc/mm/pageattr.c | 22 ++++++++++++++++------
  1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index ee6b5e3b7604..49b8e2e0581d 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -64,13 +64,18 @@ static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
int change_memory_attr(unsigned long addr, int numpages, long action)
  {
-	unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
-	unsigned long sz = numpages * PAGE_SIZE;
+	unsigned long start, size;
+
+	if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+		return 0;
if (!numpages)
  		return 0;
- return apply_to_page_range(&init_mm, start, sz, change_page_attr, (void *)action);
+	start = ALIGN_DOWN(addr, PAGE_SIZE);
+	size = numpages * PAGE_SIZE;
+
+	return apply_to_page_range(&init_mm, start, size, change_page_attr, (void *)action);

You don't need to move start and sz initialisation, neither you need to change the name of sz to size.

If you want to rename sz to size, do it in the initial patch, but take care of the length of the lines. IIRC I used a short name to have the line fit on a single line with no more than 90 chars.

Christophe

  }
/*
@@ -96,12 +101,17 @@ static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot)
  {
-	unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
-	unsigned long sz = numpages * PAGE_SIZE;
+	unsigned long start, size;
+
+	if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
+		return 0;
if (!numpages)
  		return 0;
- return apply_to_page_range(&init_mm, start, sz, set_page_attr,
+	start = ALIGN_DOWN(addr, PAGE_SIZE);
+	size = numpages * PAGE_SIZE;
+
+	return apply_to_page_range(&init_mm, start, size, set_page_attr,
  				   (void *)pgprot_val(prot));
  }




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux