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

Groups > linux.kernel > #1610075 > unrolled thread

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

Started byKees Cook <keescook@chromium.org>
First post2017-03-27 21:00 +0200
Last post2017-04-04 00:20 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  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

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

FromKees Cook <keescook@chromium.org>
Date2017-03-27 21:00 +0200
SubjectRe: [PATCH v4 2/2] extable: verify address is read-only
Message-ID<tpI1X-46a-7@gated-at.bofh.it>
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

>
> vim +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_data_ro_after_init &&
>    170              addr < (unsigned long)__end_data_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



-- 
Kees Cook
Pixel Security

[toc] | [next] | [standalone]

#1611536

FromEddie Kovsky <ewk@edkovsky.org>
Date2017-03-29 05:30 +0200
Message-ID<tqct3-VR-1@gated-at.bofh.it>
In reply to#1610075
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'. 

Eddie

> >
> > vim +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_data_ro_after_init &&
> >    170              addr < (unsigned long)__end_data_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
> 
> 
> 
> -- 
> Kees Cook
> Pixel Security

[toc] | [prev] | [next] | [standalone]

#1612533

FromJessica Yu <jeyu@redhat.com>
Date2017-03-30 05:40 +0200
Message-ID<tqz6h-8J-3@gated-at.bofh.it>
In reply to#1611536
+++ 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

[toc] | [prev] | [next] | [standalone]

#1613272

FromKees Cook <keescook@chromium.org>
Date2017-03-30 18:30 +0200
Message-ID<tqL7s-AJ-15@gated-at.bofh.it>
In reply to#1612533
On Wed, Mar 29, 2017 at 8:27 PM, Jessica Yu <jeyu@redhat.com> wrote:
> +++ 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.

Argh. Thank you for catching this! Yeah, that would have taken me
forever to find.

> 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

I don't have a blackfin cross-compiler set up, but I'm sure that'll
fix it. If you can, please base it on -next, since I rename
__[start|end]_data_ro_after_init to __[start|end]_ro_after_init (to
match the existing s390 symbols of the same purpose):

https://lkml.org/lkml/2017/3/27/685

akpm is carrying that patch, so this follow-up should likely go to him too.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1613588

FromEddie Kovsky <ewk@edkovsky.org>
Date2017-03-31 05:10 +0200
Message-ID<tqV6N-7m5-5@gated-at.bofh.it>
In reply to#1613272
On 03/30/17, Kees Cook wrote:
> On Wed, Mar 29, 2017 at 8:27 PM, Jessica Yu <jeyu@redhat.com> wrote:
> > +++ 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.
> 
> Argh. Thank you for catching this! Yeah, that would have taken me
> forever to find.
> 
> > 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
> 
> I don't have a blackfin cross-compiler set up, but I'm sure that'll
> fix it. If you can, please base it on -next, since I rename
> __[start|end]_data_ro_after_init to __[start|end]_ro_after_init (to
> match the existing s390 symbols of the same purpose):
> 
> https://lkml.org/lkml/2017/3/27/685
> 
I applied Jessica's patch and tested this again with a blackfin cross-compiler.
It fixes the error building extable.c.


> akpm is carrying that patch, so this follow-up should likely go to him too.
> 
> Thanks!
> 
> -Kees
> 
> -- 
> Kees Cook
> Pixel Security

[toc] | [prev] | [next] | [standalone]

#1615592

FromKees Cook <keescook@chromium.org>
Date2017-04-04 00:20 +0200
Message-ID<tsiul-4rW-3@gated-at.bofh.it>
In reply to#1612533
On Wed, Mar 29, 2017 at 8:27 PM, Jessica Yu <jeyu@redhat.com> wrote:
> +++ 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

Do you have a moment to send this? I'd like to get it in for v4.11 (my
renaming has landed, so you'll have to rebase). If not, I can do it.
:)

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web