Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1612533

Re: [PATCH v4 2/2] extable: verify address is read-only

Show all headers | View raw

+++ Eddie Kovsky [28/03/17 21:28 -0600]:
>On 03/27/17, Kees Cook wrote:
>> On Mon, Mar 27, 2017 at 1:43 AM, kbuild test robot <lkp@intel.com> wrote:
>> > Hi Eddie,
>> >
>> > [auto build test ERROR on next-20170323]
>> > [cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc4]
>> > [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/20170327-142922
>> > config: blackfin-BF561-EZKIT-SMP_defconfig (attached as .config)
>> > compiler: bfin-uclinux-gcc (GCC) 6.2.0
>> > reproduce:
>> >         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> >         chmod +x ~/bin/make.cross
>> >         # save the attached .config to linux build tree
>> >         make.cross ARCH=blackfin
>> >
>> > All errors (new ones prefixed by >>):
>> >
>> >    kernel/built-in.o: In function `core_kernel_rodata':
>> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init'
>> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init'
>>
>> Hm, I'm confused about this. blackfin includes
>> include/asm-generic-vmlinux.lds.h and uses the RO_DATA macro (which
>> resolves to RO_DATA_SECTION to RO_AFTER_INIT_DATA which defines
>> __[start|end]_data_ro_after_init.
>>
>> Also, it seems that commit d7c19b066dcf4bd19c4385e8065558d4e74f9e73
>> ("mm: kmemleak: scan .data.ro_after_init") added a potentially
>> redundant section name (s390 already calls this
>> __[start|end]_ro_after_init). I'd like to get this cleaned up, since
>> having multiple names for the same thing is confusing:
>>
>> diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
>> index 000e6e91f6a0..3667d20e997f 100644
>> --- a/arch/s390/kernel/vmlinux.lds.S
>> +++ b/arch/s390/kernel/vmlinux.lds.S
>> @@ -62,9 +62,11 @@ SECTIONS
>>
>>         . = ALIGN(PAGE_SIZE);
>>         __start_ro_after_init = .;
>> +       __start_data_ro_after_init = .;
>>         .data..ro_after_init : {
>>                  *(.data..ro_after_init)
>>         }
>> +       __end_data_ro_after_init = .;
>>         EXCEPTION_TABLE(16)
>>         . = ALIGN(PAGE_SIZE);
>>         __end_ro_after_init = .;
>>
>> And it seems that this hunk is wrong (__end_ro_after_init includes
>> s390's exception table, etc). I think we should remove the
>> ..._data_... name and use s390's name.
>>
>> I'll send an adjustment patch, but we'll still need to deal with blackfin.
>>
>> -Kees
>>
>
>Kees
>
>I applied your patch (mm: fix section name for .data..ro_after_init) and
>changed the new function in extable.c to use __[start|end]_ro_after_init
>instead. The new version still builds without errors on x86, which isn't
>surprising.
>
>I've cross compiled this for blackfin and I'm able to reproduce the
>build error. I'm still not sure why. As you pointed out, blackfin does
>appear to use 'include/asm-generic/vmlinux.lds.h'.

This appears to be because blackfin is one of the 2 arches that
prepends an underscore '_' to all symbols defined in C. I noticed that
__{start,end}_data_ro_after_init in vmlinux.lds.h are not wrapped with
VMLINUX_SYMBOL() which adds the necessary prefix for arches that have
HAVE_UNDERSCORE_SYMBOL_PREFIX, hence the undefined reference.

The below patch fixed the build error for me, if it works for you then
I can send a formal patch.

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 4e09b28..7b262f7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -260,9 +260,9 @@
  */
 #ifndef RO_AFTER_INIT_DATA
 #define RO_AFTER_INIT_DATA						\
-	__start_data_ro_after_init = .;					\
+	VMLINUX_SYMBOL(__start_data_ro_after_init) = .;			\
 	*(.data..ro_after_init)						\
-	__end_data_ro_after_init = .;
+	VMLINUX_SYMBOL(__end_data_ro_after_init) = .;
 #endif
 
 /*

Jessica

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v4 2/2] extable: verify address is read-only Kees Cook <keescook@chromium.org> - 2017-03-27 21:00 +0200
  Re: [PATCH v4 2/2] extable: verify address is read-only Eddie Kovsky <ewk@edkovsky.org> - 2017-03-29 05:30 +0200
    Re: [PATCH v4 2/2] extable: verify address is read-only Jessica Yu <jeyu@redhat.com> - 2017-03-30 05:40 +0200
      Re: [PATCH v4 2/2] extable: verify address is read-only Kees Cook <keescook@chromium.org> - 2017-03-30 18:30 +0200
        Re: [PATCH v4 2/2] extable: verify address is read-only Eddie Kovsky <ewk@edkovsky.org> - 2017-03-31 05:10 +0200
      Re: [PATCH v4 2/2] extable: verify address is read-only Kees Cook <keescook@chromium.org> - 2017-04-04 00:20 +0200

csiph-web