lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 28 Jul 2021 14:52:40 +0800
From:   Wang Zi-cheng <wzc@...il.nju.edu.cn>
To:     keescook@...omium.org, tycho@...ho.pizza,
        linux-hardening@...r.kernel.org
Cc:     Wang Zi-cheng <wzc@...il.nju.edu.cn>
Subject: [PATCH] RFC v2 struct const ops pointers member hardening

Signed-off-by: Wang Zi-cheng <wzc@...il.nju.edu.cn>
--------------------
RFC v2 struct const ops pointers member hardening

I apologize for making mistakes in my previous submission, I fix the previous problems,
 add compile options and make tests(compile and lmbench).

1. this is a useful hardening, my opinion was wrong in the previous patch, 
because the attacker may overwrite a struct with an "struct file*" pointer,
 which point to a manufactured file struct with malicious f_op.
Hardening operation pointers CAN help.
> On the other side, kernel uses `kmem_cache_create` to alloc file/inode rather than `kmalloc`, 
> which makes it hard to exploit through heap overflow or UAF, so maybe this is not a "must" update.

2. V2 add compile option 'STRUCT_CONST_OPS_POINTER_HARDENING' in 
'Security options -> Kernel hardening options -> Struct const operation pointers hardening'

> PATCH v1
> some security sensitive kernel objects, i.e. inode, file, use 'const' to declare operations pointers, 
> and these pointers reference to the static global variables in read only data segment.

> ```
> struct file {
>   ...
> 	const struct file_operations	*f_op;
> 
> const struct file_operations ext4_file_operations = {
> ```

> However, const pointers are just compile hints with no hardware restrictions and prone to be modified by attakers at runtime.
> It would be safer to make sure struct const pointer members are not pointing to malicious payload in the slab objects(direct mapping region).
> we only need to check "open" syscall, because most file-related operations start with "open".


---
 arch/x86/include/asm/pgtable_64_types.h |  5 +++++
 fs/open.c                               |  5 +++++
 include/linux/mm.h                      |  9 +++++++++
 security/Kconfig.hardening              | 13 +++++++++++++
 4 files changed, 32 insertions(+)

diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
index 91ac10654570..869ef3dfaf29 100644
--- a/arch/x86/include/asm/pgtable_64_types.h
+++ b/arch/x86/include/asm/pgtable_64_types.h
@@ -128,18 +128,23 @@ extern unsigned int ptrs_per_p4d;
 
 #define __VMEMMAP_BASE_L4	0xffffea0000000000UL
 #define __VMEMMAP_BASE_L5	0xffd4000000000000UL
+#define DIRECT_MAPPING_TB_L4 64UL
+#define DIRECT_MAPPING_TB_L5 32000UL
 
 #ifdef CONFIG_DYNAMIC_MEMORY_LAYOUT
 # define VMALLOC_START		vmalloc_base
 # define VMALLOC_SIZE_TB	(pgtable_l5_enabled() ? VMALLOC_SIZE_TB_L5 : VMALLOC_SIZE_TB_L4)
 # define VMEMMAP_START		vmemmap_base
+# define DIRECT_MAPPING_SIZE_TB 	(pgtable_l5_enabled() ? DIRECT_MAPPING_TB_L5 : DIRECT_MAPPING_TB_L4)
 #else
 # define VMALLOC_START		__VMALLOC_BASE_L4
 # define VMALLOC_SIZE_TB	VMALLOC_SIZE_TB_L4
 # define VMEMMAP_START		__VMEMMAP_BASE_L4
+# define DIRECT_MAPPING_SIZE_TB		DIRECT_MAPPING_TB_L4
 #endif /* CONFIG_DYNAMIC_MEMORY_LAYOUT */
 
 #define VMALLOC_END		(VMALLOC_START + (VMALLOC_SIZE_TB << 40) - 1)
+#define DIRECT_MAPPING_END		(__PAGE_OFFSET + (DIRECT_MAPPING_SIZE_TB << 40) - 1)
 
 #define MODULES_VADDR		(__START_KERNEL_map + KERNEL_IMAGE_SIZE)
 /* The module sections ends with the start of the fixmap */
diff --git a/fs/open.c b/fs/open.c
index e53af13b5835..871750b57267 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -820,6 +820,11 @@ static int do_dentry_open(struct file *f,
 
 	/* normally all 3 are set; ->open() can clear them if needed */
 	f->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
+	/* check if f_op point to malicious payload */
+#ifdef CONFIG_STRUCT_CONST_OPS_POINTER_HARDENING
+	WARN_ON (!check_valid_ops_pointer((unsigned long) f->f_op));
+	WARN_ON (!check_valid_ops_pointer((unsigned long) f->f_inode->i_op));
+#endif
 	if (!open)
 		open = f->f_op->open;
 	if (open) {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8ae31622deef..c787acda7012 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3250,6 +3250,15 @@ static inline int seal_check_future_write(int seals, struct vm_area_struct *vma)
 
 	return 0;
 }
+#ifdef CONFIG_STRUCT_CONST_OPS_POINTER_HARDENING
+static inline bool check_valid_ops_pointer(unsigned long addr)
+{
+	if (addr >= __PAGE_OFFSET && addr <= DIRECT_MAPPING_END) 
+		return false;
+	else
+		return true;
+}
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index a56c36470cb1..3a7e2a046842 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -219,4 +219,17 @@ config INIT_ON_FREE_DEFAULT_ON
 
 endmenu
 
+config STRUCT_CONST_OPS_POINTER_HARDENING
+	depends on X86_64
+	bool "Struct const operation pointers hardening"
+	help
+		Security sensitive kernel objects, i.e. 'inode', 'file' protect 
+		indirect call pointers by declaring const operation pointers and 
+		making these pointers reference to static const global variables. 
+		However const members in the kernel object are just compile hints
+		with no hardware restriction. To hardening the operations pointers
+		in structs, put a check in "open syscall" and make sure pointers
+		are not pointing to the malicious payload in slub 
+		objects(direct mapping region).
+
 endmenu
-- 
2.32.0



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ