pramfs: memory protection

PATCH In review Marco Stornelli 2 participants 7 messages Oct 10, 2010 – Oct 12, 2010

Subsystems (3)

Memory management VFS MM core

lore

Share & tools

lore

Marco Stornelli

author

2010-10-10 16:36 UTC #0 lore raw

PATCH 1/1 pramfs: memory protection

From: Marco Stornelli <[email protected]>

Memory write protection.

Signed-off-by Marco Stornelli <[email protected]>

---

fs/pramfs/wprotect.c +31 -0

diff -Nurp linux-2.6.36-orig/fs/pramfs/wprotect.c linux-2.6.36/fs/pramfs/wprotect.c
@@ -0,0 +1,31 @@
+/*
+ * FILE NAME fs/pramfs/wprotect.c
+ *
+ * BRIEF DESCRIPTION
+ *
+ * Write protection for the filesystem pages.
+ *
+ * Copyright 2009-2010 Marco Stornelli <[email protected]>
+ * Copyright 2003 Sony Corporation
+ * Copyright 2003 Matsushita Electric Industrial Co., Ltd.
+ * 2003-2004 (c) MontaVista Software, Inc. , Steve Longerbeam
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/io.h>
+#include "pram.h"
+
+void pram_writeable(void *vaddr, unsigned long size, int rw)
+{
+	int ret = 0;
+
+	ret = rw ? write_on_kernel_pte_range((unsigned long)vaddr, size) :
+		    write_off_kernel_pte_range((unsigned long)vaddr, size);
+
+	BUG_ON(ret);
+}

include/linux/mm.h +5 -0

diff -Nurp linux-2.6.36-orig/include/linux/mm.h linux-2.6.36/include/linux/mm.h
@@ -811,6 +811,11 @@ int follow_phys(struct vm_area_struct *v
 		unsigned int flags, unsigned long *prot, resource_size_t *phys);
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write);
+int writeable_kernel_pte_range(unsigned long address, unsigned long size,
+				unsigned int rw);
+
+#define write_on_kernel_pte_range(addr, size) writeable_kernel_pte_range(addr, size, 1)
+#define write_off_kernel_pte_range(addr, size) writeable_kernel_pte_range(addr, size, 0)
  static inline void unmap_shared_mapping_range(struct address_space *mapping,
 		loff_t const holebegin, loff_t const holelen)

mm/memory.c +46 -0

49 lines not on this page · raw message

Andi Kleen

2010-10-10 16:46 UTC #1 lore raw

Quoted 7 lines · Marco Stornelli

mm/memory.c +7 -0

+
+	do {
+		pgd = pgd_offset(&init_mm, address);
+		if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+			goto out;
+
+		pud = pud_offset(pgd, address);

mm/memory.c void might_fault(void) · line 3621

Quoted 10 lines

mm/memory.c +10 -0

+		if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+			goto out;
+
+		pmd = pmd_offset(pud, address);
+		if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+			goto out;
+
+		ptep = pte_offset_kernel(pmd, addr);
+		pte = *ptep;
+		if (pte_present(pte)) {

This won't work at all on x86 because you don't handle large pages.

And it doesn't work on x86-64 because the first 2GB are double mapped (direct and kernel text mapping)

Thirdly I expect it won't either on architectures that map the direct mapping with special registers (like IA64 or MIPS)

I'm not sure this is very useful anyways. It doesn't protect against stray DMA and it doesn't protect against writes through broken user PTEs.

-Andi

Signature · 7 lines

[email protected] -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Marco Stornelli

author

2010-10-11 06:57 UTC #2 lore raw

2010/10/10 Andi Kleen <[email protected]>:

Quoted 13 lines

Quoted 12 lines · Marco Stornelli

mm/memory.c +12 -0

+
+     do {
+             pgd = pgd_offset(&init_mm, address);
+             if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+                     goto out;
+
+             pud = pud_offset(pgd, address);
+             if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+                     goto out;
+
+             pmd = pmd_offset(pud, address);
+             if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))

Quoted 8 lines

Quoted 5 lines
+ goto out;
+
+ ptep = pte_offset_kernel(pmd, addr);
+ pte = *ptep;
+ if (pte_present(pte)) {
This won't work at all on x86 because you don't handle large pages.

On x86 works because I tested. Maybe there's a particular configuration with large pages. Sincerly I'm only an "user", so if you/Linus or others want to change it or rewrite it, for me it's ok. The pte manipulation are a bit out of scope for a fs, so I let the things to the mm experts.

Marco Stornelli

author

2010-10-11 17:32 UTC #3 lore raw

In reply to Andi Kleen

Il 10/10/2010 18:46, Andi Kleen ha scritto:

Quoted 8 lines

This won't work at all on x86 because you don't handle large pages.
And it doesn't work on x86-64 because the first 2GB are double mapped (direct and kernel text mapping)
Thirdly I expect it won't either on architectures that map the direct mapping with special registers (like IA64 or MIPS)

Andi, what do you think to use the already implemented follow_pte instead?

int writeable_kernel_pte_range(unsigned long address, unsigned long size,
unsigned int rw)
{

	unsigned long addr = address & PAGE_MASK;
	unsigned long end = address + size;
	unsigned long start = addr;
	int ret = -EINVAL;
	pte_t *ptep, pte;
	spinlock_t *lock = &init_mm.page_table_lock;

	do {
		ret = follow_pte(&init_mm, addr, &ptep, &lock);
		if (ret)
			goto out;
		pte = *ptep;
		if (pte_present(pte)) {
			  pte = rw ? pte_mkwrite(pte) : pte_wrprotect(pte);
			  *ptep = pte;
		}
		pte_unmap_unlock(ptep, lock);
		addr += PAGE_SIZE;
	} while (addr && (addr < end));

	ret = 0;

out:

	flush_tlb_kernel_range(start, end);
	return ret;
}

Quoted 7 lines

I'm not sure this is very useful anyways. It doesn't protect against stray DMA and it doesn't protect against writes through broken user PTEs.
-Andi

It's a way to have more protection against kernel bug, for a in-memory fs can be important. However this option can be enabled/disabled at fs level.

Marco

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andi Kleen

2010-10-12 07:45 UTC #4 lore raw

Quoted 4 lines · Marco Stornelli

Il 10/10/2010 18:46, Andi Kleen ha scritto:
Quoted 3 lines
This won't work at all on x86 because you don't handle large pages.

Quoted 8 lines

Quoted 5 lines
And it doesn't work on x86-64 because the first 2GB are double mapped (direct and kernel text mapping)
Thirdly I expect it won't either on architectures that map the direct mapping with special registers (like IA64 or MIPS)
Andi, what do you think to use the already implemented follow_pte instead?

Has all the same problems. Really you need an per architecture function. Perhaps some architectures could use a common helper, but certainly not all.

x86 already has some infrastructure for this, but it currently has serious problems too (like not merging mappings on unmap) and is generally overdesigned ugly code.

-Andi

Marco Stornelli

author

2010-10-12 10:47 UTC #5 lore raw

2010/10/12 Andi Kleen <[email protected]>:

Quoted 11 lines · Marco Stornelli

Quoted 10 lines
Il 10/10/2010 18:46, Andi Kleen ha scritto:
Quoted 8 lines
This won't work at all on x86 because you don't handle large pages.
And it doesn't work on x86-64 because the first 2GB are double mapped (direct and kernel text mapping)
Thirdly I expect it won't either on architectures that map the direct mapping with special registers (like IA64 or MIPS)

Quoted 7 lines

Quoted 2 lines
Andi, what do you think to use the already implemented follow_pte instead?
Has all the same problems. Really you need an per architecture function. Perhaps some architectures could use a common helper, but certainly not all.

per-arch?! Wow. Mmm...maybe I have to change something at fs level to avoid that. An alternative could be to use the follow_pte solution but avoid the protection via Kconfig if the fs is used on some archs (ia64 or MIPS), with large pages and so on. An help of the kernel community to know all these particular cases is welcome.

Regards,

Marco

Andi Kleen

2010-10-12 11:56 UTC #6 lore raw

Quoted 5 lines

per-arch?! Wow. Mmm...maybe I have to change something at fs level to avoid that. An alternative could be to use the follow_pte solution but avoid the protection via Kconfig if the fs is used on some archs (ia64 or MIPS), with large pages and so on. An help of the kernel community to know all these particular cases is welcome.

It depends if the protection is a fundamental part of your design (but if it is I would argue that's broken because it's really not very good protection): If it's just an optional nice to have you can stub it out on architectures that don't support it.

-Andi

Signature · 7 lines

[email protected] -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>