diff mbox

[v5,2/2] extable: verify address is read-only

Message ID 20170406033550.32525-3-ewk@edkovsky.org (mailing list archive)
State New, archived
Headers show

Commit Message

Eddie Kovsky April 6, 2017, 3:35 a.m. UTC
Provide a mechanism to check if the address of a variable is
const or ro_after_init. It mimics the existing functions that test if an
address is inside the kernel's text section.

The idea is to prevent structures that are not read-only from being
passed to functions. Other functions inside the kernel could then use
this capability to verify that their arguments are read-only.

This implements the first half of a suggestion made by Kees Cook for
the Kernel Self Protection Project:

	- provide mechanism to check for ro_after_init memory areas, and
	reject structures not marked ro_after_init in vmbus_register()

Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Eddie Kovsky <ewk@edkovsky.org>
---
Changes in v5:
 - Replace __start_data_ro_after_init with __start_ro_after_init
 - Replace __end_data_ro_after_init with __end_ro_after_init
Changes in v4:
 - Rename function core_kernel_ro_data() to core_kernel_rodata().
Changes in v3:
 - Fix missing declaration of is_module_rodata_address()
---
 include/linux/kernel.h |  2 ++
 kernel/extable.c       | 29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

Comments

kernel test robot April 6, 2017, 5:20 p.m. UTC | #1
Hi Eddie,

[auto build test WARNING on next-20170330]
[cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eddie-Kovsky/module-verify-address-is-read-only/20170407-004322
config: i386-randconfig-x014-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/trace_clock.h:12:0,
                    from include/linux/ftrace.h:9,
                    from kernel/extable.c:18:
   kernel/extable.c: In function 'core_kernel_rodata':
   kernel/extable.c:169:29: error: '__start_ro_after_init' undeclared (first use in this function)
     if (addr >= (unsigned long)__start_ro_after_init &&
                                ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> kernel/extable.c:169:2: note: in expansion of macro 'if'
     if (addr >= (unsigned long)__start_ro_after_init &&
     ^~
   kernel/extable.c:169:29: note: each undeclared identifier is reported only once for each function it appears in
     if (addr >= (unsigned long)__start_ro_after_init &&
                                ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> kernel/extable.c:169:2: note: in expansion of macro 'if'
     if (addr >= (unsigned long)__start_ro_after_init &&
     ^~
   kernel/extable.c:170:28: error: '__end_ro_after_init' undeclared (first use in this function)
         addr < (unsigned long)__end_ro_after_init)
                               ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> kernel/extable.c:169:2: note: in expansion of macro 'if'
     if (addr >= (unsigned long)__start_ro_after_init &&
     ^~

vim +/if +169 kernel/extable.c

    12	    GNU General Public License for more details.
    13	
    14	    You should have received a copy of the GNU General Public License
    15	    along with this program; if not, write to the Free Software
    16	    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
    17	*/
  > 18	#include <linux/ftrace.h>
    19	#include <linux/memory.h>
    20	#include <linux/extable.h>
    21	#include <linux/module.h>
    22	#include <linux/mutex.h>
    23	#include <linux/init.h>
    24	#include <linux/kprobes.h>
    25	#include <linux/filter.h>
    26	
    27	#include <asm/sections.h>
    28	#include <linux/uaccess.h>
    29	
    30	/*
    31	 * mutex protecting text section modification (dynamic code patching).
    32	 * some users need to sleep (allocating memory...) while they hold this lock.
    33	 *
    34	 * NOT exported to modules - patching kernel text is a really delicate matter.
    35	 */
    36	DEFINE_MUTEX(text_mutex);
    37	
    38	extern struct exception_table_entry __start___ex_table[];
    39	extern struct exception_table_entry __stop___ex_table[];
    40	
    41	/* Cleared by build time tools if the table is already sorted. */
    42	u32 __initdata __visible main_extable_sort_needed = 1;
    43	
    44	/* Sort the kernel's built-in exception table */
    45	void __init sort_main_extable(void)
    46	{
    47		if (main_extable_sort_needed && __stop___ex_table > __start___ex_table) {
    48			pr_notice("Sorting __ex_table...\n");
    49			sort_extable(__start___ex_table, __stop___ex_table);
    50		}
    51	}
    52	
    53	/* Given an address, look for it in the exception tables. */
    54	const struct exception_table_entry *search_exception_tables(unsigned long addr)
    55	{
    56		const struct exception_table_entry *e;
    57	
    58		e = search_extable(__start___ex_table, __stop___ex_table-1, addr);
    59		if (!e)
    60			e = search_module_extables(addr);
    61		return e;
    62	}
    63	
    64	static inline int init_kernel_text(unsigned long addr)
    65	{
    66		if (addr >= (unsigned long)_sinittext &&
    67		    addr < (unsigned long)_einittext)
    68			return 1;
    69		return 0;
    70	}
    71	
    72	int core_kernel_text(unsigned long addr)
    73	{
    74		if (addr >= (unsigned long)_stext &&
    75		    addr < (unsigned long)_etext)
    76			return 1;
    77	
    78		if (system_state == SYSTEM_BOOTING &&
    79		    init_kernel_text(addr))
    80			return 1;
    81		return 0;
    82	}
    83	
    84	/**
    85	 * core_kernel_data - tell if addr points to kernel data
    86	 * @addr: address to test
    87	 *
    88	 * Returns true if @addr passed in is from the core kernel data
    89	 * section.
    90	 *
    91	 * Note: On some archs it may return true for core RODATA, and false
    92	 *  for others. But will always be true for core RW data.
    93	 */
    94	int core_kernel_data(unsigned long addr)
    95	{
    96		if (addr >= (unsigned long)_sdata &&
    97		    addr < (unsigned long)_edata)
    98			return 1;
    99		return 0;
   100	}
   101	
   102	int __kernel_text_address(unsigned long addr)
   103	{
   104		if (core_kernel_text(addr))
   105			return 1;
   106		if (is_module_text_address(addr))
   107			return 1;
   108		if (is_ftrace_trampoline(addr))
   109			return 1;
   110		if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
   111			return 1;
   112		if (is_bpf_text_address(addr))
   113			return 1;
   114		/*
   115		 * There might be init symbols in saved stacktraces.
   116		 * Give those symbols a chance to be printed in
   117		 * backtraces (such as lockdep traces).
   118		 *
   119		 * Since we are after the module-symbols check, there's
   120		 * no danger of address overlap:
   121		 */
   122		if (init_kernel_text(addr))
   123			return 1;
   124		return 0;
   125	}
   126	
   127	int kernel_text_address(unsigned long addr)
   128	{
   129		if (core_kernel_text(addr))
   130			return 1;
   131		if (is_module_text_address(addr))
   132			return 1;
   133		if (is_ftrace_trampoline(addr))
   134			return 1;
   135		if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
   136			return 1;
   137		if (is_bpf_text_address(addr))
   138			return 1;
   139		return 0;
   140	}
   141	
   142	/*
   143	 * On some architectures (PPC64, IA64) function pointers
   144	 * are actually only tokens to some data that then holds the
   145	 * real function address. As a result, to find if a function
   146	 * pointer is part of the kernel text, we need to do some
   147	 * special dereferencing first.
   148	 */
   149	int func_ptr_is_kernel_text(void *ptr)
   150	{
   151		unsigned long addr;
   152		addr = (unsigned long) dereference_function_descriptor(ptr);
   153		if (core_kernel_text(addr))
   154			return 1;
   155		return is_module_text_address(addr);
   156	}
   157	
   158	/**
   159	 * core_kernel_rodata - Verify address points to read-only section
   160	 * @addr: address to test
   161	 *
   162	 */
   163	int core_kernel_rodata(unsigned long addr)
   164	{
   165		if (addr >= (unsigned long)__start_rodata &&
   166		    addr < (unsigned long)__end_rodata)
   167			return 1;
   168	
 > 169		if (addr >= (unsigned long)__start_ro_after_init &&
   170		    addr < (unsigned long)__end_ro_after_init)
   171			return 1;
   172	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot April 6, 2017, 5:41 p.m. UTC | #2
Hi Eddie,

[auto build test ERROR on next-20170330]
[cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eddie-Kovsky/module-verify-address-is-read-only/20170407-004322
config: i386-randconfig-x010-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/extable.c: In function 'core_kernel_rodata':
>> kernel/extable.c:169:29: error: '__start_ro_after_init' undeclared (first use in this function)
     if (addr >= (unsigned long)__start_ro_after_init &&
                                ^~~~~~~~~~~~~~~~~~~~~
   kernel/extable.c:169:29: note: each undeclared identifier is reported only once for each function it appears in
>> kernel/extable.c:170:28: error: '__end_ro_after_init' undeclared (first use in this function)
         addr < (unsigned long)__end_ro_after_init)
                               ^~~~~~~~~~~~~~~~~~~

vim +/__start_ro_after_init +169 kernel/extable.c

   163	int core_kernel_rodata(unsigned long addr)
   164	{
   165		if (addr >= (unsigned long)__start_rodata &&
   166		    addr < (unsigned long)__end_rodata)
   167			return 1;
   168	
 > 169		if (addr >= (unsigned long)__start_ro_after_init &&
 > 170		    addr < (unsigned long)__end_ro_after_init)
   171			return 1;
   172	
   173		return 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Eddie Kovsky April 7, 2017, 7:29 p.m. UTC | #3
On 04/07/17, kbuild test robot wrote:
> Hi Eddie,
> 
> [auto build test ERROR on next-20170330]
> [cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc5]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Eddie-Kovsky/module-verify-address-is-read-only/20170407-004322
> config: i386-randconfig-x010-201714 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    kernel/extable.c: In function 'core_kernel_rodata':
> >> kernel/extable.c:169:29: error: '__start_ro_after_init' undeclared (first use in this function)
>      if (addr >= (unsigned long)__start_ro_after_init &&
>                                 ^~~~~~~~~~~~~~~~~~~~~
>    kernel/extable.c:169:29: note: each undeclared identifier is reported only once for each function it appears in
> >> kernel/extable.c:170:28: error: '__end_ro_after_init' undeclared (first use in this function)
>          addr < (unsigned long)__end_ro_after_init)
>                                ^~~~~~~~~~~~~~~~~~~
> 
> vim +/__start_ro_after_init +169 kernel/extable.c
> 
>    163	int core_kernel_rodata(unsigned long addr)
>    164	{
>    165		if (addr >= (unsigned long)__start_rodata &&
>    166		    addr < (unsigned long)__end_rodata)
>    167			return 1;
>    168	
>  > 169		if (addr >= (unsigned long)__start_ro_after_init &&
>  > 170		    addr < (unsigned long)__end_ro_after_init)
>    171			return 1;
>    172	
>    173		return 0;
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


This looks like a false alarm.

The test build is based on next-20170330. Kees' patch for the section
names [start|end]_ro_after_init didn't appear in next until 20170403.

I cannot reproduce the build error using this config on recent versions
of next. Am I missing something here?

Eddie
Kees Cook April 7, 2017, 8:45 p.m. UTC | #4
On Fri, Apr 7, 2017 at 12:29 PM, Eddie Kovsky <ewk@edkovsky.org> wrote:
> On 04/07/17, kbuild test robot wrote:
>> Hi Eddie,
>>
>> [auto build test ERROR on next-20170330]
>> [cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc5]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url:    https://github.com/0day-ci/linux/commits/Eddie-Kovsky/module-verify-address-is-read-only/20170407-004322
>> config: i386-randconfig-x010-201714 (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=i386
>>
>> All errors (new ones prefixed by >>):
>>
>>    kernel/extable.c: In function 'core_kernel_rodata':
>> >> kernel/extable.c:169:29: error: '__start_ro_after_init' undeclared (first use in this function)
>>      if (addr >= (unsigned long)__start_ro_after_init &&
>>                                 ^~~~~~~~~~~~~~~~~~~~~
>>    kernel/extable.c:169:29: note: each undeclared identifier is reported only once for each function it appears in
>> >> kernel/extable.c:170:28: error: '__end_ro_after_init' undeclared (first use in this function)
>>          addr < (unsigned long)__end_ro_after_init)
>>                                ^~~~~~~~~~~~~~~~~~~
>>
>> vim +/__start_ro_after_init +169 kernel/extable.c
>>
>>    163        int core_kernel_rodata(unsigned long addr)
>>    164        {
>>    165                if (addr >= (unsigned long)__start_rodata &&
>>    166                    addr < (unsigned long)__end_rodata)
>>    167                        return 1;
>>    168
>>  > 169                if (addr >= (unsigned long)__start_ro_after_init &&
>>  > 170                    addr < (unsigned long)__end_ro_after_init)
>>    171                        return 1;
>>    172
>>    173                return 0;
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
>
> This looks like a false alarm.
>
> The test build is based on next-20170330. Kees' patch for the section
> names [start|end]_ro_after_init didn't appear in next until 20170403.
>
> I cannot reproduce the build error using this config on recent versions
> of next. Am I missing something here?

I agree, this was built without the renaming from the latest -next trees.

-Kees
diff mbox

Patch

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c26dc3a8295..5748784ca209 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -444,6 +444,8 @@  extern int core_kernel_data(unsigned long addr);
 extern int __kernel_text_address(unsigned long addr);
 extern int kernel_text_address(unsigned long addr);
 extern int func_ptr_is_kernel_text(void *ptr);
+extern int core_kernel_rodata(unsigned long addr);
+extern int kernel_ro_address(unsigned long addr);
 
 unsigned long int_sqrt(unsigned long);
 
diff --git a/kernel/extable.c b/kernel/extable.c
index 2676d7f8baf6..18c5e4dbe0fc 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -154,3 +154,32 @@  int func_ptr_is_kernel_text(void *ptr)
 		return 1;
 	return is_module_text_address(addr);
 }
+
+/**
+ * core_kernel_rodata - Verify address points to read-only section
+ * @addr: address to test
+ *
+ */
+int core_kernel_rodata(unsigned long addr)
+{
+	if (addr >= (unsigned long)__start_rodata &&
+	    addr < (unsigned long)__end_rodata)
+		return 1;
+
+	if (addr >= (unsigned long)__start_ro_after_init &&
+	    addr < (unsigned long)__end_ro_after_init)
+		return 1;
+
+	return 0;
+}
+
+/* Verify that address is const or ro_after_init. */
+int kernel_ro_address(unsigned long addr)
+{
+	if (core_kernel_rodata(addr))
+		return 1;
+	if (is_module_rodata_address(addr))
+		return 1;
+
+	return 0;
+}