* [PATCH v5 0/2] provide check for ro_after_init memory sections @ 2017-04-06 3:35 Eddie Kovsky 2017-04-06 3:35 ` [PATCH v5 1/2] module: verify address is read-only Eddie Kovsky ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Eddie Kovsky @ 2017-04-06 3:35 UTC (permalink / raw) To: jeyu, rusty, keescook; +Cc: linux-kernel, kernel-hardening Provide a mechanism for other functions 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() http://www.openwall.com/lists/kernel-hardening/2017/02/04/1 The idea is to prevent structures (including modules) that are not read-only from being passed to functions. It builds upon the functions in kernel/extable.c that test if an address is in the text section. A build failure on the Blackfin architecture led to the discovery of an incomplete definition of the RO_DATA macro used in this series. The fixes are in linux-next: commit 906f2a51c941 ("mm: fix section name for .data..ro_after_init") commit 939897e2d736 ("vmlinux.lds: add missing VMLINUX_SYMBOL macros") The latest version of this series uses new symbols provided in these fixes. The series now cross compiles on Blackfin without errors. I have also test compiled this series on next-20170405 for x86. I have dropped the third patch that uses these features to check the arguments to vmbus_register() because the maintainers have not been receptive to using it. My goal right now is to get the API right. Eddie Kovsky (2): module: verify address is read-only extable: verify address is read-only include/linux/kernel.h | 2 ++ include/linux/module.h | 12 ++++++++++++ kernel/extable.c | 29 +++++++++++++++++++++++++++ kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) -- 2.12.2 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 1/2] module: verify address is read-only 2017-04-06 3:35 [PATCH v5 0/2] provide check for ro_after_init memory sections Eddie Kovsky @ 2017-04-06 3:35 ` Eddie Kovsky 2017-04-07 1:58 ` Jessica Yu 2017-04-06 3:35 ` [PATCH v5 2/2] extable: " Eddie Kovsky 2017-04-07 21:53 ` [PATCH v5 0/2] provide check for ro_after_init memory sections Kees Cook 2 siblings, 1 reply; 14+ messages in thread From: Eddie Kovsky @ 2017-04-06 3:35 UTC (permalink / raw) To: jeyu, rusty, keescook; +Cc: linux-kernel, kernel-hardening Implement a mechanism to check if a module's address is in the rodata or ro_after_init sections. It mimics the existing functions that test if an address is inside a module's text section. Functions that take a module as an argument will be able to verify that the module address is in a read-only section. The idea is to prevent structures (including modules) that are not read-only from being passed to functions. 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 v4: - Rename function __module_ro_address() to __module_rodata_address() - Move module_rodata_address() stub below is_module_address() - Minor comment fixes - Verify that mod is not NULL before setting up layout variables Changes in v3: - Change function name is_module_ro_address() to is_module_rodata_address(). - Improve comments on is_module_rodata_address(). - Add a !CONFIG_MODULES stub for is_module_rodata_address. - Correct and simplify the check for the read-only memory regions in the module address. --- include/linux/module.h | 12 ++++++++++++ kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/include/linux/module.h b/include/linux/module.h index 9ad68561d8c2..a3d17b081de3 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -492,7 +492,9 @@ static inline int module_is_live(struct module *mod) struct module *__module_text_address(unsigned long addr); struct module *__module_address(unsigned long addr); +struct module *__module_rodata_address(unsigned long addr); bool is_module_address(unsigned long addr); +bool is_module_rodata_address(unsigned long addr); bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr); bool is_module_percpu_address(unsigned long addr); bool is_module_text_address(unsigned long addr); @@ -646,6 +648,11 @@ static inline struct module *__module_address(unsigned long addr) return NULL; } +static inline struct module *__module_rodata_address(unsigned long addr) +{ + return NULL; +} + static inline struct module *__module_text_address(unsigned long addr) { return NULL; @@ -656,6 +663,11 @@ static inline bool is_module_address(unsigned long addr) return false; } +static inline bool is_module_rodata_address(unsigned long addr) +{ + return false; +} + static inline bool is_module_percpu_address(unsigned long addr) { return false; diff --git a/kernel/module.c b/kernel/module.c index f953df992a11..d5753210cf34 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -4296,6 +4296,59 @@ struct module *__module_text_address(unsigned long addr) } EXPORT_SYMBOL_GPL(__module_text_address); +/** + * is_module_rodata_address - is this address inside read-only module data? + * @addr: the address to check. + * + */ +bool is_module_rodata_address(unsigned long addr) +{ + bool ret; + + preempt_disable(); + ret = __module_rodata_address(addr) != NULL; + preempt_enable(); + + return ret; +} + +/* + * __module_rodata_address - get the module whose rodata/ro_after_init sections + * contain the given address. + * @addr: the address. + * + * Must be called with preempt disabled or module mutex held so that + * module doesn't get freed during this. + */ +struct module *__module_rodata_address(unsigned long addr) +{ + struct module *mod = __module_address(addr); + + /* + * Make sure module is within the read-only section. + * range(base + text_size, base + ro_after_init_size) + * encompasses both the rodata and ro_after_init regions. + * See comment above frob_text() for the layout diagram. + */ + if (mod) { + void *init_base = mod->init_layout.base; + unsigned int init_text_size = mod->init_layout.text_size; + unsigned int init_ro_after_init_size = mod->init_layout.ro_after_init_size; + + void *core_base = mod->core_layout.base; + unsigned int core_text_size = mod->core_layout.text_size; + unsigned int core_ro_after_init_size = mod->core_layout.ro_after_init_size; + + if (!within(addr, init_base + init_text_size, + init_ro_after_init_size - init_text_size) + && !within(addr, core_base + core_text_size, + core_ro_after_init_size - core_text_size)) + mod = NULL; + } + return mod; +} +EXPORT_SYMBOL_GPL(__module_rodata_address); + /* Don't grab lock, we're oopsing. */ void print_modules(void) { -- 2.12.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/2] module: verify address is read-only 2017-04-06 3:35 ` [PATCH v5 1/2] module: verify address is read-only Eddie Kovsky @ 2017-04-07 1:58 ` Jessica Yu 2017-04-07 20:46 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Jessica Yu @ 2017-04-07 1:58 UTC (permalink / raw) To: Eddie Kovsky; +Cc: rusty, keescook, linux-kernel, kernel-hardening +++ Eddie Kovsky [05/04/17 21:35 -0600]: >Implement a mechanism to check if a module's address is in >the rodata or ro_after_init sections. It mimics the existing functions >that test if an address is inside a module's text section. > >Functions that take a module as an argument will be able to verify that the >module address is in a read-only section. The idea is to prevent structures >(including modules) that are not read-only from being passed to functions. > >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> Acked-by: Jessica Yu <jeyu@redhat.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/2] module: verify address is read-only 2017-04-07 1:58 ` Jessica Yu @ 2017-04-07 20:46 ` Kees Cook 0 siblings, 0 replies; 14+ messages in thread From: Kees Cook @ 2017-04-07 20:46 UTC (permalink / raw) To: Jessica Yu; +Cc: Eddie Kovsky, Rusty Russell, LKML, kernel-hardening On Thu, Apr 6, 2017 at 6:58 PM, Jessica Yu <jeyu@redhat.com> wrote: > +++ Eddie Kovsky [05/04/17 21:35 -0600]: >> >> Implement a mechanism to check if a module's address is in >> the rodata or ro_after_init sections. It mimics the existing functions >> that test if an address is inside a module's text section. >> >> Functions that take a module as an argument will be able to verify that >> the >> module address is in a read-only section. The idea is to prevent >> structures >> (including modules) that are not read-only from being passed to functions. >> >> 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> > > > Acked-by: Jessica Yu <jeyu@redhat.com> Thanks! I'll either get these into my kspp tree or ask akpm to carry these since they depend on the renaming in his tree already. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/2] extable: verify address is read-only 2017-04-06 3:35 [PATCH v5 0/2] provide check for ro_after_init memory sections Eddie Kovsky 2017-04-06 3:35 ` [PATCH v5 1/2] module: verify address is read-only Eddie Kovsky @ 2017-04-06 3:35 ` Eddie Kovsky 2017-04-06 17:20 ` kbuild test robot 2017-04-06 17:41 ` kbuild test robot 2017-04-07 21:53 ` [PATCH v5 0/2] provide check for ro_after_init memory sections Kees Cook 2 siblings, 2 replies; 14+ messages in thread From: Eddie Kovsky @ 2017-04-06 3:35 UTC (permalink / raw) To: jeyu, rusty, keescook; +Cc: linux-kernel, kernel-hardening 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(+) 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; +} -- 2.12.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/2] extable: verify address is read-only 2017-04-06 3:35 ` [PATCH v5 2/2] extable: " Eddie Kovsky @ 2017-04-06 17:20 ` kbuild test robot 2017-04-06 17:41 ` kbuild test robot 1 sibling, 0 replies; 14+ messages in thread From: kbuild test robot @ 2017-04-06 17:20 UTC (permalink / raw) To: Eddie Kovsky Cc: kbuild-all, jeyu, rusty, keescook, linux-kernel, kernel-hardening [-- Attachment #1: Type: text/plain, Size: 7804 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 25874 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/2] extable: verify address is read-only 2017-04-06 3:35 ` [PATCH v5 2/2] extable: " Eddie Kovsky 2017-04-06 17:20 ` kbuild test robot @ 2017-04-06 17:41 ` kbuild test robot 2017-04-07 19:29 ` Eddie Kovsky 1 sibling, 1 reply; 14+ messages in thread From: kbuild test robot @ 2017-04-06 17:41 UTC (permalink / raw) To: Eddie Kovsky Cc: kbuild-all, jeyu, rusty, keescook, linux-kernel, kernel-hardening [-- Attachment #1: Type: text/plain, Size: 1741 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 27264 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/2] extable: verify address is read-only 2017-04-06 17:41 ` kbuild test robot @ 2017-04-07 19:29 ` Eddie Kovsky 2017-04-07 20:45 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Eddie Kovsky @ 2017-04-07 19:29 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, jeyu, rusty, keescook, linux-kernel, kernel-hardening 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 2/2] extable: verify address is read-only 2017-04-07 19:29 ` Eddie Kovsky @ 2017-04-07 20:45 ` Kees Cook 0 siblings, 0 replies; 14+ messages in thread From: Kees Cook @ 2017-04-07 20:45 UTC (permalink / raw) To: Eddie Kovsky Cc: kbuild test robot, kbuild-all, Jessica Yu, Rusty Russell, LKML, kernel-hardening 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 -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 0/2] provide check for ro_after_init memory sections 2017-04-06 3:35 [PATCH v5 0/2] provide check for ro_after_init memory sections Eddie Kovsky 2017-04-06 3:35 ` [PATCH v5 1/2] module: verify address is read-only Eddie Kovsky 2017-04-06 3:35 ` [PATCH v5 2/2] extable: " Eddie Kovsky @ 2017-04-07 21:53 ` Kees Cook 2017-04-07 22:12 ` Andrew Morton 2 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2017-04-07 21:53 UTC (permalink / raw) To: Andrew Morton Cc: Jessica Yu, Rusty Russell, LKML, kernel-hardening, Eddie Kovsky On Wed, Apr 5, 2017 at 8:35 PM, Eddie Kovsky <ewk@edkovsky.org> wrote: > Provide a mechanism for other functions 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() > > http://www.openwall.com/lists/kernel-hardening/2017/02/04/1 > > The idea is to prevent structures (including modules) that are not > read-only from being passed to functions. It builds upon the functions > in kernel/extable.c that test if an address is in the text section. > > A build failure on the Blackfin architecture led to the discovery of > an incomplete definition of the RO_DATA macro used in this series. The > fixes are in linux-next: > > commit 906f2a51c941 ("mm: fix section name for .data..ro_after_init") > > commit 939897e2d736 ("vmlinux.lds: add missing VMLINUX_SYMBOL macros") > > The latest version of this series uses new symbols provided in these > fixes. The series now cross compiles on Blackfin without errors. I have > also test compiled this series on next-20170405 for x86. > > I have dropped the third patch that uses these features to check the > arguments to vmbus_register() because the maintainers have not been > receptive to using it. My goal right now is to get the API right. > > Eddie Kovsky (2): > module: verify address is read-only > extable: verify address is read-only > > include/linux/kernel.h | 2 ++ > include/linux/module.h | 12 ++++++++++++ > kernel/extable.c | 29 +++++++++++++++++++++++++++ > kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 96 insertions(+) Andrew, do you have these in your mailbox (it went to lkml), or should I resend them directly to you? Since they depend on the __start_ro_after_init naming fixes in -mm, it seemed like it'd be best to carry these two patches there. If so, please consider them both: Acked-by: Kees Cook <keescook@chromium.org> (And, from the thread on the module patch, Jessica has Acked that one too.) Thanks! -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 0/2] provide check for ro_after_init memory sections 2017-04-07 21:53 ` [PATCH v5 0/2] provide check for ro_after_init memory sections Kees Cook @ 2017-04-07 22:12 ` Andrew Morton 2017-04-07 22:15 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Andrew Morton @ 2017-04-07 22:12 UTC (permalink / raw) To: Kees Cook; +Cc: Jessica Yu, Rusty Russell, LKML, kernel-hardening, Eddie Kovsky On Fri, 7 Apr 2017 14:53:23 -0700 Kees Cook <keescook@chromium.org> wrote: > > Eddie Kovsky (2): > > module: verify address is read-only > > extable: verify address is read-only > > > > include/linux/kernel.h | 2 ++ > > include/linux/module.h | 12 ++++++++++++ > > kernel/extable.c | 29 +++++++++++++++++++++++++++ > > kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 96 insertions(+) > > Andrew, do you have these in your mailbox (it went to lkml), or should > I resend them directly to you? Since they depend on the > __start_ro_after_init naming fixes in -mm, it seemed like it'd be best > to carry these two patches there. If so, please consider them both: > > Acked-by: Kees Cook <keescook@chromium.org> > > (And, from the thread on the module patch, Jessica has Acked that one too.) Well I grabbed them, but the patches don't actually do anything - they add interfaces with no users. What's the plan here? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 0/2] provide check for ro_after_init memory sections 2017-04-07 22:12 ` Andrew Morton @ 2017-04-07 22:15 ` Kees Cook 2017-04-07 22:23 ` Andrew Morton 0 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2017-04-07 22:15 UTC (permalink / raw) To: Andrew Morton Cc: Jessica Yu, Rusty Russell, LKML, kernel-hardening, Eddie Kovsky On Fri, Apr 7, 2017 at 3:12 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 7 Apr 2017 14:53:23 -0700 Kees Cook <keescook@chromium.org> wrote: > >> > Eddie Kovsky (2): >> > module: verify address is read-only >> > extable: verify address is read-only >> > >> > include/linux/kernel.h | 2 ++ >> > include/linux/module.h | 12 ++++++++++++ >> > kernel/extable.c | 29 +++++++++++++++++++++++++++ >> > kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> > 4 files changed, 96 insertions(+) >> >> Andrew, do you have these in your mailbox (it went to lkml), or should >> I resend them directly to you? Since they depend on the >> __start_ro_after_init naming fixes in -mm, it seemed like it'd be best >> to carry these two patches there. If so, please consider them both: >> >> Acked-by: Kees Cook <keescook@chromium.org> >> >> (And, from the thread on the module patch, Jessica has Acked that one too.) > > Well I grabbed them, but the patches don't actually do anything - they > add interfaces with no users. What's the plan here? I'd like to have a way for interfaces (especially the various *_register()) to be able to check that a structure is either const or __ro_after_init. My expectation is to add those and similar sanity-checks now that we can do so. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 0/2] provide check for ro_after_init memory sections 2017-04-07 22:15 ` Kees Cook @ 2017-04-07 22:23 ` Andrew Morton 2017-04-07 22:47 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Andrew Morton @ 2017-04-07 22:23 UTC (permalink / raw) To: Kees Cook; +Cc: Jessica Yu, Rusty Russell, LKML, kernel-hardening, Eddie Kovsky On Fri, 7 Apr 2017 15:15:36 -0700 Kees Cook <keescook@chromium.org> wrote: > On Fri, Apr 7, 2017 at 3:12 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > > On Fri, 7 Apr 2017 14:53:23 -0700 Kees Cook <keescook@chromium.org> wrote: > > > >> > Eddie Kovsky (2): > >> > module: verify address is read-only > >> > extable: verify address is read-only > >> > > >> > include/linux/kernel.h | 2 ++ > >> > include/linux/module.h | 12 ++++++++++++ > >> > kernel/extable.c | 29 +++++++++++++++++++++++++++ > >> > kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >> > 4 files changed, 96 insertions(+) > >> > >> Andrew, do you have these in your mailbox (it went to lkml), or should > >> I resend them directly to you? Since they depend on the > >> __start_ro_after_init naming fixes in -mm, it seemed like it'd be best > >> to carry these two patches there. If so, please consider them both: > >> > >> Acked-by: Kees Cook <keescook@chromium.org> > >> > >> (And, from the thread on the module patch, Jessica has Acked that one too.) > > > > Well I grabbed them, but the patches don't actually do anything - they > > add interfaces with no users. What's the plan here? > > I'd like to have a way for interfaces (especially the various > *_register()) to be able to check that a structure is either const or > __ro_after_init. My expectation is to add those and similar > sanity-checks now that we can do so. OK. But I'd rather sit on the patches until we have working, tested, reviewed callers which are agreed to be useful. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 0/2] provide check for ro_after_init memory sections 2017-04-07 22:23 ` Andrew Morton @ 2017-04-07 22:47 ` Kees Cook 0 siblings, 0 replies; 14+ messages in thread From: Kees Cook @ 2017-04-07 22:47 UTC (permalink / raw) To: Andrew Morton Cc: Jessica Yu, Rusty Russell, LKML, kernel-hardening, Eddie Kovsky On Fri, Apr 7, 2017 at 3:23 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Fri, 7 Apr 2017 15:15:36 -0700 Kees Cook <keescook@chromium.org> wrote: > >> On Fri, Apr 7, 2017 at 3:12 PM, Andrew Morton <akpm@linux-foundation.org> wrote: >> > On Fri, 7 Apr 2017 14:53:23 -0700 Kees Cook <keescook@chromium.org> wrote: >> > >> >> > Eddie Kovsky (2): >> >> > module: verify address is read-only >> >> > extable: verify address is read-only >> >> > >> >> > include/linux/kernel.h | 2 ++ >> >> > include/linux/module.h | 12 ++++++++++++ >> >> > kernel/extable.c | 29 +++++++++++++++++++++++++++ >> >> > kernel/module.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> >> > 4 files changed, 96 insertions(+) >> >> >> >> Andrew, do you have these in your mailbox (it went to lkml), or should >> >> I resend them directly to you? Since they depend on the >> >> __start_ro_after_init naming fixes in -mm, it seemed like it'd be best >> >> to carry these two patches there. If so, please consider them both: >> >> >> >> Acked-by: Kees Cook <keescook@chromium.org> >> >> >> >> (And, from the thread on the module patch, Jessica has Acked that one too.) >> > >> > Well I grabbed them, but the patches don't actually do anything - they >> > add interfaces with no users. What's the plan here? >> >> I'd like to have a way for interfaces (especially the various >> *_register()) to be able to check that a structure is either const or >> __ro_after_init. My expectation is to add those and similar >> sanity-checks now that we can do so. > > OK. But I'd rather sit on the patches until we have working, tested, > reviewed callers which are agreed to be useful. That sounds fine to me. Thanks! -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-04-07 22:47 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-06 3:35 [PATCH v5 0/2] provide check for ro_after_init memory sections Eddie Kovsky 2017-04-06 3:35 ` [PATCH v5 1/2] module: verify address is read-only Eddie Kovsky 2017-04-07 1:58 ` Jessica Yu 2017-04-07 20:46 ` Kees Cook 2017-04-06 3:35 ` [PATCH v5 2/2] extable: " Eddie Kovsky 2017-04-06 17:20 ` kbuild test robot 2017-04-06 17:41 ` kbuild test robot 2017-04-07 19:29 ` Eddie Kovsky 2017-04-07 20:45 ` Kees Cook 2017-04-07 21:53 ` [PATCH v5 0/2] provide check for ro_after_init memory sections Kees Cook 2017-04-07 22:12 ` Andrew Morton 2017-04-07 22:15 ` Kees Cook 2017-04-07 22:23 ` Andrew Morton 2017-04-07 22:47 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).